Implement `e` (execute) command by PranavRJoshi · Pull Request #481 · uutils/sed

PranavRJoshi · 2026-07-03T10:34:43Z

Fixes #476.

Implements the standalone e command (a GNU extension). Supports the command with and without arguments.

Changes:

Added the e command in compiler.rs/processor.rs, both forms (with argument, bare/pattern-space). Argument parsing reuses the same escape decoding and backslash continuation as a/c/i, as GNU sed's e argument follows the same syntax.
Rejected under --posix and --sandbox at compile time.
Fixed e and s///e to silently swallowing the child shell's stderr.
Added 17 tests covering both command forms, addressing, ';' not separating the argument, escape decoding, backslash continuation, --posix/--sandbox rejection, and stderr surfacing on failure.

Irrelevant: fixed a typo found at delimited_parser.rs.

Implement the 'e' command. No argument implies execution of pattern space and any respective argument is to run with the output written to stream. Rejected under --posix and --sandbox. Also stop silently swallowing shell stderr for e and s///e. Signed-off-by: PranavRJoshi <pranavrjoshi1@gmail.com>

Includes 17 new tests. Cover both forms of the new 'e' command: with-argument execution (including shell side effects, addressing, ';' not separating the argument, escape decoding, and backslash continuation) and the no-argument pattern-space form. Also cover --posix/--sandbox rejection and stderr surfacing on command failure. Signed-off-by: PranavRJoshi <pranavrjoshi1@gmail.com>

codecov · 2026-07-03T10:37:40Z

Codecov Report

❌ Patch coverage is 0% with 127 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.60%. Comparing base (15def35) to head (fecf889).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/sed/processor.rs	0.00%	64 Missing ⚠️
src/sed/compiler.rs	0.00%	63 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #481      +/-   ##
==========================================
- Coverage   83.05%   81.60%   -1.46%     
==========================================
  Files          13       13              
  Lines        6120     6229     +109     
  Branches      358      367       +9     
==========================================
  Hits         5083     5083              
- Misses       1034     1143     +109     
  Partials        3        3

Flag	Coverage Δ
macos_latest	`82.31% <0.00%> (-1.46%)`	⬇️
ubuntu_latest	`82.41% <0.00%> (-1.43%)`	⬇️
windows_latest	`0.00% <0.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

PranavRJoshi · 2026-07-03T10:46:57Z

Seems like CRLF issue on Windows. Will make those tests conditionally compile on UNIX only.

dspinellis

Thank you for the quick turnaround and thorough testing! I added a few improvement suggestions.

dspinellis · 2026-07-03T13:38:09Z

+    let mut child = shell_command(cmd)
+        .stdout(std::process::Stdio::piped())
+        .spawn()?;
+    let mut stdout = child.stdout.take().expect("stdout was piped");


Better: "stdout should be piped"

dspinellis · 2026-07-03T13:42:53Z

+        .unwrap_err()
+    })?;
+    let mut shell_out = String::from_utf8_lossy(&stdout_bytes).into_owned();
+    if shell_out.ends_with("\r\n") {


Make this active only for #[cfg(windows)]. I wanted to comment on the previous PR, but let it pass to move ahead.

dspinellis · 2026-07-03T13:44:05Z

+    if shell_out.ends_with("\r\n") {
+        // On windows, both return carriage and newline characters are used
+        shell_out.truncate(shell_out.len() - 2);
+    } else if shell_out.ends_with('\n') {


But keep this also for Windows, because some Windows tools use \n.

dspinellis · 2026-07-03T13:59:41Z

+        )
+        .unwrap_err()
+    })?;
+    let mut shell_out = String::from_utf8_lossy(&stdout_bytes).into_owned();


As sed is a data processing tool rather than a UI front-end, we should avoid from_utf8_lossy, which can lead to silent data corruption. I suggest using (through a suitable method) IOChunk::from_content.

dspinellis · 2026-07-03T14:00:11Z

-                shell_out.pop();
-            }
-            pattern.set_to_string(shell_out, pattern.is_newline_terminated());
+            execute_pattern_as_shell(pattern, command, context)?;


Great reuse!

dspinellis · 2026-07-03T14:03:45Z

+
+/// Execute the pattern space as a shell command, replacing its contents
+/// with the command's standard output, minus one trailing newline.
+fn execute_pattern_as_shell(


Better names execute_pattern_through_shell or execute_pattern_as_shell_command .

dspinellis · 2026-07-03T14:12:41Z

+                            )
+                            .unwrap_err()
+                        })?;
+                        output.write_str(String::from_utf8_lossy(&stdout_bytes).into_owned())?;


Please avoid from_utf8_lossy

PranavRJoshi · 2026-07-03T15:40:52Z

Appreciate the thorough review. Will make the appropriate changes and commit them later.

Validate shell output as utf-8 and error out instead of lossily converting it. Make CRLF strip conditionally compile to Windows only. Rename execute_pattern_as_shell to execute_pattern_as_shell_command. Add tests for non-utf-8 error paths. Signed-off-by: PranavRJoshi <pranavrjoshi1@gmail.com>

Signed-off-by: PranavRJoshi <pranavrjoshi1@gmail.com>

dspinellis

I see some potential clashes with the highly relevant #487. Are you OK to delay the review a bit until we can land that so that we can ensure compatibility? In the meantime, please

try to address the drop in code coverage (I thought it would be addressed by your integration tests) , and
see why the GNU test suite results haven't changed. You can run the suite through util/run-gnu-testsuite.sh.

PranavRJoshi added 4 commits July 1, 2026 23:05

chore: fix typo

5f02eee

Merge branch 'uutils:main' into feat/e-comm

8a6e5bb

chore: gate LF-related tests to UNIIX only

0513b13

dspinellis requested changes Jul 3, 2026

View reviewed changes

PranavRJoshi added 5 commits July 7, 2026 20:44

Merge branch 'uutils:main' into feat/e-comm

11f1024

Merge branch 'uutils:main' into feat/e-comm

e70e5a9

add: Windows tests for e command with argument

31fd8b0

Signed-off-by: PranavRJoshi <pranavrjoshi1@gmail.com>

fix: broken pipe race in sandbox e command test

fecf889

Signed-off-by: PranavRJoshi <pranavrjoshi1@gmail.com>

PranavRJoshi requested a review from dspinellis July 8, 2026 16:22

dspinellis mentioned this pull request Jul 8, 2026

Improve GNU compatibility through byte processing #487

Open

dspinellis requested changes Jul 8, 2026

View reviewed changes

Uh oh!

Conversation

PranavRJoshi commented Jul 3, 2026

Uh oh!

codecov Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

PranavRJoshi commented Jul 3, 2026

Uh oh!

dspinellis left a comment

Choose a reason for hiding this comment

Uh oh!

dspinellis Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

dspinellis Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

dspinellis Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

dspinellis Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

dspinellis Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

dspinellis Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

dspinellis Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PranavRJoshi commented Jul 3, 2026

Uh oh!

dspinellis left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Jul 3, 2026 •

edited

Loading