Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove trailing \r from raw line, in presence of ANSI escape sequences #800

Merged
merged 1 commit into from
Nov 25, 2021
Merged

Remove trailing \r from raw line, in presence of ANSI escape sequences #800

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions src/delta.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,17 @@ impl<'a> StateMachine<'a> {
fn ingest_line(&mut self, raw_line_bytes: &[u8]) {
// TODO: retain raw_line as Cow
self.raw_line = String::from_utf8_lossy(raw_line_bytes).to_string();
// When a file has \r\n line endings, git sometimes adds ANSI escape sequences between the
// \r and \n, in which case byte_lines does not remove the \r. Remove it now.
if let Some(cr_index) = self.raw_line.rfind('\r') {
if ansi::strip_ansi_codes(&self.raw_line[cr_index + 1..]).is_empty() {
self.raw_line = format!(
"{}{}",
&self.raw_line[..cr_index],
&self.raw_line[cr_index + 1..]
);
}
}
if self.config.max_line_length > 0
&& self.raw_line.len() > self.config.max_line_length
// We must not truncate ripgrep --json output
Expand All @@ -152,13 +163,6 @@ impl<'a> StateMachine<'a> {
.to_string()
};
self.line = ansi::strip_ansi_codes(&self.raw_line);

// Strip the neglected CR.
// (CR-LF is unfortunately split by git because it adds ansi escapes between them.
// Thus byte_lines library can't remove the CR properly.)
if let Some(b'\r') = self.line.bytes().nth_back(0) {
self.line.truncate(self.line.len() - 1);
}
}
Copy link
Owner Author

@dandavison dandavison Nov 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I owe some tests, but we can see that with this commit we are still fixing the git add -p bug #664 that was the reason for the addition of the lines above (now deleted and replaced by the lines further above):

git add a.py && git commit -m .
mv b.py a.py

Without any \r-stripping, notice that one of the lines is not rendered due to the \r:

image
 With the code after this commit:
image


/// Skip file metadata lines unless a raw diff style has been requested.
Expand Down