Handle a mode change on a renamed file. (#875)

* Handle a mode change on a renamed file.

I changed the diff parsing to cache the mode info from the old/new mode
lines until the parsing bumps into the start of the actual related diff,
finds the diff line for an unrelated diff, or runs out of input.  This
allows the mode info to be output in conjunction with a file event
instead of as a separate heading (that used to have an empty name when
the mode change was for a rename).

The mode info is passed down into the draw routines as a separate
"addendum" string that the draw code can use as it likes. Currently that
means that it appends the addendum string to the non-raw string in
parens. I imagine that future draw routines could decide to put it in a
separate box or some other per-routine method. There is currently a
single function in src/handlers/draw.rs that joins the strings in the
same way for everyone.

A couple examples of how the new code looks:

Δ foo.rs (mode +x)
───────────────────────────────────────────────────

renamed: old-longer-name → shorter-name (mode +x)
───────────────────────────────────────────────────

Would it look better on its own line?

Δ foo.rs
• mode +x
───────────────────────────────────────────────────

renamed: old-longer-name → shorter-name
• mode +x
───────────────────────────────────────────────────

Would it look better appended after a "•" character?

Δ foo.rs • mode +x
───────────────────────────────────────────────────

renamed: old-longer-name → shorter-name • mode +x
───────────────────────────────────────────────────

Should it be a user option? If so, we can do that later.

1 files changed, 86 insertions, 62 deletions

diff --git a/src/handlers/diff_header.rs b/src/handlers/diff_header.rs
index f90c653..e3b29a3 100644
--- a/src/handlers/diff_header.rs
+++ b/src/handlers/diff_header.rs
@@ -17,20 +17,47 @@ pub enum FileEvent {
     Change,
     Copy,
     Rename,
-    ModeChange(String),
     NoEvent,
 }
 
 impl<'a> StateMachine<'a> {
+    /// Check for the old mode|new mode lines and cache their info for later use.
+    pub fn handle_diff_header_mode_line(&mut self) -> std::io::Result<bool> {
+        let mut handled_line = false;
+        if let Some(line_suf) = self.line.strip_prefix("old mode ") {
+            self.state = State::DiffHeader(DiffType::Unified);
+            if self.should_handle() && !self.config.color_only {
+                self.mode_info = line_suf.to_string();
+                handled_line = true;
+            }
+        } else if let Some(line_suf) = self.line.strip_prefix("new mode ") {
+            self.state = State::DiffHeader(DiffType::Unified);
+            if self.should_handle() && !self.config.color_only && !self.mode_info.is_empty() {
+                self.mode_info = match (self.mode_info.as_str(), line_suf) {
+                    // 100755 for executable and 100644 for non-executable are the only file modes Git records.
+                    // https://medium.com/@tahteche/how-git-treats-changes-in-file-permissions-f71874ca239d
+                    ("100644", "100755") => "mode +x".to_string(),
+                    ("100755", "100644") => "mode -x".to_string(),
+                    _ => format!(
+                        "mode {} {} {}",
+                        self.mode_info, self.config.right_arrow, line_suf
+                    ),
+                };
+                handled_line = true;
+            }
+        }
+        Ok(handled_line)
+    }
+
     #[inline]
     fn test_diff_header_minus_line(&self) -> bool {
         (matches!(self.state, State::DiffHeader(_)) || self.source == Source::DiffUnified)
             && (self.line.starts_with("--- ")
                 || self.line.starts_with("rename from ")
-                || self.line.starts_with("copy from ")
-                || self.line.starts_with("old mode "))
+                || self.line.starts_with("copy from "))
     }
 
+    /// Check for and handle the "--- filename ..." line.
     pub fn handle_diff_header_minus_line(&mut self) -> std::io::Result<bool> {
         if !self.test_diff_header_minus_line() {
             return Ok(false);
@@ -46,14 +73,7 @@ impl<'a> StateMachine<'a> {
                 None
             },
         );
-        // In the case of ModeChange only, the file path is taken from the diff
-        // --git line (since that is the only place the file path occurs);
-        // otherwise it is taken from the --- / +++ line.
-        self.minus_file = if let FileEvent::ModeChange(_) = &file_event {
-            get_repeated_file_path_from_diff_line(&self.diff_line).unwrap_or(path_or_mode)
-        } else {
-            path_or_mode
-        };
+        self.minus_file = path_or_mode;
         self.minus_file_event = file_event;
 
         if self.source == Source::DiffUnified {
@@ -76,6 +96,7 @@ impl<'a> StateMachine<'a> {
                 &self.line,
                 &self.raw_line,
                 &mut self.painter,
+                &mut self.mode_info,
                 self.config,
             )?;
             handled_line = true;
@@ -88,10 +109,10 @@ impl<'a> StateMachine<'a> {
         (matches!(self.state, State::DiffHeader(_)) || self.source == Source::DiffUnified)
             && (self.line.starts_with("+++ ")
                 || self.line.starts_with("rename to ")
-                || self.line.starts_with("copy to ")
-                || self.line.starts_with("new mode "))
+                || self.line.starts_with("copy to "))
     }
 
+    /// Check for and handle the "+++ filename ..." line.
     pub fn handle_diff_header_plus_line(&mut self) -> std::io::Result<bool> {
         if !self.test_diff_header_plus_line() {
             return Ok(false);
@@ -106,14 +127,7 @@ impl<'a> StateMachine<'a> {
                 None
             },
         );
-        // In the case of ModeChange only, the file path is taken from the diff
-        // --git line (since that is the only place the file path occurs);
-        // otherwise it is taken from the --- / +++ line.
-        self.plus_file = if let FileEvent::ModeChange(_) = &file_event {
-            get_repeated_file_path_from_diff_line(&self.diff_line).unwrap_or(path_or_mode)
-        } else {
-            path_or_mode
-        };
+        self.plus_file = path_or_mode;
         self.plus_file_event = file_event;
         self.painter
             .set_syntax(get_file_extension_from_diff_header_line_file_path(
@@ -130,6 +144,7 @@ impl<'a> StateMachine<'a> {
                 &self.line,
                 &self.raw_line,
                 &mut self.painter,
+                &mut self.mode_info,
                 self.config,
             )?;
             handled_line = true
@@ -154,7 +169,45 @@ impl<'a> StateMachine<'a> {
             self.config,
         );
         // FIXME: no support for 'raw'
-        write_generic_diff_header_header_line(&line, &line, &mut self.painter, self.config)
+        write_generic_diff_header_header_line(
+            &line,
+            &line,
+            &mut self.painter,
+            &mut self.mode_info,
+            self.config,
+        )
+    }
+
+    pub fn handle_pending_mode_line_with_diff_name(&mut self) -> std::io::Result<()> {
+        if !self.mode_info.is_empty() {
+            let format_label = |label: &str| {
+                if !label.is_empty() {
+                    format!("{} ", label)
+                } else {
+                    "".to_string()
+                }
+            };
+            let format_file = |file| {
+                if self.config.hyperlinks {
+                    features::hyperlinks::format_osc8_file_hyperlink(file, None, file, self.config)
+                } else {
+                    Cow::from(file)
+                }
+            };
+            let label = format_label(&self.config.file_modified_label);
+            let name = get_repeated_file_path_from_diff_line(&self.diff_line)
+                .unwrap_or_else(|| "".to_string());
+            let line = format!("{}{}", label, format_file(&name));
+            write_generic_diff_header_header_line(
+                &line,
+                &line,
+                &mut self.painter,
+                &mut self.mode_info,
+                self.config,
+            )
+        } else {
+            Ok(())
+        }
     }
 }
 
@@ -163,6 +216,7 @@ pub fn write_generic_diff_header_header_line(
     line: &str,
     raw_line: &str,
     painter: &mut Painter,
+    mode_info: &mut String,
     config: &Config,
 ) -> std::io::Result<()> {
     // If file_style is "omit", we'll skip the process and print nothing.
@@ -181,10 +235,14 @@ pub fn write_generic_diff_header_header_line(
         painter.writer,
         &format!("{}{}", line, if pad { " " } else { "" }),
         &format!("{}{}", raw_line, if pad { " " } else { "" }),
+        mode_info,
         &config.decorations_width,
         config.file_style,
         decoration_ansi_term_style,
     )?;
+    if !mode_info.is_empty() {
+        mode_info.truncate(0);
+    }
     Ok(())
 }
 
@@ -239,18 +297,11 @@ fn parse_diff_header_line(
         line if line.starts_with("copy to ") => {
             (line[8..].to_string(), FileEvent::Copy) // "copy to ".len()
         }
-        line if line.starts_with("old mode ") => {
-            ("".to_string(), FileEvent::ModeChange(line[9..].to_string())) // "old mode ".len()
-        }
-        line if line.starts_with("new mode ") => {
-            ("".to_string(), FileEvent::ModeChange(line[9..].to_string())) // "new mode ".len()
-        }
         _ => ("".to_string(), FileEvent::NoEvent),
     };
 
     if let Some(base) = relative_path_base {
-        if let FileEvent::ModeChange(_) = file_event {
-        } else if let Some(relative_path) = pathdiff::diff_paths(&path_or_mode, base) {
+        if let Some(relative_path) = pathdiff::diff_paths(&path_or_mode, base) {
             if let Some(relative_path) = relative_path.to_str() {
                 path_or_mode = relative_path.to_owned();
             }
@@ -326,33 +377,6 @@ pub fn get_file_change_description_from_file_paths(
             }
         };
         match (minus_file, plus_file, minus_file_event, plus_file_event) {
-            (
-                minus_file,
-                plus_file,
-                FileEvent::ModeChange(old_mode),
-                FileEvent::ModeChange(new_mode),
-            ) if minus_file == plus_file => match (old_mode.as_str(), new_mode.as_str()) {
-                // 100755 for executable and 100644 for non-executable are the only file modes Git records.
-                // https://medium.com/@tahteche/how-git-treats-changes-in-file-permissions-f71874ca239d
-                ("100644", "100755") => format!(
-                    "{}{}: mode +x",
-                    format_label(&config.file_modified_label),
-                    format_file(plus_file)
-                ),
-                ("100755", "100644") => format!(
-                    "{}{}: mode -x",
-                    format_label(&config.file_modified_label),
-                    format_file(plus_file)
-                ),
-                _ => format!(
-                    "{}{}: {} {} {}",
-                    format_label(&config.file_modified_label),
-                    format_file(plus_file),
-                    old_mode,
-                    config.right_arrow,
-                    new_mode
-                ),
-            },
             (minus_file, plus_file, _, _) if minus_file == plus_file => format!(
                 "{}{}",
                 format_label(&config.file_modified_label),
@@ -368,8 +392,7 @@ pub fn get_file_change_description_from_file_paths(
                 format_label(&config.file_added_label),
                 format_file(plus_file)
             ),
-            // minus_file_event == plus_file_event, except in the ModeChange
-            // case above.
+            // minus_file_event == plus_file_event
             (minus_file, plus_file, file_event, _) => format!(
                 "{}{} {} {}",
                 format_label(match file_event {
@@ -549,8 +572,9 @@ mod tests {
             None
         );
         assert_eq!(
-        get_repeated_file_path_from_diff_line("diff --git a/.config/Code - Insiders/User/settings.json b/.config/Code - Insiders/User/settings.json"),
-        Some(".config/Code - Insiders/User/settings.json".to_string())
-    );
+            get_repeated_file_path_from_diff_line(
+                "diff --git a/.config/Code - Insiders/User/settings.json b/.config/Code - Insiders/User/settings.json"),
+            Some(".config/Code - Insiders/User/settings.json".to_string())
+        );
     }
 }