bat
bat copied to clipboard
Add `bat::PrettyPrinter::clear_highlights`
Fixes #1919
With this API, the code written in #1919 can be improved as follows:
use bat::PrettyPrinter;
let targets: (PathBuf, Vec<(usize, usize)>) = vec![];
let mut pp = PrettyPrinter::new();
for (path, ranges) in targets.iter() {
pp.input_file(path);
for (start, end) in ranges.iter() {
pp.highlight_range(*start, *end);
}
pp.print().unwrap();
// Clear highlights for next iteration
pp.clear_highlights();
}
This request/bug-ticket looks very reasonable to me - thank you! However, do we really need a new public method for that? Shouldn't clear_highlights be called automatically after .print()ing?
@sharkdp
Shouldn't clear_highlights be called automatically after .print()ing?
It also makes sense.
https://github.com/sharkdp/bat/blob/7fe4fdf33df3aa958b3263a939a8b24b9d06f534/src/pretty_printer.rs#L242-L248
The line ranges are cloned into config.highlighted_lines at L248. So the line ranges in PrettyPrinter instance does not change currently.
Since inputs are cleared, clearing line ranges makes sense to me. The point is that it is a breaking change in API. I avoided the breaking change but clearing the line ranges at print() is sufficient for me. Do you think the breaking change is OK?
let hls = std::mem::take(&mut self.highlighted_lines);
self.config.highlighted_lines = HighlightedLineRanges(LineRanges::from(hls));
The point is that it is a breaking change in API. I avoided the breaking change but clearing the line ranges at
print()is sufficient for me. Do you think the breaking change is OK?
Not sure I understand this correctly: wouldn't the breaking change only affect the cases where the behavior is currently broken/buggy anyway? (#1919)
@rhysd Can you provide a quick update please? Are you still interested in this change? Or has the situation improved due to syntax lazy loading (see accompanying ticket)?
I'm sorry for the lack of response. I forgot this PR. I'll take a look tonight.
wouldn't the breaking change only affect the cases where the behavior is currently broken/buggy anyway?
I'm not sure it is broken/buggy, but I agree that it is very edge case.
@sharkdp I updated this branch at 3d7817d. Would you review changes?
Looks great - thank you. I like the implementation :+1: