Fix 663 msg scroll (#669)

* fix line skipping and added unittest

Author: Stephan Dilly

Diff for: asyncgit/src/sync/commit_details.rs

@@ -4,7 +4,7 @@ use git2::Signature;
 use scopetime::scope_time;
 
 ///
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Default, Clone)]
 pub struct CommitSignature {
     ///
     pub name: String,
@@ -27,6 +27,7 @@ impl CommitSignature {
 }
 
 ///
+#[derive(Default, Clone)]
 pub struct CommitMessage {
     /// first line
     pub subject: String,
@@ -67,6 +68,7 @@ impl CommitMessage {
 }
 
 ///
+#[derive(Default, Clone)]
 pub struct CommitDetails {
     ///
     pub author: CommitSignature,

Diff for: asyncgit/src/sync/mod.rs

@@ -36,7 +36,7 @@ pub use branch::{
 };
 pub use commit::{amend, commit, tag};
 pub use commit_details::{
-    get_commit_details, CommitDetails, CommitMessage,
+    get_commit_details, CommitDetails, CommitMessage, CommitSignature,
 };
 pub use commit_files::get_commit_files;
 pub use commits_info::{

Diff for: src/components/commit_details/details.rs

@@ -39,6 +39,7 @@ pub struct DetailsComponent {
     current_size: Cell<(u16, u16)>,
     scroll_top: Cell<usize>,
     key_config: SharedKeyConfig,
+    scroll_to_bottom_on_redraw: Cell<bool>,
 }
 
 type WrappedCommitMessage<'a> =
@@ -58,6 +59,7 @@ impl DetailsComponent {
             focused,
             current_size: Cell::new((0, 0)),
             scroll_top: Cell::new(0),
+            scroll_to_bottom_on_redraw: Cell::new(false),
             key_config,
         }
     }
@@ -89,10 +91,7 @@ impl DetailsComponent {
 
         if let Some(ref body) = message.body {
             let wrapped_message: Vec<Cow<'_, str>> =
-                textwrap::wrap(body, width)
-                    .into_iter()
-                    .skip(1)
-                    .collect();
+                textwrap::wrap(body, width).into_iter().collect();
 
             (wrapped_title, wrapped_message)
         } else {
@@ -101,10 +100,10 @@ impl DetailsComponent {
     }
 
     fn get_wrapped_lines(
-        &self,
+        data: &Option<CommitDetails>,
         width: usize,
     ) -> WrappedCommitMessage<'_> {
-        if let Some(ref data) = self.data {
+        if let Some(ref data) = data {
             if let Some(ref message) = data.message {
                 return Self::wrap_commit_details(message, width);
             }
@@ -113,9 +112,12 @@ impl DetailsComponent {
         (vec![], vec![])
     }
 
-    fn get_number_of_lines(&self, width: usize) -> usize {
+    fn get_number_of_lines(
+        details: &Option<CommitDetails>,
+        width: usize,
+    ) -> usize {
         let (wrapped_title, wrapped_message) =
-            self.get_wrapped_lines(width);
+            Self::get_wrapped_lines(details, width);
 
         wrapped_title.len() + wrapped_message.len()
     }
@@ -134,7 +136,7 @@ impl DetailsComponent {
         height: usize,
     ) -> Vec<Spans> {
         let (wrapped_title, wrapped_message) =
-            self.get_wrapped_lines(width);
+            Self::get_wrapped_lines(&self.data, width);
 
         [&wrapped_title[..], &wrapped_message[..]]
             .concat()
@@ -278,7 +280,8 @@ impl DetailsComponent {
             let width = self.current_size.get().0 as usize;
             let height = self.current_size.get().1 as usize;
 
-            let number_of_lines = self.get_number_of_lines(width);
+            let number_of_lines =
+                Self::get_number_of_lines(&self.data, width);
 
             let max = number_of_lines.saturating_sub(height) as usize;
 
@@ -290,7 +293,9 @@ impl DetailsComponent {
                 _ => old,
             };
 
-            if new_scroll_top > max {
+            let new_scroll_top = new_scroll_top.clamp(0, max);
+
+            if new_scroll_top == old {
                 return false;
             }
 
@@ -336,6 +341,17 @@ impl DrawableComponent for DetailsComponent {
 
         self.current_size.set((width, height));
 
+        if self.scroll_to_bottom_on_redraw.get() {
+            self.scroll_top.set(
+                Self::get_number_of_lines(
+                    &self.data,
+                    usize::from(width),
+                )
+                .saturating_sub(usize::from(height)),
+            );
+            self.scroll_to_bottom_on_redraw.set(false);
+        }
+
         let wrapped_lines = self.get_wrapped_text_message(
             width as usize,
             height as usize,
@@ -358,7 +374,7 @@ impl DrawableComponent for DetailsComponent {
                 f,
                 chunks[1],
                 &self.theme,
-                self.get_number_of_lines(width as usize),
+                Self::get_number_of_lines(&self.data, width as usize),
                 self.scroll_top.get(),
             )
         }
@@ -376,7 +392,8 @@ impl Component for DetailsComponent {
         // visibility_blocking(self)
 
         let width = self.current_size.get().0 as usize;
-        let number_of_lines = self.get_number_of_lines(width);
+        let number_of_lines =
+            Self::get_number_of_lines(&self.data, width);
 
         out.push(
             CommandInfo::new(
@@ -422,13 +439,7 @@ impl Component for DetailsComponent {
 
     fn focus(&mut self, focus: bool) {
         if focus {
-            let width = self.current_size.get().0 as usize;
-            let height = self.current_size.get().1 as usize;
-
-            self.scroll_top.set(
-                self.get_number_of_lines(width)
-                    .saturating_sub(height),
-            );
+            self.scroll_to_bottom_on_redraw.set(true);
         }
 
         self.focused = focus;
@@ -475,7 +486,7 @@ mod tests {
         );
 
         let message_with_body = CommitMessage::from(
-            "Commit message\n\nFirst line\nSecond line",
+            "Commit message\nFirst line\nSecond line",
         );
 
         assert_eq!(
@@ -491,3 +502,28 @@ mod tests {
         );
     }
 }
+
+#[cfg(test)]
+mod test_line_count {
+    use super::*;
+
+    #[test]
+    fn test_smoke() {
+        let commit = CommitDetails {
+            message: Some(CommitMessage {
+                subject: String::from("subject line"),
+                body: Some(String::from("body lone")),
+            }),
+            ..CommitDetails::default()
+        };
+        let lines = DetailsComponent::get_number_of_lines(
+            &Some(commit.clone()),
+            50,
+        );
+        assert_eq!(lines, 2);
+
+        let lines =
+            DetailsComponent::get_number_of_lines(&Some(commit), 8);
+        assert_eq!(lines, 4);
+    }
+}