Repository navigation
pr: stop merged output after the last line without a page trailer - #15673
jayhemnani9910 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
|
Binary size comparison: |
| // page trailer to pad for. | ||
| if merge | ||
| && row.iter().all(Option::is_none) | ||
| && (feed_line_present || !options.display_header_and_trailer) |
There was a problem hiding this comment.
same condition as the break below, could you please put it in a variable like no_trailer?
There was a problem hiding this comment.
Done in 8df7385: no_trailer is set once before the loop and both breaks use it.
|
please don't compare with 9.7, it is old, you should compare with 9.12 |
| let (at, mut ucmd) = at_and_ucmd!(); | ||
| at.write("one", "a\nb\n"); | ||
| at.write("two", "c\nd\n"); | ||
| let result = ucmd.args(&["-m", flag, "one", "two"]).succeeds(); |
There was a problem hiding this comment.
please check the exact stdout rather than only the line count
With -m and -t, -T or -F, the merged files were padded with empty rows to the full page length, so two two-line files gave 56 lines; GNU pr prints the two lines.
28f7bb0 to
8df7385
Compare
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
|
Rebased on main (it conflicted with #15671 in test_pr.rs) and checked against GNU 9.12 instead of 9.7: the same 2 and 7 lines for the examples, and 45 |
With
-mand no page trailer (-t,-T,-F), pr pads the merged columns with empty rows up to the page length:This is the symptom in #14334, which no longer shows for a single file on main but still does with
-m. Single-file mode already stops at the first empty row in this case;write_columnsnow does the same for merged files. With a header and trailer the page is still padded, as GNU does.New test
test_merge_stops_after_last_line_without_headerchecks the exact output of-m -Jwith-t,-Tand-F(fails on main). The pr tests pass (86), clippy is clean. 45-minvocations (-t,-T,-F,-f,-l,-d,-n,-o,-J, three pairs of files with even and uneven lengths, with and without header) now print the same number of lines as GNU 9.12; apart from the column separator issues in #12313 the text matches GNU up to tab/space padding. #13540 rewrites merge mode and may cover this too.