Merge pull request #50 from ClementTsang/fix_cursor_screen_bug

Fix cursor screen bug

.github/pull_request_template.md

@@ -14,6 +14,7 @@ _Remove the irrelevant one:_
 
 - [x] Bug fix (non-breaking change which fixes an issue)
 - [x] New feature (non-breaking change which adds functionality)
+- [x] Other (something else)
 
 ## Test methodology
 
@@ -24,7 +25,7 @@ _Please state how this was tested:_
 _Please ensure all are ticked (and actually done):_
 
 - [ ] Change has been tested to work
-- [ ] Code has been linted using rustfmt
+- [ ] Areas your change affects has been linted using rustfmt
 - [ ] Code has been self-reviewed
 - [ ] Code has been tested and no new breakage is introduced
 - [ ] Documentation has been added/updated if needed

Cargo.toml

@@ -30,7 +30,6 @@ heim = "0.0.10"
 log = "0.4.8"
 regex = "1.3.4"
 sysinfo = "0.11"
-winapi = "0.3.8"
 crossterm = "0.16"
 tui = {version = "0.8", features = ["crossterm"], default-features = false }
 lazy_static = "1.4.0"
@@ -40,6 +39,9 @@ serde = {version = "1.0", features = ["derive"] }
 unicode-segmentation = "1.6.0"
 unicode-width = "0.1.7"
 
+[target.'cfg(windows)'.dependencies]
+winapi = "0.3.8"
+
 [dev-dependencies]
 assert_cmd = "0.12"
 predicates = "1"

README.md

@@ -206,7 +206,7 @@ Note that `q` is disabled while in the search widget.
 
 ## Contribution
 
-Contribution is welcome! Just submit a PR.
+Contribution is welcome! Just submit a PR. Note that I develop and test on stable Rust.
 
 If you spot any issue with nobody assigned to it, or it seems like no work has started on it, feel free to try and do it!
 

rustfmt.toml

@@ -11,4 +11,3 @@ reorder_modules = true
 reorder_impl_items = true
 tab_spaces = 4
 format_strings = true
-space_after_colon = true

src/app.rs

@@ -8,8 +8,8 @@ use data_farmer::*;
 use crate::{canvas, constants, utils::error::Result};
 mod process_killer;
 
-use unicode_segmentation::{GraphemeCursor};
-use unicode_width::UnicodeWidthStr;
+use unicode_segmentation::GraphemeCursor;
+use unicode_width::{UnicodeWidthChar, UnicodeWidthStr};
 
 const MAX_SEARCH_LENGTH: usize = 200;
 
@@ -33,7 +33,7 @@ pub enum ScrollDirection {
 }
 
 #[derive(Debug)]
