Linenumber refactoring #740
Conversation
Also move number and style logic out of `format_and_paint_line_numbers()` and into a separate `linenumbers_and_styles()` function.
| Some("commit") => s.push_str(&pad(&delta::format_raw_line(blame.commit, config))), | ||
| Some(Placeholder::Str("author")) => s.push_str(&pad(blame.author)), | ||
| Some(Placeholder::Str("commit")) => { | ||
| s.push_str(&pad(&delta::format_raw_line(blame.commit, config))) |
There was a problem hiding this comment.
To check my understanding: we might decide in the future to add "timestamp", "author", and "commit" to the Placeholder enum and handle them in the try_from implementation?
There was a problem hiding this comment.
Sure, these could be turned into enum fields as well. I left it at strs to show that this is still flexible enough for generic input.
| pub alignment_spec: Option<&'a str>, | ||
| pub width: Option<usize>, | ||
| pub suffix: &'a str, | ||
| pub prefix: SmolStr, |
There was a problem hiding this comment.
Is the basic idea here that this will be more efficient when the prefix/suffix material is incorporated into a "painted" string (i.e. wrapped with ANSI escapes) by ansi_term::Style::paint, and that this efficiency increase is due to the O(1) clone of SmolStr?
There was a problem hiding this comment.
If I read the ANSIGenericString<str> and style.paint() correctly then it won't copy anything but just hold a reference into the SmolStr in its Cow<'a, str>, the .into() should not copy anything, as per from: "Converts a string slice into a Borrowed variant. No heap allocation is performed, and the string is not copied."
And I actually thought of pre-calculating these, but then the placeholder would need a {prefix,suffix} x {left, right} = 4 extra fields for that. I was also tempted to sacrifice clarity for imagined performance gains and move suffix and prefix into a single field and only store offsets into it :)
One way to restore the ANSI background in side-by-side mode is to ensure the whole width of the terminal is used.
However if the width is odd, then one extra character needs to be inserted somewhere. A good place would be at the end of the first prefix, i.e. usually before the first line number. This lays the groundwork for that.
And according to
hyperfine, usingSmolStrdoes not cause a slowdown (here it was a tiny bit faster more often than not, but that might just be luck with the linker).