Skip to content

fold: parse the obsolete -WIDTH form the way GNU does - #14248

Merged
cakebaker merged 5 commits into
uutils:mainfrom
arbelonson-source:fix/fold-obsolete-width-parsing
Oct 1, 2026
Merged

cakebaker merged 5 commits into
uutils:mainfrom
arbelonson-source:fix/fold-obsolete-width-parsing

Conversation

@arbelonson-source

@arbelonson-source arbelonson-source commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor

fold's handle_obsolete scanned every argument for -DIGITS and pulled the first match out, without checking whether that argument was already spoken for. Five GNU incompatibilities followed from that one function.

Found by differential testing against GNU coreutils 9.11; each row below was confirmed by running both binaries.

Command GNU uutils before
fold -w -1 FILE invalid number of columns: '-1' invalid number of columns: 'FILE'
fold -w3 -5 folds at 5 folds at 3
fold -3 -5 folds at 5 error: unexpected argument '-5' found
fold -w 3 -w 5 folds at 5 error: ... cannot be used multiple times
fold -- -3 fold: -3: No such file or directory reads stdin

The first row also applies to -bw -1, -sw -1 and --width -1: -1 was taken as the obsolete form, so -w bound to the file name and reported that as the invalid width.

Approach

Rather than lifting one width out of the argument list, the obsolete form is now rewritten into --width=N in place. That keeps the original left-to-right order, so clap can apply the same last-one-wins rule GNU gets from processing options in sequence — enabled here with args_override_self(true), matching what cut already does.

An argument is only eligible for that rewrite when it is not the value of -w/--width and has not been placed after a -- terminator. Short clusters are handled by position: -sw takes the next argument, while -w3 and -wb carry their value inline.

Testing

Three regression tests added. I verified they actually catch the bugs by reverting the change to fold.rs alone — all three fail, then pass again with it restored.

  • cargo test --features fold --no-default-features test_fold: 99 passed, 0 failed (96 pre-existing, 3 new)
  • cargo fmt --check and cargo clippy -p uu_fold --all-targets: clean
  • 26-case differential sweep against GNU: 24 match exactly

The 2 that still differ are fold -0 / fold -w 0, where GNU says Numerical result out of range and uutils says Result too large. That is a pre-existing wording difference in the error string, unrelated to argument parsing, so I left it alone rather than widen this PR.

Partially related to #5998 (allowing repeated flags), though this only changes fold.

Disclosure

Prepared with AI assistance (Claude Opus 5, via Claude Code), per the AI policy in CONTRIBUTING.md. On the GPL concern raised there specifically: the expected behavior was derived by running the installed GNU fold binary as a black box and recording its output. I did not read GNU coreutils source while writing this, and the implementation approach (rewrite-in-place + args_override_self) follows the existing pattern in this repo's cut, not anything from GNU. All testing above was run locally.

@github-actions

github-actions Bot commented Aug 30, 2026 •

Copy link
Copy Markdown

Binary size comparison:

Individual binary size comparison VS main (threshold: >=5% AND >=4 KB).

Total size of compared binaries: 152.20 MB (+1016 KB, +0.66%)

Significant per-binary changes:
  comm     1.12 MB ->    2.33 MB  (+1.21 MB, +107.29%)

@github-actions

github-actions Bot commented Aug 30, 2026 •

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/tail/tail-n0f (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/pr/bounded-memory (was skipped on 'main', now failing)

Comment thread src/uu/fold/src/fold.rs Outdated
arbelonson-source and others added 2 commits August 30, 2026 22:47
`handle_obsolete` scanned every argument for `-DIGITS` and pulled the
first match out, without checking whether that argument was already
spoken for. Five GNU incompatibilities followed:

  fold -w -1 FILE   reported FILE as the invalid width, not -1, because
                    -1 was taken as the obsolete form and -w then bound
                    to the file name. Same for -bw, -sw and --width.
  fold -w3 -5       folded at 3; GNU folds at 5, since it reads options
                    left to right and the last width wins.
  fold -3 -5        failed with "unexpected argument '-5'", as only the
                    first obsolete width was consumed.
  fold -w 3 -w 5    failed with "cannot be used multiple times".
  fold -- -3        read stdin, swallowing the width past the -- that
                    should have made it a file name.

Rewrite the obsolete form into --width=N in place instead, leaving the
order intact, and let clap pick the last one via args_override_self.
Arguments that are the value of -w/--width, and anything after --, are
no longer eligible.

The remaining difference on `fold -w 0` ("Result too large" against
GNU's "Numerical result out of range") is a separate wording issue and
is left alone.
@sylvestre
sylvestre force-pushed the fix/fold-obsolete-width-parsing branch from 61569ed to 17a7fe9 Compare August 30, 2026 20:47
@sylvestre

Copy link
Copy Markdown
Contributor

sorry, i broke it

sylvestre and others added 2 commits August 30, 2026 23:05
The underflow fixed in the previous commit had no test behind it. This
pins `-` as an operand in each position, including as the value of -w
where GNU rejects it as a width.
@arbelonson-source

Copy link
Copy Markdown
Contributor Author

No apology needed — and thank you for pushing the fix rather than waiting on me. short.find('w') == short.len().checked_sub(1) is neater than what I'd have written; comparing the two Options directly makes the empty case fall out instead of needing a guard in front of it.

I've added a test on top so it stays fixed: - as an operand on its own, alongside a width, and as the value of -w (where GNU rejects it as a width rather than reading it as the operand). It reproduces the abort on the commit before yours.

100 fold tests pass, cargo fmt --check clean.

Prepared with AI assistance (Claude Opus 5, via Claude Code); comparisons run locally against GNU coreutils 9.11.

`takes_width_value` decided a trailing `w` by comparing `find('w')` to
`len().checked_sub(1)`. For a bare `-` (short = ""), both sides are
`None` -- "no 'w' found" and "length underflowed" are different reasons,
but the comparison can't tell them apart, so it reported a match:

    $ fold - -5 <<< hello
    error: unexpected argument '-5' found          # uutils, before
    hello                                          # GNU, and uutils after

That made the `-` operand swallow whatever followed it as a width value
instead of leaving it for the obsolete-width scan, so `fold - -5` failed
to parse where GNU folds normally. `ends_with('w')` is false on an empty
string, so it doesn't share the bug.

Found while investigating an unrelated CI failure in this PR (a broken
pipe in test_dash_operand_reads_stdin, which turned out to be a flaky
cross-compiled job, not this) -- this bug doesn't explain that failure,
but turned up real and worth fixing while looking at this function.
@cakebaker
cakebaker merged commit 2b85f6f into uutils:main Oct 1, 2026
175 of 176 checks passed
@cakebaker

Copy link
Copy Markdown
Contributor

Thanks!

@xtqqczze

xtqqczze commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

test_fold::test_dash_operand_reads_stdin (introduced here) is failing: #15004

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.

4 participants