-pub enum SearchDirection {
+pub enum CursorDirection {
 	LEFT,
 	RIGHT,
 }
@@ -73,6 +73,10 @@ pub struct AppSearchState {
 	pub is_blank_search: bool,
 	pub is_invalid_search: bool,
 	pub grapheme_cursor: GraphemeCursor,
+	pub cursor_direction: CursorDirection,
+	pub cursor_bar: usize,
+	/// This represents the position in terms of CHARACTERS, not graphemes
+	pub char_cursor_position: usize,
 }
 
 impl Default for AppSearchState {
@@ -84,11 +88,21 @@ impl Default for AppSearchState {
 			is_invalid_search: false,
 			is_blank_search: true,
 			grapheme_cursor: GraphemeCursor::new(0, 0, true),
+			cursor_direction: CursorDirection::RIGHT,
+			cursor_bar: 0,
+			char_cursor_position: 0,
 		}
 	}
 }
 
 impl AppSearchState {
+	/// Returns a reset but still enabled app search state
+	pub fn reset() -> Self {
+		let mut app_search_state = AppSearchState::default();
+		app_search_state.is_enabled = true;
+		app_search_state
+	}
+
 	pub fn is_invalid_or_blank_search(&self) -> bool {
 		self.is_blank_search || self.is_invalid_search
 	}
@@ -549,6 +563,10 @@ impl App {
 			.cur_cursor()
 	}
 
+	pub fn get_char_cursor_position(&self) -> usize {
+		self.process_search_state.search_state.char_cursor_position
+	}
+
 	/// One of two functions allowed to run while in a dialog...
 	pub fn on_enter(&mut self) {
 		if self.delete_dialog_state.is_showing_dd {
@@ -620,19 +638,14 @@ impl App {
 	#[allow(unused_variables)]
 	pub fn skip_word_backspace(&mut self) {
 		if let WidgetPosition::ProcessSearch = self.current_widget_selected {
-			if self.process_search_state.search_state.is_enabled {
-			}
+			if self.process_search_state.search_state.is_enabled {}
 		}
 	}
 
 	pub fn clear_search(&mut self) {
 		if let WidgetPosition::ProcessSearch = self.current_widget_selected {
-			self.process_search_state.search_state.grapheme_cursor =
-				GraphemeCursor::new(0, 0, true);
-			self.process_search_state.search_state.current_search_query = String::default();
-			self.process_search_state.search_state.is_blank_search = true;
-			self.process_search_state.search_state.is_invalid_search = false;
 			self.update_process_gui = true;
+			self.process_search_state.search_state = AppSearchState::reset();
 		}
 	}
 
@@ -663,7 +676,8 @@ impl App {
 			if self.process_search_state.search_state.is_enabled && self.get_cursor_position() > 0 {
 				self.search_walk_back(self.get_cursor_position());
 
-				self.process_search_state
+				let removed_char = self
+					.process_search_state
 					.search_state
 					.current_search_query
 					.remove(self.get_cursor_position());
@@ -677,6 +691,10 @@ impl App {
 					true,
 				);
 
+				self.process_search_state.search_state.char_cursor_position -=
+					UnicodeWidthChar::width(removed_char).unwrap_or(0);
+				self.process_search_state.search_state.cursor_direction = CursorDirection::LEFT;
+
 				self.update_regex();
 				self.update_process_gui = true;
 			}
@@ -710,7 +728,15 @@ impl App {
 	pub fn on_left_key(&mut self) {
 		if !self.is_in_dialog() {
 			if let WidgetPosition::ProcessSearch = self.current_widget_selected {
+				let prev_cursor = self.get_cursor_position();
 				self.search_walk_back(self.get_cursor_position());
+				if self.get_cursor_position() < prev_cursor {
+					let str_slice = &self.process_search_state.search_state.current_search_query
+						[self.get_cursor_position()..prev_cursor];
+					self.process_search_state.search_state.char_cursor_position -=
+						UnicodeWidthStr::width(str_slice);
+					self.process_search_state.search_state.cursor_direction = CursorDirection::LEFT;
+				}
 			}
 		} else if self.delete_dialog_state.is_showing_dd && !self.delete_dialog_state.is_on_yes {
 			self.delete_dialog_state.is_on_yes = true;
@@ -720,7 +746,16 @@ impl App {
 	pub fn on_right_key(&mut self) {
 		if !self.is_in_dialog() {
 			if let WidgetPosition::ProcessSearch = self.current_widget_selected {
+				let prev_cursor = self.get_cursor_position();
 				self.search_walk_forward(self.get_cursor_position());
+				if self.get_cursor_position() > prev_cursor {
+					let str_slice = &self.process_search_state.search_state.current_search_query
+						[prev_cursor..self.get_cursor_position()];
+					self.process_search_state.search_state.char_cursor_position +=
+						UnicodeWidthStr::width(str_slice);
+					self.process_search_state.search_state.cursor_direction =
+						CursorDirection::RIGHT;
+				}
 			}
 		} else if self.delete_dialog_state.is_showing_dd && self.delete_dialog_state.is_on_yes {
 			self.delete_dialog_state.is_on_yes = false;
@@ -738,6 +773,8 @@ impl App {
 						.len(),
 					true,
 				);
+				self.process_search_state.search_state.char_cursor_position = 0;
+				self.process_search_state.search_state.cursor_direction = CursorDirection::LEFT;
 			}
 		}
 	}
@@ -756,6 +793,14 @@ impl App {
 						.len(),
 					true,
 				);
+				self.process_search_state.search_state.char_cursor_position =
+					UnicodeWidthStr::width(
+						self.process_search_state
+							.search_state
+							.current_search_query
+							.as_str(),
+					);
+				self.process_search_state.search_state.cursor_direction = CursorDirection::RIGHT;
 			}
 		}
 	}
@@ -840,6 +885,10 @@ impl App {
 						true,
 					);
 					self.search_walk_forward(self.get_cursor_position());
+
+					self.process_search_state.search_state.char_cursor_position +=
+						UnicodeWidthChar::width(caught_char).unwrap_or(0);
+
 					self.update_regex();
 					self.update_process_gui = true;
 				}

src/canvas.rs

@@ -4,7 +4,7 @@ use crate::{
 	data_conversion::{ConvertedCpuData, ConvertedProcessData},
 	utils::error,
 };
-use std::cmp::{max, min};
+use std::cmp::max;
 use std::collections::HashMap;
 use tui::{
 	backend,
@@ -1162,27 +1162,27 @@ impl Painter {
 		&self, f: &mut Frame<'_, B>, app_state: &mut app::App, draw_loc: Rect,
 	) {
 		let width = max(0, draw_loc.width as i64 - 34) as u64; // TODO: [REFACTOR] Hard coding this is terrible.
-		let query = app_state.get_current_search_query().as_str();
-		let grapheme_indices = UnicodeSegmentation::grapheme_indices(query, true).rev(); // Reverse due to us wanting to draw from back -> front
 		let cursor_position = app_state.get_cursor_position();
-		let right_border = min(UnicodeWidthStr::width(query), width as usize);
+		let char_cursor_position = app_state.get_char_cursor_position();
 
-		let mut itx = 0;
-		let mut query_with_cursor: Vec<Text<'_>> = if let app::WidgetPosition::ProcessSearch =
-			app_state.current_widget_selected
-		{
-			let mut res = Vec::new();
-			if cursor_position >= query.len() {
-				res.push(Text::styled(
-					" ",
-					self.colours.currently_selected_text_style,
-				))
-			}
+		let start_position: usize = get_search_start_position(
+			width as usize,
+			&app_state.process_search_state.search_state.cursor_direction,
+			&mut app_state.process_search_state.search_state.cursor_bar,
+			char_cursor_position,
+			app_state.is_resized,
+		);
 
-			res.extend(
-				grapheme_indices
+		let query = app_state.get_current_search_query().as_str();
+		let grapheme_indices = UnicodeSegmentation::grapheme_indices(query, true);
+		let mut current_grapheme_posn = 0;
+		let query_with_cursor: Vec<Text<'_>> =
+			if let app::WidgetPosition::ProcessSearch = app_state.current_widget_selected {
+				let mut res = grapheme_indices
 					.filter_map(|grapheme| {
-						if itx >= right_border {
+						current_grapheme_posn += UnicodeWidthStr::width(grapheme.1);
+
+						if current_grapheme_posn <= start_position {
 							None
 						} else {
 							let styled = if grapheme.0 == cursor_position {
@@ -1190,32 +1190,34 @@ impl Painter {
 							} else {
 								Text::styled(grapheme.1, self.colours.text_style)
 							};
-							itx += UnicodeWidthStr::width(grapheme.1);
 							Some(styled)
 						}
 					})
-					.collect::<Vec<_>>(),
-			);
+					.collect::<Vec<_>>();
 
-			res
-		} else {
-			// This is easier - we just need to get a range of graphemes, rather than
-			// dealing with possibly inserting a cursor (as none is shown!)
-			grapheme_indices
-				.filter_map(|grapheme| {
-					if itx >= right_border {
-						None
-					} else {
-						let styled = Text::styled(grapheme.1, self.colours.text_style);
-						itx += UnicodeWidthStr::width(grapheme.1);
-						Some(styled)
-					}
-				})
-				.collect::<Vec<_>>()
-		};
+				if cursor_position >= query.len() {
+					res.push(Text::styled(
+						" ",
+						self.colours.currently_selected_text_style,
+					))
+				}
 
-		// I feel like this is most definitely not the efficient way of doing this but eh
-		query_with_cursor.reverse();
+				res
+			} else {
+				// This is easier - we just need to get a range of graphemes, rather than
+				// dealing with possibly inserting a cursor (as none is shown!)
+				grapheme_indices
+					.filter_map(|grapheme| {
+						current_grapheme_posn += UnicodeWidthStr::width(grapheme.1);
+						if current_grapheme_posn <= start_position {
+							None
+						} else {
+							let styled = Text::styled(grapheme.1, self.colours.text_style);
+							Some(styled)
+						}
+					})
+					.collect::<Vec<_>>()
+			};
 
 		let mut search_text = vec![if app_state.is_grouped() {
 			Text::styled("Search by Name: ", self.colours.table_header_style)

src/canvas/drawing_utils.rs

@@ -72,11 +72,43 @@ pub fn get_variable_intrinsic_widths(
 
 #[allow(dead_code, unused_variables)]
 pub fn get_search_start_position(
-	num_rows: u64, scroll_direction: &app::ScrollDirection, scroll_position_bar: &mut u64,
-	currently_selected_position: u64, is_resized: bool,
-) -> u64 {
-	//TODO: [Scroll] Gotta fix this too lol
-	0
+	num_columns: usize, cursor_direction: &app::CursorDirection, cursor_bar: &mut usize,
+	current_cursor_position: usize, is_resized: bool,
+) -> usize {
+	if is_resized {
+		*cursor_bar = 0;
+	}
+
+	match cursor_direction {
+		app::CursorDirection::RIGHT => {
+			if current_cursor_position < *cursor_bar + num_columns {
+				// If, using previous_scrolled_position, we can see the element
+				// (so within that and + num_rows) just reuse the current previously scrolled position
+				*cursor_bar
+			} else if current_cursor_position >= num_columns {
+				// Else if the current position past the last element visible in the list, omit
+				// until we can see that element
+				*cursor_bar = current_cursor_position - num_columns;
+				*cursor_bar
+			} else {
+				// Else, if it is not past the last element visible, do not omit anything
+				0
+			}
+		}
+		app::CursorDirection::LEFT => {
+			if current_cursor_position <= *cursor_bar {
+				// If it's past the first element, then show from that element downwards
+				*cursor_bar = current_cursor_position;
+				*cursor_bar
+			} else if current_cursor_position >= *cursor_bar + num_columns {
+				*cursor_bar = current_cursor_position - num_columns;
+				*cursor_bar
+			} else {
+				// Else, don't change what our start position is from whatever it is set to!
+				*cursor_bar
+			}
+		}
+	}
 }
 
 pub fn get_start_position(

src/main.rs

@@ -399,6 +399,8 @@ fn handle_key_event_or_break(
 				KeyCode::Char('u') => app.clear_search(),
 				KeyCode::Char('a') => app.skip_cursor_beginning(),
 				KeyCode::Char('e') => app.skip_cursor_end(),
+				// Can't do now, CTRL+BACKSPACE doesn't work and graphemes
+				// are hard to iter while truncating last (eloquently).
 				// KeyCode::Backspace => app.skip_word_backspace(),
 				_ => {}
 			}