fix: correct newline handling for p and x commands

src/sed/fast_io.rs

@@ -559,8 +559,11 @@ pub struct OutputBuffer {
     max_pending_write: usize,       // Max bytes to keep before flushing
     #[cfg(unix)]
     mmap_chunk: Option<MmapOutput>, // Chunk to write
+    // True when the last write didn't end with \n; the \n is deferred so
+    // that commands like `p` don't emit a spurious newline under -n.
+    pending_newline: bool,
     #[cfg(test)]
-    low_level_flushes: usize,       // Number of system call flushes
+    low_level_flushes: usize, // Number of system call flushes
 }
 
 /// Threshold to use buffered writes for output
@@ -591,6 +594,7 @@ impl OutputBuffer {
     pub fn new(w: Box<dyn OutputWrite + 'static>) -> Self {
         Self {
             out: BufWriter::new(w),
+            pending_newline: false,
             #[cfg(test)]
             low_level_flushes: 0,
         }
@@ -609,16 +613,22 @@ impl OutputBuffer {
             fast_copy,
             max_pending_write,
             mmap_chunk: None,
+            pending_newline: false,
             #[cfg(test)]
             low_level_flushes: 0,
         }
     }
 
-    /// Schedule the specified String or &strfor eventual output
+    /// Schedule the specified String or &str for eventual output
     pub fn write_str<S: Into<String>>(&mut self, s: S) -> io::Result<()> {
+        let mut s = s.into();
+        let has_newline = s.ends_with('\n');
+        if has_newline {
+            s.truncate(s.len() - 1);
+        }
         self.write_chunk(&IOChunk::from_content(IOChunkContent::new_owned(
-            s.into(),
-            false,
+            s,
+            has_newline,
         )))
     }
 
@@ -667,6 +677,16 @@ enum WriteRange {
 impl OutputBuffer {
     /// Schedule the specified output chunk for eventual output
     pub fn write_chunk(&mut self, new_chunk: &IOChunk) -> io::Result<()> {
+        if new_chunk.is_empty() && !new_chunk.is_newline_terminated() {
+            return Ok(());
+        }
+
+        if self.pending_newline {
+            self.flush_mmap(WriteRange::Complete)?;
+            self.out.write_all(b"\n")?;
+            self.pending_newline = false;
+        }
+
         match &new_chunk.content {
             IOChunkContent::MmapInput {
                 full_span,
@@ -712,6 +732,7 @@ impl OutputBuffer {
                         len: new_len,
                     });
                 }
+                self.pending_newline = !new_chunk.is_newline_terminated();
             }
 
             IOChunkContent::Owned {
@@ -724,6 +745,7 @@ impl OutputBuffer {
                 if *has_newline {
                     self.out.write_all(b"\n")?;
                 }
+                self.pending_newline = !has_newline;
             }
         }
         Ok(())
@@ -772,6 +794,16 @@ impl OutputBuffer {
         Ok(())
     }
 
+    /// Write a deferred newline if the last output didn't end with one.
+    pub fn flush_pending_newline(&mut self) -> io::Result<()> {
+        if self.pending_newline {
+            self.flush_mmap(WriteRange::Complete)?;
+            self.out.write_all(b"\n")?;
+            self.pending_newline = false;
+        }
+        Ok(())
+    }
+
     /// Flush everything: pending mmap and buffered data.
     pub fn flush(&mut self) -> io::Result<()> {
         self.flush_mmap(WriteRange::Complete)?; // flush mmap if any
@@ -783,6 +815,15 @@ impl OutputBuffer {
 impl OutputBuffer {
     /// Schedule the specified output chunk for eventual output
     pub fn write_chunk(&mut self, chunk: &IOChunk) -> io::Result<()> {
+        if chunk.is_empty() && !chunk.is_newline_terminated() {
+            return Ok(());
+        }
+
+        if self.pending_newline {
+            self.out.write_all(b"\n")?;
+            self.pending_newline = false;
+        }
+
         match &chunk.content {
             IOChunkContent::Owned {
                 content,
@@ -793,11 +834,21 @@ impl OutputBuffer {
                 if *has_newline {
                     self.out.write_all(b"\n")?;
                 }
+                self.pending_newline = !has_newline;
                 Ok(())
             }
         }
     }
 
+    /// Write a deferred newline if the last output didn't end with one.
+    pub fn flush_pending_newline(&mut self) -> io::Result<()> {
+        if self.pending_newline {
+            self.out.write_all(b"\n")?;
+            self.pending_newline = false;
+        }
+        Ok(())
+    }
+
     /// Flush everything: pending mmap and buffered data.
     pub fn flush(&mut self) -> io::Result<()> {
         self.out.flush() // then flush buffered data
@@ -1796,6 +1847,7 @@ mod tests {
             max_pending_write: 8,
             #[cfg(unix)]
             mmap_chunk: None,
+            pending_newline: false,
             low_level_flushes: 0,
         };
         (buf, file)
@@ -1847,9 +1899,9 @@ mod tests {
     fn mmap_new_chunk_and_coalesce() {
         let (mut outbuf, _file) = new_for_test(); // OutputBuffer
 
-        let backing = b"abcdefgh"; // contiguous buffer
-        let c1 = make_mmap_chunk(&backing[0..4]); // "abcd"
-        let c2 = make_mmap_chunk(&backing[4..8]); // "efgh"
+        let backing = b"abc\nefg\n"; // contiguous buffer, newline-terminated lines
+        let c1 = make_mmap_chunk(&backing[0..4]); // "abc\n"
+        let c2 = make_mmap_chunk(&backing[4..8]); // "efg\n"
 
         outbuf.write_chunk(&c1).unwrap();
         outbuf.write_chunk(&c2).unwrap();
@@ -1879,13 +1931,13 @@ mod tests {
     fn mmap_coalesce_and_flush_blocks() {
         let (mut buf, _file) = new_for_test();
         buf.max_pending_write = 4;
-        let backing = b"abcdefgh"; // contiguous buffer
-        let c1 = make_mmap_chunk(&backing[0..5]); // "abcde"
-        let c2 = make_mmap_chunk(&backing[5..8]); // "fgh"
+        let backing = b"abcde\nfgh\n"; // contiguous newline-terminated lines
+        let c1 = make_mmap_chunk(&backing[0..6]); // "abcde\n"
+        let c2 = make_mmap_chunk(&backing[6..10]); // "fgh\n"
 
         buf.write_chunk(&c1).unwrap();
         buf.write_chunk(&c2).unwrap();
-        // After a flush
+        // After a flush triggered by exceeding max_pending_write
         assert_eq!(buf.mmap_chunk.as_ref().unwrap().len, 0);
     }
 
@@ -1916,4 +1968,58 @@ mod tests {
 
         assert_eq!(out, "world\n");
     }
+
+    // pending_newline is injected between two no-newline chunks
+    #[test]
+    fn pending_newline_injected_between_chunks() {
+        let (mut buf, mut file) = new_for_test();
+        buf.write_chunk(&make_owned_chunk("first", false)).unwrap();
+        buf.write_chunk(&make_owned_chunk("second", true)).unwrap();
+        buf.out.flush().unwrap();
+        file.seek(SeekFrom::Start(0)).unwrap();
+        let mut out = String::new();
+        file.read_to_string(&mut out).unwrap();
+        assert_eq!(out, "first\nsecond\n");
+    }
+
+    // flush_pending_newline emits the deferred newline
+    #[test]
+    fn flush_pending_newline_emits_newline() {
+        let (mut buf, mut file) = new_for_test();
+        buf.write_chunk(&make_owned_chunk("foo", false)).unwrap();
+        assert!(buf.pending_newline);
+        buf.flush_pending_newline().unwrap();
+        assert!(!buf.pending_newline);
+        buf.out.flush().unwrap();
+        file.seek(SeekFrom::Start(0)).unwrap();
+        let mut out = String::new();
+        file.read_to_string(&mut out).unwrap();
+        assert_eq!(out, "foo\n");
+    }
+
+    // write_str strips trailing newline and sets pending_newline correctly
+    #[test]
+    fn write_str_with_trailing_newline() {
+        let (mut buf, mut file) = new_for_test();
+        buf.write_str("bar\n").unwrap();
+        assert!(!buf.pending_newline);
+        buf.out.flush().unwrap();
+        file.seek(SeekFrom::Start(0)).unwrap();
+        let mut out = String::new();
+        file.read_to_string(&mut out).unwrap();
+        assert_eq!(out, "bar\n");
+    }
+
+    #[test]
+    fn write_str_without_trailing_newline() {
+        let (mut buf, mut file) = new_for_test();
+        buf.write_str("baz").unwrap();
+        assert!(buf.pending_newline);
+        buf.flush_pending_newline().unwrap();
+        buf.out.flush().unwrap();
+        file.seek(SeekFrom::Start(0)).unwrap();
+        let mut out = String::new();
+        file.read_to_string(&mut out).unwrap();
+        assert_eq!(out, "baz\n");
+    }
 }

src/sed/mod.rs

@@ -211,7 +211,10 @@ fn build_context(matches: &ArgMatches) -> ProcessingContext {
         stop_processing: false,
         saved_regex: None,
         input_action: None,
-        hold: StringSpace::default(),
+        hold: StringSpace {
+            content: String::new(),
+            has_newline: true,
+        },
         parsed_block_nesting: 0,
         label_to_command_map: HashMap::new(),
         range_commands: Vec::new(),

src/sed/processor.rs

@@ -557,21 +557,14 @@ fn process_file(
                     continue 'lines;
                 }
                 'p' => {
-                    // Write the pattern space to standard output.
                     write_chunk(output, context, &pattern)?;
-                    if !pattern.is_newline_terminated() {
-                        // !chomped equivalent
-                        output.write_str("\n")?; // explicit \n
-                    }
                 }
                 'P' => {
-                    // Output pattern space, up to the first \n.
                     let line = pattern.as_str()?;
                     if let Some(pos) = line.find('\n') {
                         output.write_str(&line[..=pos])?;
                     } else {
-                        output.write_str(line)?;
-                        output.write_str("\n")?;
+                        write_chunk(output, context, &pattern)?;
                     }
                 }
                 'q' => {
@@ -652,6 +645,7 @@ fn process_file(
         flush_appends(output, context)?;
 
         if context.stop_processing {
+            output.flush_pending_newline()?;
             break;
         }
     }

tests/by-util/test_sed.rs

@@ -1212,7 +1212,7 @@ fn test_missing_address_re() {
 }
 
 ////////////////////////////////////////////////////////////
-// Test for issue #143: Missing newline in output with `-e p`
+// issue #143: p with no trailing newline
 #[test]
 fn test_print_command_adds_newline() {
     new_ucmd!()
@@ -1222,8 +1222,36 @@ fn test_print_command_adds_newline() {
         .stdout_is("foo\nfoo");
 }
 
-////////////////////////////////////////////////////////////
-// Test for issue #254: Missing newline in exchanged output with `2x`
+#[test]
+fn test_print_command_multiline_no_newline() {
+    new_ucmd!()
+        .args(&["-e", "p"])
+        .pipe_in("a\nfoo")
+        .succeeds()
+        .stdout_is("a\na\nfoo\nfoo");
+}
+
+// -n p must not add a trailing newline when input has none
+#[test]
+fn test_print_command_silent_no_newline() {
+    new_ucmd!()
+        .args(&["-n", "p"])
+        .pipe_in("foo")
+        .succeeds()
+        .stdout_is("foo");
+}
+
+// sanity: normal newline-terminated input is unaffected
+#[test]
+fn test_print_command_with_newline() {
+    new_ucmd!()
+        .args(&["-e", "p"])
+        .pipe_in("foo\n")
+        .succeeds()
+        .stdout_is("foo\nfoo\n");
+}
+
+// issue #254: 2x missing newline
 #[test]
 fn test_exchange_command_adds_newline() {
     new_ucmd!()
@@ -1232,3 +1260,33 @@ fn test_exchange_command_adds_newline() {
         .succeeds()
         .stdout_is("a\n\nc\n");
 }
+
+// issue #306: 1x with no-newline input should output an empty line
+#[test]
+fn test_exchange_no_newline_outputs_empty_line() {
+    new_ucmd!()
+        .args(&["1x"])
+        .pipe_in("abc")
+        .succeeds()
+        .stdout_is("\n");
+}
+
+// q with no newline input must not drop the trailing newline
+#[test]
+fn test_quit_no_newline() {
+    new_ucmd!()
+        .args(&["q"])
+        .pipe_in("foo")
+        .succeeds()
+        .stdout_is("foo\n");
+}
+
+// P with single line no newline input
+#[test]
+fn test_print_first_line_no_newline() {
+    new_ucmd!()
+        .args(&["-n", "P"])
+        .pipe_in("foo")
+        .succeeds()
+        .stdout_is("foo");
+}