pr: implement the -e flag #10167

cerdelen · 2026-01-10T19:58:02Z

In Issue: #10100 the abscence of -e is raised. This Pr draft is the parsing as well as the implementation.

Tests are missing so far.

cerdelen · 2026-01-10T19:59:14Z

@ChrisDryden maybe you could have a look if this looks good to you as you also looked at this issue?

src/uu/pr/src/pr.rs

ChrisDryden · 2026-01-10T20:30:51Z

Can you add some integration tests for the cases that we talked about in the other PR? not to validate the correct output but high level things like it not consuming the file afterwards and working for both the short form and the long form?

cerdelen · 2026-01-10T20:32:31Z

Yes i will do that tomorrow (its late here already).

Also im almost certain i expand the chars with +1 too much but thats why this is a draft and i was just wanting to have someone look over it from a high level.

github-actions · 2026-01-11T20:04:43Z

GNU testsuite comparison:

Skip an intermittent issue tests/tty/tty-eof (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/tail/follow-name is no longer failing!

cerdelen · 2026-01-13T20:15:16Z

Hi, just wanted to ask if there is anything else needed for this to be merged? Thanks in advance.

ChrisDryden · 2026-01-14T00:52:16Z

Hey my bad that it took a while to get to this.

When I was thinking of tests I was thinking it would be better to have a few very basic ones instead of that comprehensive test suite you have, for the main reason that the current implementation of pr has many different missing things from the GNU implementation that it would probably be better to just add the basic tests that recognize that the values are not throwing an error and that they pass very basic tests that show that the value is being parsed, with a comment saying that more tests need to be added when the pr outputs are more closely matching the GNU pr implementation

Another way to view this is to look at the output of PR for the test cases you have currently and you will see that the GNU pr utility will product a very different output, so it would be better to wait until we are far enough along with this utility so that our integ tests test that it actually matches the output of the GNU implementation

ChrisDryden · 2026-01-14T00:54:17Z

src/uu/pr/locales/en-US.ftl

 # Page header text
 pr-page = Page

+pr-try-help-message = Try 'pr --help' for more information.


This message in other utilities we get from using the UError from uucore, it just wraps the error message with this for each utility. For example this error message: https://github.com/uutils/coreutils/blob/main/src/uu/stty/src/stty.rs#L447 is wrapped with: "Try "stty --help" for more imformation."

ChrisDryden · 2026-01-14T00:54:53Z

src/uu/pr/src/pr.rs

+                } else if s.len() > 1 {
+                    s[1..]
+                        .parse()
+                        .map_err(|_e| PrError::EncounteredErrors { msg: format!("{}\n{}", translate!("pr-error-invalid-expand-tab-argument", "arg" => &s[1..]), translate!("pr-try-help-message")) })


Same comment as here: https://github.com/uutils/coreutils/pull/10167/changes#r2688579349

ChrisDryden · 2026-01-14T00:54:59Z

src/uu/pr/src/pr.rs

+                if c.is_ascii_digit() {
+                    s
+                        .parse()
+                        .map_err(|_e| PrError::EncounteredErrors { msg: format!("{}\n{}", translate!("pr-error-invalid-expand-tab-argument", "arg" => s), translate!("pr-try-help-message")) })


Same comment as here: https://github.com/uutils/coreutils/pull/10167/changes#r2688579349

ChrisDryden · 2026-01-14T00:57:38Z

src/uu/pr/src/pr.rs

 }

-fn split_lines_if_form_feed(file_content: Result<String, std::io::Error>) -> Vec<FileLine> {
+fn split_lines_if_form_feed(


This is a bit tricky to review from the context that when trying to see if the implementation of -e is correct I don't have the baseline that the other aspects of the pr utility is working correctly.

I would personally just try to implement the handler first and then making sure that the base case for pr actually produces the same output as pr before implementing the logic for the many flags and options that can be provided.

cerdelen · 2026-01-14T18:47:15Z

Another way to view this is to look at the output of PR for the test cases you have currently and you will see that the GNU pr utility will product a very different output, so it would be better to wait until we are far enough along with this utility so that our integ tests test that it actually matches the output of the GNU implementation

Could you tell me which ones are having different output?

I created the expected files with the GNU pr and the flags i wanted to push so it should not produce different outputs. I also crossreferenced them on Mac as well as on Arch with the GNU utils so i would be sure that these are the outputs that GNU produces.

sylvestre · 2026-02-04T21:49:12Z

sorry, it needs to be rebased

pr: rebase

sylvestre · 2026-02-11T20:36:42Z

tests/by-util/test_pr.rs

+fn test_simple_expand_tab() {
+    let test_cases = vec![
+        (
+            "-e",


please generate the files on the fly

i would like to avoid having that many new files in the repo for the tests, thanks

src/uu/pr/locales/en-US.ftl

src/uu/pr/locales/fr-FR.ftl

github-actions · 2026-02-11T20:43:39Z

GNU testsuite comparison:

GNU test failed: tests/pr/bounded-memory. tests/pr/bounded-memory is passing on 'main'. Maybe you have to rebase?

tests/by-util/test_pr.rs

github-actions · 2026-02-12T21:19:29Z

GNU testsuite comparison:

GNU test failed: tests/pr/bounded-memory. tests/pr/bounded-memory is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/factor/t32 is no longer failing!
Congrats! The gnu test tests/factor/t33 is no longer failing!
Note: The gnu test tests/cp/link-heap is now being skipped but was previously passing.
Note: The gnu test tests/seq/seq-epipe is now being skipped but was previously passing.
Congrats! The gnu test tests/printf/printf-surprise is now passing!

codspeed-hq · 2026-02-12T21:23:10Z

Merging this PR will not alter performance

✅ 284 untouched benchmarks
⏩ 38 skipped benchmarks¹

_{Comparing cerdelen:pr_e_flag (df212b1) with main (dec633c)}

38 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

github-actions · 2026-02-12T22:58:35Z

GNU testsuite comparison:

GNU test failed: tests/misc/io-errors. tests/misc/io-errors is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/pr/bounded-memory. tests/pr/bounded-memory is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/tail/retry. tests/tail/retry is passing on 'main'. Maybe you have to rebase?
Note: The gnu test tests/expand/bounded-memory is now being skipped but was previously passing.
Note: The gnu test tests/rm/many-dir-entries-vs-OOM is now being skipped but was previously passing.
Congrats! The gnu test tests/printf/printf-surprise is now passing!

ChrisDryden reviewed Jan 10, 2026

View reviewed changes

src/uu/pr/src/pr.rs Outdated Show resolved Hide resolved

cerdelen force-pushed the pr_e_flag branch from 4d327cf to 48eb688 Compare January 11, 2026 19:54

cerdelen requested a review from ChrisDryden January 11, 2026 20:25

cerdelen marked this pull request as ready for review January 11, 2026 20:26

ChrisDryden mentioned this pull request Jan 14, 2026

pr: add -e/--expand-tabs option parsing #10146

Closed

ChrisDryden reviewed Jan 14, 2026

View reviewed changes

pr: new functionality: expand-tabs + tests

c1035d1

pr: rebase

cerdelen force-pushed the pr_e_flag branch from 2b98632 to c1035d1 Compare February 11, 2026 20:34

sylvestre reviewed Feb 11, 2026

View reviewed changes

sylvestre changed the title ~~Pr e flag~~ pr: implement the -e flag Feb 11, 2026

sylvestre reviewed Feb 11, 2026

View reviewed changes

src/uu/pr/locales/en-US.ftl Outdated Show resolved Hide resolved

sylvestre reviewed Feb 11, 2026

View reviewed changes

src/uu/pr/locales/fr-FR.ftl Outdated Show resolved Hide resolved

pr: pipe test input instead of using files

4de4228

cerdelen force-pushed the pr_e_flag branch from 8561b3b to 4de4228 Compare February 12, 2026 21:04

ChrisDryden reviewed Feb 12, 2026

View reviewed changes

tests/by-util/test_pr.rs Outdated Show resolved Hide resolved

pr: parameterize tests

df212b1

cerdelen force-pushed the pr_e_flag branch from 0bda210 to df212b1 Compare February 12, 2026 21:47

Uh oh!

pr: implement the -e flag #10167

Are you sure you want to change the base?

pr: implement the -e flag #10167

Conversation

cerdelen commented Jan 10, 2026

Uh oh!

cerdelen commented Jan 10, 2026

Uh oh!

Uh oh!

ChrisDryden commented Jan 10, 2026

Uh oh!

cerdelen commented Jan 10, 2026

Uh oh!

github-actions bot commented Jan 11, 2026

Uh oh!

cerdelen commented Jan 13, 2026

Uh oh!

ChrisDryden commented Jan 14, 2026

Uh oh!

ChrisDryden Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

ChrisDryden Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

ChrisDryden Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

ChrisDryden Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

cerdelen commented Jan 14, 2026

Uh oh!

sylvestre commented Feb 4, 2026

Uh oh!

sylvestre Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Feb 11, 2026

Uh oh!

Uh oh!

github-actions bot commented Feb 12, 2026

Uh oh!

codspeed-hq bot commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

github-actions bot commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codspeed-hq bot commented Feb 12, 2026 •

edited

Loading