join: apply the -e filler to empty output fields - #13718
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes join’s -e (“empty filler”) behavior so it applies not only when an output field is missing, but also when the field exists and is zero-length (e.g., blank lines, adjacent separators), matching POSIX wording and GNU join.
Changes:
- Add
Repr::field_or_emptyand route default output and-oformatting through it so empty fields receive the-efiller. - Remove an
unwrap()in default-field output while applying the filler consistently. - Add regression tests covering blank/whitespace-only lines and adjacent-separator empty fields, including
-e ""equivalence.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/uu/join/src/join.rs | Centralizes “field vs empty filler” resolution and applies it across default and -o output paths. |
| tests/by-util/test_join.rs | Adds tests ensuring -e is applied to zero-length fields and that -e "" preserves empty output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+224
to
+227
| /// The `--empty` filler stands in for output fields that are empty, which | ||
| /// covers both fields missing from the input line and fields that are | ||
| /// present but zero length. When `--empty` is not given the filler is | ||
| /// itself empty, so this leaves the output unchanged. |
Contributor
Author
There was a problem hiding this comment.
Correct, thanks — there is no --empty long option. Arg::new(e) only sets .short('e'), and GNU doesn't accept one either (join --empty=X gives join: unknown option -- empty; its --help lists just -e EMPTY). Reworded the doc comment to say -e.
GNU join replaces output fields that are empty as well as fields that are
missing from the input, which is how POSIX describes -e ("replace empty
output fields"). uutils only substituted when the requested field index ran
past the end of the line, so a field that was present but zero length was
printed as nothing.
A blank line is the common case: under whitespace splitting it yields a
single empty field rather than no fields at all, so `join -e X a a` printed
blank lines where GNU prints the filler.
Route the three output paths in Repr through a helper that treats an empty
field the same as a missing one. The filler defaults to the empty string, so
output is unchanged when -e is not given.
Fixes uutils#12770
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
dhruv-15-03
force-pushed
the
join-empty-field-filler
branch
from
August 2, 2026 18:13
696d982 to
ccf50b8
Compare
|
GNU testsuite comparison: |
Contributor
|
Thanks for your PR! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #12770
The problem
-ewas applied only when a requested output field was absent — that is, when the field index ran past the end of the input line. It was not applied when the field was present but zero length. POSIX specifies-eas replacing "empty output fields", and GNUjoinsubstitutes in both cases.A blank line is the case from the issue: under whitespace splitting it produces a single empty field rather than no fields at all, so the join key was an empty string and the filler was never reached.
Measured against GNU coreutils 8.32 on
main(b13ee7a), withacontaininghellofollowed by two blank lines andgapcontaininga,,b:join -e X a ahello,X,X,X,Xhellofollowed by four blank linesjoin -t, -e X -o 0,1.1,1.2,1.3 gap gapa,a,X,ba,a,,bjoin -t, -e X gap gapa,X,b,X,ba,,b,,bThe change
Adds
Repr::field_or_empty, which yields the-efiller for a field that is either missing or empty, and routes the three output paths through it:write_field— the join key in the default output formatwrite_fields— the remaining fields in the default output format. This one never consulted the filler at all, so-ehad no effect on non-key fields; the change also removes an.unwrap()write_format— the-opath, including-o autoThe filler defaults to the empty string, so output is unchanged when
-eis not given, and-e ''behaves the same as omitting the flag. All 35 existingjointests pass untouched, including the three that already exercise-e.I first suspected
WhitespaceSep::field_rangesof manufacturing a spurious trailing empty field, but GNU reports the same field counts there —join -e Xon the single linea b(trailing space) printsa b X b X— so the splitter is correct and is left alone.Verification
Ubuntu 22.04, rustc 1.97.1. Expected behaviour was established by running the GNU binary as a black box and from the POSIX wording for
-e; no GNU source was consulted.join, comparing stdout, stderr and exit status. 7 differed before the change, 0 differ after.cargo test --no-default-features --features join --test tests -- test_join— 37 passed, 0 failed (35 pre-existing plus 2 new).cargo clippy --no-default-features --features join --bin coreutils --all-targets -- -D warnings— clean.cargo fmt --all -- --check— clean.I did not run the GNU upstream test suite locally, so I am not claiming anything about its results.