From 887221a51887ab9f2b6095be86633afd40e02638 Mon Sep 17 00:00:00 2001 From: gwenn Date: Tue, 30 Jun 2020 18:57:31 +0200 Subject: [PATCH 1/4] Improve History::save No copy. Single pass. --- src/history.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/history.rs b/src/history.rs index ffcd389d75..a83dfe68b5 100644 --- a/src/history.rs +++ b/src/history.rs @@ -141,7 +141,18 @@ impl History { wtr.write_all(Self::FILE_VERSION_V2.as_bytes())?; for entry in &self.entries { wtr.write_all(b"\n")?; - wtr.write_all(entry.replace('\\', "\\\\").replace('\n', "\\n").as_bytes())?; + let mut bytes = entry.as_bytes(); + while let Some(i) = memchr::memchr2(b'\\', b'\n', bytes) { + wtr.write_all(&bytes[..i])?; + if bytes[i] == b'\n' { + wtr.write_all(b"\\n")?; + } else { + debug_assert_eq!(bytes[i], b'\\'); + wtr.write_all(b"\\\\")?; + } + bytes = &bytes[i + 1..]; + } + wtr.write_all(bytes)?; // remaining bytes } wtr.write_all(b"\n")?; // https://github.com/rust-lang/rust/issues/32677#issuecomment-204833485 From 0f8cbe5591a7118c0d55b405c1ddea6b8e736bba Mon Sep 17 00:00:00 2001 From: gwenn Date: Tue, 30 Jun 2020 18:59:37 +0200 Subject: [PATCH 2/4] Add unit test to reproduce bug #409 --- src/history.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/history.rs b/src/history.rs index a83dfe68b5..7b7bd88fb6 100644 --- a/src/history.rs +++ b/src/history.rs @@ -360,6 +360,12 @@ mod tests { check_save("line\nfour \\ abc") } + #[test] + fn save_windows_path() -> Result<()> { + let path = "cd source\\repos\\forks\\nushell\\"; + check_save(path) + } + #[cfg_attr(miri, ignore)] // unsupported operation: `getcwd` not available when isolation is enabled fn check_save(line: &str) -> Result<()> { let mut history = init(); From 13b999eb90b777b49910851b33d8b85bf20e8090 Mon Sep 17 00:00:00 2001 From: gwenn Date: Tue, 30 Jun 2020 19:09:13 +0200 Subject: [PATCH 3/4] Fix History::load No copy if no escaped char, single copy otherwise. Single pass. --- src/history.rs | 41 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/src/history.rs b/src/history.rs index 7b7bd88fb6..ee0ee05f0f 100644 --- a/src/history.rs +++ b/src/history.rs @@ -1,5 +1,6 @@ //! History API +use log::warn; use std::collections::vec_deque; use std::collections::VecDeque; use std::fs::File; @@ -180,14 +181,44 @@ impl History { } } for line in lines { - let line = if v2 { - line?.replace("\\n", "\n").replace("\\\\", "\\") - } else { - line? - }; + let mut line = line?; if line.is_empty() { continue; } + if v2 { + let mut copy = None; + let bytes = line.as_bytes(); + let mut i = 0; + while let Some(j) = memchr::memchr(b'\\', &bytes[i..]) { + if copy.is_none() { + copy = Some(String::with_capacity(line.len())); + } + let s = copy.as_mut().unwrap(); + s.push_str(&line[i..i + j]); + let k = i + j + 1; // escaped char idx + let bad = if k >= bytes.len() { + true + } else if bytes[k] == b'n' { + s.push('\n'); + false + } else if bytes[k] == b'\\' { + s.push('\\'); + false + } else { + true + }; + if bad { + warn!(target: "rustyline", "bad escaped line: {}", line); + copy = None; + break; + } + i = k + 1; + } + if let Some(mut s) = copy { + s.push_str(&line[i..]); + line = s; + } + } self.add(line); // TODO truncate to MAX_LINE } Ok(()) From 441ed1be2cb16dfeee677ad5f0facafbb450a1a8 Mon Sep 17 00:00:00 2001 From: gwenn Date: Wed, 1 Jul 2020 20:22:23 +0200 Subject: [PATCH 4/4] Using a string slice seems more idiomatic --- src/history.rs | 50 ++++++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/src/history.rs b/src/history.rs index ee0ee05f0f..655a59c8d2 100644 --- a/src/history.rs +++ b/src/history.rs @@ -146,14 +146,14 @@ impl History { while let Some(i) = memchr::memchr2(b'\\', b'\n', bytes) { wtr.write_all(&bytes[..i])?; if bytes[i] == b'\n' { - wtr.write_all(b"\\n")?; + wtr.write_all(b"\\n")?; // escaped line feed } else { debug_assert_eq!(bytes[i], b'\\'); - wtr.write_all(b"\\\\")?; + wtr.write_all(b"\\\\")?; // escaped backslash } bytes = &bytes[i + 1..]; } - wtr.write_all(bytes)?; // remaining bytes + wtr.write_all(bytes)?; // remaining bytes with no \n or \ } wtr.write_all(b"\n")?; // https://github.com/rust-lang/rust/issues/32677#issuecomment-204833485 @@ -186,36 +186,38 @@ impl History { continue; } if v2 { - let mut copy = None; - let bytes = line.as_bytes(); - let mut i = 0; - while let Some(j) = memchr::memchr(b'\\', &bytes[i..]) { + let mut copy = None; // lazily copy line if unescaping is needed + let mut str = line.as_str(); + while let Some(i) = str.find('\\') { if copy.is_none() { copy = Some(String::with_capacity(line.len())); } let s = copy.as_mut().unwrap(); - s.push_str(&line[i..i + j]); - let k = i + j + 1; // escaped char idx - let bad = if k >= bytes.len() { - true - } else if bytes[k] == b'n' { - s.push('\n'); - false - } else if bytes[k] == b'\\' { - s.push('\\'); - false + s.push_str(&str[..i]); + let j = i + 1; // escaped char idx + let b = if j < str.len() { + str.as_bytes()[j] } else { - true + 0 // unexpected if History::save works properly }; - if bad { - warn!(target: "rustyline", "bad escaped line: {}", line); - copy = None; - break; + match b { + b'n' => { + s.push('\n'); // unescaped line feed + } + b'\\' => { + s.push('\\'); // unescaped back slash + } + _ => { + // only line feed and back slash should have been escaped + warn!(target: "rustyline", "bad escaped line: {}", line); + copy = None; + break; + } } - i = k + 1; + str = &str[j + 1..]; } if let Some(mut s) = copy { - s.push_str(&line[i..]); + s.push_str(str); // remaining bytes with no escaped char line = s; } }