Skip to content

sort: reject conflicting -t separators - #15707

Merged
sylvestre merged 3 commits into
uutils:mainfrom
fixfon:fix/15598-sort-incompatible-tabs
Oct 25, 2026
Merged

sylvestre merged 3 commits into
uutils:mainfrom
fixfon:fix/15598-sort-incompatible-tabs

Conversation

@fixfon

@fixfon fixfon commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Fixes #15598

sort -t@ -t! file used to sort with ! and say nothing. Because of args_override_self(true), the second -t quietly replaced the first, and uumain only ever saw the last value through get_one. Now -t is ArgAction::Append, and if two separators differ sort exits with code 2 and prints sort: incompatible tabs, which matches GNU. I copied the existing check for repeated -o that gives "multiple output files specified". Passing the same separator twice (-t: -t:) still works.

The comparison runs on the raw values, before the one-character check. So -t ab -t cd now gives incompatible tabs instead of "separator must be exactly one character long". Both exit with code 2.

I added two tests to tests/by-util/test_sort.rs. One checks that different separators fail, given as -t@, -t @ and --field-separator=@. The other checks that the same separator given twice still sorts. The first test fails without the fix, and test_sort passes with it. cargo fmt and clippy -D warnings are clean. I've only tested this on macOS. The French string in fr-FR.ftl is my own translation.

The -t argument kept the default set-action together with
args_override_self, so a later -t silently replaced an earlier one.
Collect every occurrence and fail with "incompatible tabs" (exit 2)
when they differ, the same way repeated -o is handled. Repeating the
same separator keeps working.

Fixes uutils#15598
Comment thread src/uu/sort/src/sort.rs Outdated

// Prevent -t/--field-separator from being given different values. Raw
// values suffice: `\0` is the only spelling rewritten later, and argv cannot hold a NUL.
if matches

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

almost the same code as the -o check just above, could be dedup in a small helper, no?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, extracted a helper.

@sylvestre
sylvestre merged commit 24f90f5 into uutils:main Oct 25, 2026
193 checks passed
@sylvestre

Copy link
Copy Markdown
Contributor

Thanks for your PR


#[test]
fn test_error_on_incompatible_separators() {
let cases: [&[&str]; 3] = [

@xtqqczze xtqqczze Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fixfon could you please follow-up by converting the test cases to #[rstest] cases

hubot pushed a commit to coreutils/coreutils that referenced this pull request Oct 25, 2026
* tests/sort/sort.pl (tab-dup1, tab-dup2, tab-incompat1)
(tab-incompat2, tab-incompat3): New tests.  Ensure differing -t
separators, including NUL, evoke "incompatible tabs", while
repeating the same one is accepted.
uutils/coreutils#15707

Link: #399
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(sort): different -t multiple values is accepted, instead of rejected

3 participants