Skip to content

Commit

Permalink
Fix weird but correct line handling
Browse files Browse the repository at this point in the history
This handler function was returning `false`, thus signaling that it
had not handled the line, when it was not the responsibility of any
other handler to handle the line. It was doing this to rely on the
fall-through handlers determining whether to emit the line or skip it.
But this risks another handler handling it and is a violation of the
contract. It is much more appropriate to make the determination in the
handler itself, emit it if appropriate, and signal that it has been
handled.
  • Loading branch information
dandavison committed Dec 4, 2021
1 parent 5b8ce8c commit c80521b
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 3 deletions.
4 changes: 2 additions & 2 deletions src/delta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,12 @@ impl<'a> StateMachine<'a> {
}

/// Skip file metadata lines unless a raw diff style has been requested.
fn should_skip_line(&self) -> bool {
pub fn should_skip_line(&self) -> bool {
self.state == State::DiffHeader && self.should_handle() && !self.config.color_only
}

/// Emit unchanged any line that delta does not handle.
fn emit_line_unchanged(&mut self) -> std::io::Result<bool> {
pub fn emit_line_unchanged(&mut self) -> std::io::Result<bool> {
self.painter.emit()?;
writeln!(
self.painter.writer,
Expand Down
5 changes: 4 additions & 1 deletion src/handlers/diff_header_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ impl<'a> StateMachine<'a> {
self.state = State::DiffHeader;
self.handled_diff_header_header_line_file_pair = None;
self.diff_line = self.line.clone();
Ok(false)
if !self.should_skip_line() {
self.emit_line_unchanged()?;
}
Ok(true)
}
}

0 comments on commit c80521b

Please sign in to comment.