Merge pull request #169 from MitMaro/tim/show-commit-performance

 Improve performance of show commit

11 files changed, 231 insertions, 229 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8e475d6..957ae88 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Support for 256-color terminals
 - Highlight of selected line(s) on 256-color terminals
 
+### Fixed
+- Performance issue with show commit
+
 ### Removed
 - Unused `errorColor` configuration
 
diff --git a/scripts/format.bash b/scripts/format.bash
index aa4def6..b2accaf 100755
--- a/scripts/format.bash
+++ b/scripts/format.bash
@@ -4,6 +4,8 @@ set -e
 set -u
 set -o pipefail
 
-rustup update nightly
-rustup component add rustfmt --toolchain nightly
-cargo +nightly fmt --all -- --check
+rust_version="nightly-2019-09-13"
+
+rustup update "$rust_version"
+rustup component add rustfmt --toolchain "$rust_version"
+cargo +"$rust_version"  fmt --all -- --check
diff --git a/src/git_interactive.rs b/src/git_interactive.rs
index 60c59a2..87288fc 100644
--- a/src/git_interactive.rs
+++ b/src/git_interactive.rs
@@ -39,7 +39,6 @@ fn load_filepath(path: &PathBuf, comment_char: &str) -> Result<Vec<Line>, String
 pub struct GitInteractive {
 	filepath: PathBuf,
 	lines: Vec<Line>,
-	selected_commit_stats: Option<Commit>,
 	selected_line_index: usize,
 	visual_index_start: usize,
 }
@@ -52,7 +51,6 @@ impl GitInteractive {
 		Ok(GitInteractive {
 			filepath: path,
 			lines,
-			selected_commit_stats: None,
 			selected_line_index: 1,
 			visual_index_start: 1,
 		})
@@ -211,12 +209,10 @@ impl GitInteractive {
 		}
 	}
 
