generic popup stacking solution (#1124)

* generic popup stacking solution
* allow going back to file-revision popup
* do not select diff in coming back to files-revlog
* handle filetree popup via stacking
* allow going back to inspect commit
* allow coming back to compare/inspect commit

filetreelist/src/filetree.rs

@@ -53,6 +53,11 @@ impl FileTree {
 		self.items.file_count() == 0
 	}
 
+	///
+	pub const fn selection(&self) -> Option<usize> {
+		self.selection
+	}
+
 	///
 	pub fn collapse_but_root(&mut self) {
 		if !self.is_empty() {

src/app.rs

@@ -15,7 +15,10 @@ use crate::{
 	},
 	input::{Input, InputEvent, InputState},
 	keys::{KeyConfig, SharedKeyConfig},
-	queue::{Action, InternalEvent, NeedsUpdate, Queue},
+	popup_stack::PopupStack,
+	queue::{
+		Action, InternalEvent, NeedsUpdate, Queue, StackablePopupOpen,
+	},
 	setup_popups,
 	strings::{self, order},
 	tabs::{FilesTab, Revlog, StashList, Stashing, Status},
@@ -79,6 +82,7 @@ pub struct App {
 	theme: SharedTheme,
 	key_config: SharedKeyConfig,
 	input: Input,
+	popup_stack: PopupStack,
 
 	// "Flags"
 	requires_redraw: Cell<bool>,
@@ -284,6 +288,7 @@ impl App {
 			requires_redraw: Cell::new(false),
 			file_to_open: None,
 			repo,
+			popup_stack: PopupStack::default(),
 		}
 	}
 
@@ -666,6 +671,31 @@ impl App {
 		Ok(())
 	}
 
+	fn open_popup(
+		&mut self,
+		popup: StackablePopupOpen,
+	) -> Result<()> {
+		match popup {
+			StackablePopupOpen::BlameFile(params) => {
+				self.blame_file_popup.open(params)?;
+			}
+			StackablePopupOpen::FileRevlog(param) => {
+				self.file_revlog_popup.open(param)?;
+			}
+			StackablePopupOpen::FileTree(param) => {
+				self.revision_files_popup.open(param)?;
+			}
+			StackablePopupOpen::InspectCommit(param) => {
+				self.inspect_commit_popup.open(param)?;
+			}
+			StackablePopupOpen::CompareCommits(param) => {
+				self.compare_commits_popup.open(param)?;
+			}
+		}
+
+		Ok(())
+	}
+
 	fn process_internal_events(&mut self) -> Result<NeedsUpdate> {
 		let mut flags = NeedsUpdate::empty();
 
@@ -715,16 +745,7 @@ impl App {
 			InternalEvent::TagCommit(id) => {
 				self.tag_commit_popup.open(id)?;
 			}
-			InternalEvent::BlameFile(path, commit_id) => {
-				self.blame_file_popup.open(&path, commit_id)?;
-				flags
-					.insert(NeedsUpdate::ALL | NeedsUpdate::COMMANDS);
-			}
-			InternalEvent::OpenFileRevlog(path) => {
-				self.file_revlog_popup.open(&path)?;
-				flags
-					.insert(NeedsUpdate::ALL | NeedsUpdate::COMMANDS);
-			}
+
 			InternalEvent::CreateBranch => {
 				self.create_branch_popup.open()?;
 			}
@@ -739,11 +760,6 @@ impl App {
 				self.tags_popup.open()?;
 			}
 			InternalEvent::TabSwitchStatus => self.set_tab(0)?,
-			InternalEvent::InspectCommit(id, tags) => {
-				self.inspect_commit_popup.open(id, tags)?;
-				flags
-					.insert(NeedsUpdate::ALL | NeedsUpdate::COMMANDS);
-			}
 			InternalEvent::SelectCommitInRevlog(id) => {
 				if let Err(error) = self.revlog.select_commit(id) {
 					self.queue.push(InternalEvent::ShowErrorMsg(
@@ -788,11 +804,6 @@ impl App {
 			InternalEvent::StatusLastFileMoved => {
 				self.status_tab.last_file_moved()?;
 			}
-			InternalEvent::OpenFileTree(c) => {
-				self.revision_files_popup.open(c)?;
-				flags
-					.insert(NeedsUpdate::ALL | NeedsUpdate::COMMANDS);
-			}
 			InternalEvent::OpenFileFinder(files) => {
 				self.find_file_popup.open(&files)?;
 				flags
@@ -812,17 +823,30 @@ impl App {
 
 				flags.insert(NeedsUpdate::ALL);
 			}
-			InternalEvent::CompareCommits(id, other) => {
-				self.compare_commits_popup.open(id, other)?;
-				flags
-					.insert(NeedsUpdate::ALL | NeedsUpdate::COMMANDS);
-			}
 			InternalEvent::FileFinderChanged(file) => {
 				self.files_tab.file_finder_update(&file);
 				self.revision_files_popup.file_finder_update(&file);
 				flags
 					.insert(NeedsUpdate::ALL | NeedsUpdate::COMMANDS);
 			}
+			InternalEvent::OpenPopup(popup) => {
+				self.open_popup(popup)?;
+				flags
+					.insert(NeedsUpdate::ALL | NeedsUpdate::COMMANDS);
+			}
+			InternalEvent::PopupStackPop => {
+				if let Some(popup) = self.popup_stack.pop() {
+					self.open_popup(popup)?;
+					flags.insert(
+						NeedsUpdate::ALL | NeedsUpdate::COMMANDS,
+					);
+				}
+			}
+			InternalEvent::PopupStackPush(popup) => {
+				self.popup_stack.push(popup);
+				flags
+					.insert(NeedsUpdate::ALL | NeedsUpdate::COMMANDS);
+			}
 		};
 
 		Ok(flags)

src/components/blame_file.rs

@@ -1,11 +1,12 @@
 use super::{
 	utils, visibility_blocking, CommandBlocking, CommandInfo,
-	Component, DrawableComponent, EventState,
+	Component, DrawableComponent, EventState, FileRevOpen,
+	InspectCommitOpen,
 };
 use crate::{
 	components::{utils::string_width_align, ScrollType},
 	keys::SharedKeyConfig,
-	queue::{InternalEvent, Queue},
+	queue::{InternalEvent, Queue, StackablePopupOpen},
 	string_utils::tabs_to_spaces,
 	strings,
 	ui::{self, style::SharedTheme},
@@ -27,41 +28,31 @@ use tui::{
 	Frame,
 };
 
+static NO_COMMIT_ID: &str = "0000000";
+static NO_AUTHOR: &str = "<no author>";
+static MIN_AUTHOR_WIDTH: usize = 3;
+static MAX_AUTHOR_WIDTH: usize = 20;
+
+#[derive(Clone, Debug)]
+pub struct BlameFileOpen {
+	pub file_path: String,
+	pub commit_id: Option<CommitId>,
+	pub selection: Option<usize>,
+}
+
 pub struct BlameFileComponent {
 	title: String,
 	theme: SharedTheme,
 	queue: Queue,
 	async_blame: AsyncBlame,
 	visible: bool,
+	open_request: Option<BlameFileOpen>,
 	params: Option<BlameParams>,
 	file_blame: Option<FileBlame>,
 	table_state: std::cell::Cell<TableState>,
 	key_config: SharedKeyConfig,
 	current_height: std::cell::Cell<usize>,
 }
-
-static NO_COMMIT_ID: &str = "0000000";
-static NO_AUTHOR: &str = "<no author>";
-static MIN_AUTHOR_WIDTH: usize = 3;
-static MAX_AUTHOR_WIDTH: usize = 20;
-
-fn get_author_width(width: usize) -> usize {
-	(width.saturating_sub(19) / 3)
-		.clamp(MIN_AUTHOR_WIDTH, MAX_AUTHOR_WIDTH)
-}
-
-const fn number_of_digits(number: usize) -> usize {
-	let mut rest = number;
-	let mut result = 0;
-
-	while rest > 0 {
-		rest /= 10;
-		result += 1;
-	}
-
-	result
-}
-
 impl DrawableComponent for BlameFileComponent {
 	fn draw<B: Backend>(
 		&self,
@@ -197,7 +188,7 @@ impl Component for BlameFileComponent {
 		if self.is_visible() {
 			if let Event::Key(key) = event {
 				if key == self.key_config.keys.exit_popup {
-					self.hide();
+					self.hide_stacked(false);
 				} else if key == self.key_config.keys.move_up {
 					self.move_selection(ScrollType::Up);
 				} else if key == self.key_config.keys.move_down {
@@ -215,22 +206,26 @@ impl Component for BlameFileComponent {
 				} else if key == self.key_config.keys.page_up {
 					self.move_selection(ScrollType::PageUp);
 				} else if key == self.key_config.keys.focus_right {
-					if let Some(id) = self.selected_commit() {
-						self.hide();
-						self.queue.push(
-							InternalEvent::InspectCommit(id, None),
-						);
+					if let Some(commit_id) = self.selected_commit() {
+						self.hide_stacked(true);
+						self.queue.push(InternalEvent::OpenPopup(
+							StackablePopupOpen::InspectCommit(
+								InspectCommitOpen::new(commit_id),
+							),
+						));
 					}
 				} else if key == self.key_config.keys.file_history {
 					if let Some(filepath) = self
 						.params
 						.as_ref()
 						.map(|p| p.file_path.clone())
 					{
-						self.hide();
-						self.queue.push(
-							InternalEvent::OpenFileRevlog(filepath),
-						);
+						self.hide_stacked(true);
+						self.queue.push(InternalEvent::OpenPopup(
+							StackablePopupOpen::FileRevlog(
+								FileRevOpen::new(filepath),
+							),
+						));
 					}
 				}
 
@@ -245,10 +240,6 @@ impl Component for BlameFileComponent {
 		self.visible
 	}
 
-	fn hide(&mut self) {
-		self.visible = false;
-	}
-
 	fn show(&mut self) -> Result<()> {
 		self.visible = true;
 
@@ -277,25 +268,40 @@ impl BlameFileComponent {
 			visible: false,
 			params: None,
 			file_blame: None,
+			open_request: None,
 			table_state: std::cell::Cell::new(TableState::default()),
 			key_config,
 			current_height: std::cell::Cell::new(0),
 		}
 	}
 
+	fn hide_stacked(&mut self, stack: bool) {
+		self.visible = false;
+		if stack {
+			if let Some(request) = self.open_request.clone() {
+				self.queue.push(InternalEvent::PopupStackPush(
+					StackablePopupOpen::BlameFile(BlameFileOpen {
+						file_path: request.file_path,
+						commit_id: request.commit_id,
+						selection: self.get_selection(),
+					}),
+				));
+			}
+		} else {
+			self.queue.push(InternalEvent::PopupStackPop);
+		}
+	}
+
 	///
-	pub fn open(
-		&mut self,
-		file_path: &str,
-		commit_id: Option<CommitId>,
-	) -> Result<()> {
+	pub fn open(&mut self, open: BlameFileOpen) -> Result<()> {
+		self.open_request = Some(open.clone());
 		self.params = Some(BlameParams {
-			file_path: file_path.into(),
-			commit_id,
+			file_path: open.file_path,
+			commit_id: open.commit_id,
 		});
 		self.file_blame = None;
 		self.table_state.get_mut().select(Some(0));
-		self.show()?;
+		self.visible = true;
 
 		self.update()?;
 
@@ -329,6 +335,7 @@ impl BlameFileComponent {
 				{
 					if previous_blame_params == *params {
 						self.file_blame = Some(last_file_blame);
+						self.set_open_selection();
 
 						return Ok(());
 					}
@@ -526,6 +533,28 @@ impl BlameFileComponent {
 		needs_update
 	}
 
+	fn set_open_selection(&mut self) {
+		if let Some(selection) =
+			self.open_request.as_ref().and_then(|req| req.selection)
+		{
+			let mut table_state = self.table_state.take();
+			table_state.select(Some(selection));
+			self.table_state.set(table_state);
+		}
+	}
+
+	fn get_selection(&self) -> Option<usize> {
+		self.file_blame.as_ref().and_then(|_| {
+			let table_state = self.table_state.take();
+
+			let selection = table_state.selected();
+
+			self.table_state.set(table_state);
+
+			selection
+		})
+	}
+
 	fn selected_commit(&self) -> Option<CommitId> {
 		self.file_blame.as_ref().and_then(|file_blame| {
 			let table_state = self.table_state.take();
@@ -544,3 +573,20 @@ impl BlameFileComponent {
 		})
 	}
 }
+
+fn get_author_width(width: usize) -> usize {
+	(width.saturating_sub(19) / 3)
+		.clamp(MIN_AUTHOR_WIDTH, MAX_AUTHOR_WIDTH)
+}
+
+const fn number_of_digits(number: usize) -> usize {
+	let mut rest = number;
+	let mut result = 0;
+
+	while rest > 0 {
+		rest /= 10;
+		result += 1;
+	}
+
+	result
+}