fix(diff): better handling of text with only line ending differences #11212
Conversation
| /// Diff format is loosely based on Github diff formatting. | ||
| pub fn diff(orig_text: &str, edit_text: &str) -> String { | ||
| let lines = edit_text.split('\n').count(); | ||
| let line_number_width = lines.to_string().chars().count(); |
There was a problem hiding this comment.
Bug here fixed in new code. Should take into account the max of orig_text and edit_text line count.
| } | ||
| if !text2.ends_with('\n') { | ||
| text2.push('\n'); | ||
| } |
There was a problem hiding this comment.
Bug here. Should not do this because it should display a diff for when one has a trailing newline and the other doesn't.
| orig: String, | ||
| edit: String, | ||
| has_changes: bool, | ||
| } |
There was a problem hiding this comment.
Unfortunately I had to change the function to a struct in order to make the borrow checker happy and not duplicate a lot of code with the new flush_changes method.
| let edit_text = edit_text.replace("\r\n", "\n"); | ||
|
|
||
| if orig_text == edit_text { | ||
| return " | Text differed by line endings.\n".to_string(); |
There was a problem hiding this comment.
Fix here. This is what I do in dprint. It prevents entire files from being spammed.
| let line_count = std::cmp::max( | ||
| orig_text.split('\n').count(), | ||
| edit_text.split('\n').count(), | ||
| ); |
| } | ||
| } | ||
|
|
||
| self.flush_changes(); |
There was a problem hiding this comment.
Fix here to flush the changes here as well.
| /// Print diff of the same file_path, before and after formatting. | ||
| /// | ||
| /// Diff format is loosely based on Github diff formatting. | ||
| pub fn diff(orig_text: &str, edit_text: &str) -> String { |
There was a problem hiding this comment.
Sorry, the diff here wasn't going to be pretty no matter what, so I took the liberty of reorganizing the file. For the most part it's largely the same other than a few fixes. I've commented everywhere that's changed.
| run_test( | ||
| "console.log('Hello World')", | ||
| "console.log(\"Hello World\");", | ||
| concat!( | ||
| "1 | -console.log('Hello World')\n", | ||
| "1 | +console.log(\"Hello World\");\n", | ||
| ), | ||
| ); | ||
|
|
||
| let line_number_unfmt = "\n\n\n\nconsole.log(\n'Hello World'\n)"; | ||
| let line_number_fmt = "console.log(\n\"Hello World\"\n);"; | ||
| run_test( | ||
| "\n\n\n\nconsole.log(\n'Hello World'\n)", | ||
| "console.log(\n\"Hello World\"\n);", | ||
| concat!( | ||
| "1 | -\n", | ||
| "2 | -\n", | ||
| "3 | -\n", | ||
| "4 | -\n", | ||
| "5 | -console.log(\n", | ||
| "1 | +console.log(\n", | ||
| "6 | -'Hello World'\n", | ||
| "2 | +\"Hello World\"\n", | ||
| "7 | -)\n3 | +);\n", | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_eof_newline_missing() { | ||
| run_test( |
Also fixed a few additional bugs like:
99lines and the edit text had100lines.I commented everywhere in the PR that actually changed other than the refactor to make the borrow checker happy without duplicating code.
Closes #8118