Skip to content

fmt: default the width to goal + 10 when only --goal is given - #14275

Open
abhijeetvichare76 wants to merge 1 commit into
uutils:mainfrom
abhijeetvichare76:fmt-goal-linebreak
Open

fmt: default the width to goal + 10 when only --goal is given#14275
abhijeetvichare76 wants to merge 1 commit into
uutils:mainfrom
abhijeetvichare76:fmt-goal-linebreak

Conversation

@abhijeetvichare76

Copy link
Copy Markdown
Contributor

GNU documents the width as "75, or goal + 10 if goal is provided", and fmt -g N is byte-identical to fmt -w N+10 -g N there. We derived the width by inverting the 93% goal ratio instead, which comes out narrower and breaks lines GNU keeps together:

$ printf 'this\nis\na\nfile\nwith\none\nword\nper\nline\n' | fmt -g 30
GNU:  this is a file with one word per line
uu:   this is a file with one word
      per line

With -g 30 our width was 33 rather than 40, so the 37-column paragraph did not fit.

Refs #5162. That issue also covers a second difference which this does not address: once the width matches, the line-breaking cost function still disagrees with GNU, because it penalises a line for overshooting the goal as heavily as for undershooting it. The two #[ignore]d tests in test_fmt.rs need that part too, so they stay ignored here.

@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.19 MB (+1012 KB, +0.65%)

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

@abhijeetvichare76
abhijeetvichare76 marked this pull request as ready for review August 30, 2026 14:46
@github-actions

github-actions Bot commented Aug 30, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

Note: The gnu test tests/rm/many-dir-entries-vs-OOM is now being skipped but was previously passing.
Note: The gnu test tests/tail/pipe-f is now being skipped but was previously passing.

@luantaraschi luantaraschi left a comment

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.

I went and pinned GNU's actual rule here, because it is an easy one to get backwards, and this has it right.

GNU validates the goal against the default width first and widens afterwards, so the ceiling is 75 rather than the widened value:

$ fmt -g 75 f    ok
$ fmt -g 76 f    fmt: invalid width: '76': Numerical result out of range

which is why keeping the g > DEFAULT_WIDTH check next to the new w = g + DEFAULT_GOAL_WIDTH_SLACK is correct and not a leftover. Somebody reading the diff cold could easily take it for one. I compared -g G on its own against -w G+10 -g G on GNU coreutils 8.32 across the range and they agree.

One optional note on the test. Asserting that a 37 column paragraph fits at goal 30 pins the outcome but not the rule, so a wrong goal + 20 would pass it too. Comparing -g G against -w G+10 -g G would pin the rule itself.

GNU documents the width as "75, or goal + 10 if goal is provided", and
`fmt -g N` is byte-identical to `fmt -w N+10 -g N` there. We instead
inverted the 93% rule, which gives a narrower width and so breaks lines
GNU keeps together: with -g 30 the width came out as 33 rather than 40.

This is only part of the difference reported in uutils#5162; the line-breaking
cost function still disagrees with GNU once the width matches.
@abhijeetvichare76

abhijeetvichare76 commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

Hi @luantaraschi thanks for reviewing!
I've reworked the assertion to compare -g G against -w G+10 -g G over goals 5, 10, 20, 30, 50 and 65. I checked it does fail if I set the slack to 15 or 20, which the previous version didn't.

Also, dropped the old --goal loop, so the long form is now only exercised by that last assertion.

@luantaraschi

Copy link
Copy Markdown
Contributor

That pins the rule rather than the outcome. Thanks.

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.

2 participants