-	// TODO this is kind of clunky and might be replaceable with a RefCell
-	pub fn load_commit_stats(&mut self) -> Result<(), String> {
+	pub fn load_commit_stats(&self) -> Result<Commit, String> {
 		let selected_action = self.lines[self.selected_line_index - 1].get_action();
 		if *selected_action != Action::Exec && *selected_action != Action::Break {
-			self.selected_commit_stats = Some(Commit::from_commit_hash(self.get_selected_line_hash().as_str())?);
-			return Ok(());
+			return Ok(Commit::from_commit_hash(self.get_selected_line_hash().as_str())?);
 		}
 		Err(String::from("Cannot load commit for the selected action"))
 	}
@@ -225,10 +221,6 @@ impl GitInteractive {
 		!self.lines.is_empty() && *self.lines[0].get_action() == Action::Noop
 	}
 
-	pub fn get_commit_stats(&self) -> &Option<Commit> {
-		&self.selected_commit_stats
-	}
-
 	pub fn get_selected_line_hash(&self) -> &String {
 		self.lines[self.selected_line_index - 1].get_hash()
 	}
diff --git a/src/help/help.rs b/src/help/help.rs
index 7934d57..9d42287 100644
--- a/src/help/help.rs
+++ b/src/help/help.rs
@@ -93,7 +93,7 @@ impl<'h> ProcessModule for Help<'h> {
 		}
 
 		view.draw_view_lines(
-			view_lines,
+			&view_lines,
 			self.scroll_position.get_top_position(),
 			self.scroll_position.get_left_position(),
 			view_height - 3,
diff --git a/src/list/list.rs b/src/list/list.rs
index 869eada..a81b9d4 100644
--- a/src/list/list.rs
+++ b/src/list/list.rs
@@ -116,7 +116,7 @@ impl<'l> ProcessModule for List<'l> {
 		view.draw_title(true);
 
 		view.draw_view_lines(
-			view_lines,
+			&view_lines,
 			self.scroll_position.get_top_position(),
 			self.scroll_position.get_left_position(),
 			view_height - 2,
diff --git a/src/show_commit/data.rs b/src/show_commit/data.rs
new file mode 100644
index 0000000..fa16e7d
--- /dev/null
+++ b/src/show_commit/data.rs
@@ -0,0 +1,150 @@
+use crate::commit::Commit;
+use crate::constants::MINIMUM_FULL_WINDOW_WIDTH;
+use crate::display::DisplayColor;
+use crate::show_commit::util::get_stat_item_segments;
+use crate::view::{LineSegment, ViewLine};
+use std::cmp;
+use unicode_segmentation::UnicodeSegmentation;
+
+pub struct Data {
+	height: usize,
+	width: usize,
+	lines: Vec<ViewLine>,
+	line_lengths: Vec<usize>,
+	max_line_length: usize,
+}
+
+impl Data {
+	pub fn new() -> Self {
+		Self {
+			height: 0,
+			width: 0,
+			lines: Vec::new(),
+			line_lengths: Vec::new(),
+			max_line_length: 0,
+		}
+	}
+
+	pub fn reset(&mut self) {
+		self.height = 0;
+		self.width = 0;
+		self.lines.clear();
+		self.line_lengths.clear();
+		self.max_line_length = 0;
+	}
+
+	pub fn update(&mut self, commit: &Commit, window_width: usize, window_height: usize) {
+		if window_width != self.width || window_height != self.height {
+			self.reset();
+
+			self.height = window_height;
+			self.width = window_width;
+
+			let is_full_width = window_width >= MINIMUM_FULL_WINDOW_WIDTH;
+
+			let full_hash = commit.get_hash();
+			let author = commit.get_author();
+			let committer = commit.get_committer();
+			let date = commit.get_date();
+			let body = commit.get_body();
+			let file_stats = commit.get_file_stats();
+
+			let hash_line = if is_full_width {
+				format!("Commit: {}", full_hash)
+			}
+			else {
+				let max_index = cmp::min(full_hash.len(), 8);
+				format!("{:8} ", full_hash[0..max_index].to_string())
+			};
+
+			self.lines.push(ViewLine::new(vec![LineSegment::new_with_color(
+				hash_line.as_str(),
+				DisplayColor::IndicatorColor,
+			)]));
+			self.line_lengths.push(hash_line.len());
+
+			let date_line = if is_full_width {
+				format!("Date: {}", date.format("%c %z"))
+			}
+			else {
+				format!("{}", date.format("%c %z"))
+			};
+
+			self.lines
+				.push(ViewLine::new(vec![LineSegment::new(date_line.as_str())]));
+			self.line_lengths.push(date_line.len());
+
+			if let Some(a) = author.to_string() {
+				let author_line = if is_full_width {
+					format!("Author: {}", a)
+				}
+				else {
+					format!("A: {}", a)
+				};
+				self.lines
+					.push(ViewLine::new(vec![LineSegment::new(author_line.as_str())]));
+				self.line_lengths
+					.push(UnicodeSegmentation::graphemes(author_line.as_str(), true).count());
+			}
+
+			if let Some(c) = committer.to_string() {
+				let committer_line = if is_full_width {
+					format!("Committer: {}", c)
+				}
+				else {
+					format!("C: {}", c)
+				};
+				self.lines
+					.push(ViewLine::new(vec![LineSegment::new(committer_line.as_str())]));
+				self.line_lengths
+					.push(UnicodeSegmentation::graphemes(committer_line.as_str(), true).count());
+			}
+
+			match body {
+				Some(b) => {
+					for line in b.lines() {
+						self.lines.push(ViewLine::new(vec![LineSegment::new(line)]));
+						self.line_lengths
+							.push(UnicodeSegmentation::graphemes(line, true).count());
+					}
+				},
+				None => {},
+			}
+
+			self.lines.push(ViewLine::new(vec![LineSegment::new("")]));
+			self.line_lengths.push(0);
+
+			match file_stats {
+				Some(stats) => {
+					for stat in stats {
+						let stat_to_name = stat.get_to_name();
+						let stat_from_name = stat.get_from_name();
+						let stat_view_line = ViewLine::new(get_stat_item_segments(
+							*stat.get_status(),
+							stat_to_name.as_str(),
+							stat_from_name.as_str(),
+							is_full_width,
+						));
+						self.line_lengths.push(stat_view_line.get_length());
+						self.lines.push(stat_view_line);
+					}
+				},
+				None => {},
+			}
+		}
+	}
+
+	pub fn get_lines(&self) -> &Vec<ViewLine> {
+		&self.lines
+	}
+
+	pub fn get_max_line_length(&self, start: usize, end: usize) -> usize {
+		let mut max_length = 0;
+		for len in self.line_lengths[start..=end].iter() {
+			if *len > max_length {
+				max_length = *len;
+			}
+		}
+		max_length
+	}
+}
diff --git a/src/show_commit/mod.rs b/src/show_commit/mod.rs
index 9294506..ba21d8f 100644
--- a/src/show_commit/mod.rs
+++ b/src/show_commit/mod.rs
@@ -1,3 +1,4 @@
+mod data;
 #[allow(clippy::module_inception)]
 mod show_commit;
 mod util;
diff --git a/src/show_commit/show_commit.rs b/src/show_commit/show_commit.rs
index 561e967..810053e 100644
--- a/src/show_commit/show_commit.rs
+++ b/src/show_commit/show_commit.rs
@@ -1,5 +1,4 @@
 use crate::commit::Commit;
-use crate::constants::MINIMUM_FULL_WINDOW_WIDTH;
 use crate::display::DisplayColor;
 use crate::git_interactive::GitInteractive;
 use crate::input::{Input, InputHandler};
@@ -12,32 +11,46 @@ use crate::process::{
 	State,
 };
 use crate::scroll::ScrollPosition;
-use crate::show_commit::util::get_stat_item_segments;
-use crate::view::{LineSegment, View, ViewLine};
-use std::cmp;
-use unicode_segmentation::UnicodeSegmentation;
+use crate::show_commit::data::Data;
+use crate::view::View;
 
 pub struct ShowCommit {
+	commit: Option<Result<Commit, String>>,
+	data: Data,
 	scroll_position: ScrollPosition,
 }
 
 impl ProcessModule for ShowCommit {
-	fn activate(&mut self, _state: State, _git_interactive: &GitInteractive) {
+	fn activate(&mut self, _state: State, git_interactive: &GitInteractive) {
 		self.scroll_position.reset();
+		self.commit = Some(git_interactive.load_commit_stats());
 	}
 
-	fn process(&mut self, git_interactive: &mut GitInteractive, _view: &View) -> ProcessResult {
+	fn deactivate(&mut self) {
+		self.data.reset();
+	}
+
+	fn process(&mut self, _git_interactive: &mut GitInteractive, view: &View) -> ProcessResult {
+		let (view_width, view_height) = view.get_view_size();
 		let mut result = ProcessResultBuilder::new();
-		if let Err(e) = git_interactive.load_commit_stats() {
-			result = result.error(e.as_str(), State::List(false));
+
+		if let Some(commit) = &self.commit {
+			match commit {
+				Ok(c) => self.data.update(&c, view_width, view_height),
+				Err(e) => {
+					result = result.error(e.as_str(), State::List(false));
+					self.data.reset()
+				},
+			}
 		}
+
 		result.build()
 	}
 
 	fn handle_input(
 		&mut self,
 		input_handler: &InputHandler,
-		git_interactive: &mut GitInteractive,
+		_git_interactive: &mut GitInteractive,
 		view: &View,
 	) -> HandleInputResult
 	{
@@ -47,40 +60,24 @@ impl ProcessModule for ShowCommit {
 		let mut result = HandleInputResultBuilder::new(input);
 		match input {
 			Input::MoveCursorLeft => {
-				self.scroll_position.scroll_left(
-					view_width,
-					self.get_max_line_length(
-						git_interactive.get_commit_stats(),
-						view_height >= MINIMUM_FULL_WINDOW_WIDTH,
-					),
-				)
+				self.scroll_position
+					.scroll_left(view_width, self.get_max_line_length(view_height))
 			},
 			Input::MoveCursorRight => {
-				self.scroll_position.scroll_right(
-					view_width,
-					self.get_max_line_length(
-						git_interactive.get_commit_stats(),
-						view_height >= MINIMUM_FULL_WINDOW_WIDTH,
-					),
-				)
+				self.scroll_position
+					.scroll_right(view_width, self.get_max_line_length(view_height))
 			},
 			Input::MoveCursorDown => {
-				self.scroll_position.scroll_down(
-					view_height,
-					self.get_commit_stats_length(git_interactive.get_commit_stats()),
-				)
+				self.scroll_position
+					.scroll_down(view_height, self.get_commit_stats_length())
 			},
 			Input::MoveCursorUp => {
-				self.scroll_position.scroll_up(
-					view_height,
-					self.get_commit_stats_length(git_interactive.get_commit_stats()),
-				)
+				self.scroll_position
+					.scroll_up(view_height, self.get_commit_stats_length())
 			},
 			Input::Resize => {
-				self.scroll_position.scroll_up(
-					view_height as usize,
-					self.get_commit_stats_length(git_interactive.get_commit_stats()),
-				);
+				self.scroll_position
+					.scroll_up(view_height as usize, self.get_commit_stats_length());
 			},
 			_ => {
 				result = result.state(State::List(false));
@@ -89,105 +86,22 @@ impl ProcessModule for ShowCommit {
 		result.build()
 	}
 
-	fn render(&self, view: &View, git_interactive: &GitInteractive) {
-		let commit_data = git_interactive.get_commit_stats();
-		let (window_width, window_height) = view.get_view_size();
+	fn render(&self, view: &View, _git_interactive: &GitInteractive) {
+		let (_, window_height) = view.get_view_size();
 		let view_height = window_height - 2;
 
-		let is_full_width = window_width >= MINIMUM_FULL_WINDOW_WIDTH;
-
 		view.draw_title(false);
 
-		let commit = match commit_data {
+		match &self.commit {
 			None => {
 				view.draw_error("Not commit data to show");
 				return;
 			},
-			Some(c) => c,
-		};
-
-		let full_hash = commit.get_hash();
-		let author = commit.get_author();
-		let committer = commit.get_committer();
-		let date = commit.get_date();
-		let body = commit.get_body();
-		let file_stats = commit.get_file_stats();
-
-		let mut lines: Vec<ViewLine> = vec![];
-
-		lines.push(ViewLine::new(vec![LineSegment::new_with_color(
-			if is_full_width {
-				format!("Commit: {}", full_hash)
-			}
-			else {
-				let max_index = cmp::min(full_hash.len(), 8);
-				format!("{:8} ", full_hash[0..max_index].to_string())
-			}
-			.as_str(),
-			DisplayColor::IndicatorColor,
-		)]));
-
-		lines.push(ViewLine::new(vec![LineSegment::new(
-			if is_full_width {
-				format!("Date: {}", date.format("%c %z"))
-			}
-			else {
-				format!("{}", date.format("%c %z"))
-			}
-			.as_str(),
-		)]));
-
-		if let Some(a) = author.to_string() {
-			lines.push(ViewLine::new(vec![LineSegment::new(
-				if is_full_width {
-					format!("Author: {}", a)
-				}
-				else {
-					format!("A: {}", a)
-				}
-				.as_str(),
-			)]));
-		}
-
-		if let Some(c) = committer.to_string() {
-			lines.push(ViewLine::new(vec![LineSegment::new(
-				if is_full_width {
-					format!("Committer: {}", c)
-				}
-				else {
-					format!("C: {}", c)
-				}
-				.as_str(),
-			)]))
+			Some(c) => c.as_ref().unwrap(), // safe unwrap
 		};
 
-		match body {
-			Some(b) => {
-				for line in b.lines() {
-					lines.push(ViewLine::new(vec![LineSegment::new(line)]));
-				}
-			},
-			None => {},
-		};
-
-		lines.push(ViewLine::new(vec![LineSegment::new("")]));
-
-		match file_stats {
-			Some(stats) => {
-				for stat in stats {
-					lines.push(ViewLine::new(get_stat_item_segments(
-						*stat.get_status(),
-						stat.get_to_name().as_str(),
-						stat.get_from_name().as_str(),
-						is_full_width,
-					)))
-				}
-			},
-			None => {},
-		}
-
 		view.draw_view_lines(
-			lines,
+			self.data.get_lines(),
 			self.scroll_position.get_top_position(),
 			self.scroll_position.get_left_position(),
 			view_height,
@@ -201,13 +115,15 @@ impl ProcessModule for ShowCommit {
 impl ShowCommit {
 	pub fn new() -> Self {
 		Self {
+			commit: None,
+			data: Data::new(),
 			scroll_position: ScrollPosition::new(3, 6, 3),
 		}
 	}
 
-	fn get_commit_stats_length(&self, commit: &Option<Commit>) -> usize {
-		match commit {
-			Some(c) => {
+	fn get_commit_stats_length(&self) -> usize {
+		if let Some(commit) = &self.commit {
+			if let Ok(c) = commit {
 				let mut len = c.get_file_stats_length();
 
 				match c.get_body() {
@@ -216,94 +132,16 @@ impl ShowCommit {
 					},
 					None => {},
 				}
-				len + 3 // author + date + commit hash
-			},
-			None => 0,
+				return len + 3; // author + date + commit hash
+			}
 		}
+		0
 	}
 
-	fn get_max_line_length(&self, commit: &Option<Commit>, is_full_width: bool) -> usize {
-		match commit {
-			Some(c) => {
-				let full_hash = c.get_hash();
-				let author = c.get_author();
-				let committer = c.get_committer();
-				let body = c.get_body();
-				let file_stats = c.get_file_stats();
-
-				let mut max_line_length = if is_full_width {
-					full_hash.len() + 8 // 8 = "Commit: "
-				}
-				else {
-					cmp::min(full_hash.len(), 8)
-				};
-
-				max_line_length = cmp::max(
-					if is_full_width {
-						35 // "Date: Sun Jul 8 00:34:60 2001+09:30"
-					}
-					else {
-						29 // "Sun Jul 8 00:34:60 2001+09:30"
-					},
-					max_line_length,
-				);
-
-				if let Some(a) = author.to_string() {
-					max_line_length = cmp::max(
-						if is_full_width {
-							UnicodeSegmentation::graphemes(a.as_str(), true).count() + 8 // 8 = "Author: "
-						}
-						else {
-							UnicodeSegmentation::graphemes(a.as_str(), true).count() + 3 // 3 = "A: "
-						},
-						max_line_length,
-					);
-				}
-
-				if let Some(c) = committer.to_string() {
-					max_line_length = cmp::max(
-						if is_full_width {
-							UnicodeSegmentation::graphemes(c.as_str(), true).count() + 11 // 11 = "Committer: "
-						}
-						else {
-							UnicodeSegmentation::graphemes(c.as_str(), true).count() + 3 // 3 = "C: "
-						},
-						max_line_length,
-					);
-				};
-
-				if let Some(b) = body {
-					for line in b.lines() {
-						let line_length = UnicodeSegmentation::graphemes(line, true).count();
-						if line_length > max_line_length {
-							max_line_length = line_length;
-						}
-					}
-				}
-
-				if let Some(stats) = file_stats {
-					let additional_line_length = if is_full_width {
-						13 // stat name + arrow
-					}
-					else {
-						3 // stat name + arrow
-					};
-
-					for stat in stats {
-						let stat_line_length =
-							UnicodeSegmentation::graphemes(stat.get_to_name().as_str(), true).count()
-								+ UnicodeSegmentation::graphemes(stat.get_from_name().as_str(), true).count()
-								+ additional_line_length;
-
-						if stat_line_length > max_line_length {
-							max_line_length = stat_line_length;
-						}
-					}
-				}
-
-				max_line_length
-			},
-			None => 0,
-		}
+	fn get_max_line_length(&self, view_height: usize) -> usize {
+		self.data.get_max_line_length(
+			self.scroll_position.get_top_position(),
+			self.scroll_position.get_top_position() + view_height,
+		)
 	}
 }
diff --git a/src/view/line_segment.rs b/src/view/line_segment.rs
index b007204..320407e 100644
--- a/src/view/line_segment.rs
+++ b/src/view/line_segment.rs
@@ -6,6 +6,7 @@ pub struct LineSegment {
 	dim: bool,
 	reverse: bool,
 	text: String,
+	length: usize,
 	underline: bool,
 }
 
@@ -16,6 +17,7 @@ impl LineSegment {
 			color: DisplayColor::Normal,
 			reverse: false,
 			dim: false,
+			length: UnicodeSegmentation::graphemes(text, true).count(),
 			underline: false,
 		}
 	}
@@ -26,6 +28,7 @@ impl LineSegment {
 			color,
 			reverse: false,
 			dim: false,
+			length: UnicodeSegmentation::graphemes(text, true).count(),
 			underline: false,
 		}
 	}
@@ -43,10 +46,15 @@ impl LineSegment {
 			color,
 			reverse,
 			dim,
+			length: UnicodeSegmentation::graphemes(text, true).count(),
 			underline,
 		}
 	}
 
+	pub fn get_length(&self) -> usize {
+		self.length
+	}
+
 	pub fn draw(&self, left: usize, max_width: usize, selected: bool, display: &Display) -> (usize, usize) {
 		display.color(self.color, selected);
 		display.set_style(self.dim, self.underline, self.reverse);
diff --git a/src/view/view.rs b/src/view/view.rs
index f6fdb18..73eb264 100644
--- a/src/view/view.rs
+++ b/src/view/view.rs
@@ -62,7 +62,7 @@ impl<'v> View<'v> {
 		self.display.refresh();
 	}
 
-	pub fn draw_view_lines(&self, lines: Vec<ViewLine>, top: usize, left: usize, height: usize) {
+	pub fn draw_view_lines(&self, lines: &[ViewLine], top: usize, left: usize, height: usize) {
 		let number_of_lines = lines.len();
 
 		let scroll_indicator_index = get_scroll_position(top, number_of_lines, height);
diff --git a/src/view/view_line.rs b/src/view/view_line.rs
index 562c6f7..2e877af 100644
--- a/src/view/view_line.rs
+++ b/src/view/view_line.rs
@@ -23,6 +23,14 @@ impl ViewLine {
 		}
 	}
 
+	pub fn get_length(&self) -> usize {
+		let mut length = 0;
+		for s in self.segments.iter() {
+			length += s.get_length();
+		}
+		length
+	}
+
 	pub fn set_selected(mut self, selected: bool) -> Self {
 		self.selected = selected;
 		self