Merge pull request #43 from ClementTsang/fix_unicode

.github/ISSUE_TEMPLATE/bug_report.md

@@ -23,12 +23,14 @@ _If applicable, add screenshots to help explain your problem:_
 
 ## Platform
 
-_Provide information on:_
+_If relevant, please provide information on:_
 
 **Operating System:**
 
 **Terminal:**
 
+**Shell:**
+
 **Any other relevant information (more details are always good!):**
 
 ## Additional context

.github/pull_request_template.md

@@ -23,11 +23,11 @@ _Please state how this was tested:_
 
 _Please ensure all are ticked (and actually done):_
 
-- [ ] Feature itself has been tested and verified to work
+- [ ] Change has been tested to work
-- [ ] Code has been linted
+- [ ] Code has been linted using rustfmt
 - [ ] Code has been self-reviewed
 - [ ] Code has been tested and no new breakage is introduced
-- [ ] Documentation has been added
+- [ ] Documentation has been added/updated if needed
 
 ## Other information
 

Cargo.toml

@@ -37,6 +37,8 @@ lazy_static = "1.4.0"
 backtrace = "0.3"
 toml = "0.5.6"
 serde = {version = "1.0", features = ["derive"] }
+unicode-segmentation = "1.6.0"
+unicode-width = "0.1.7"
 
 [dev-dependencies]
 assert_cmd = "0.12"

README.md

@@ -36,19 +36,25 @@ For information about config files, see [this document](./docs/config.md) for mo
 
 ## Installation
 
-In all cases you can install the in-development version by cloning from this repo and using `cargo build --release`. This is built and tested with Rust Stable (1.41.0 as of writing).
+In all cases you can install the in-development version by cloning from this repo and using `cargo build --release`. This is built and tested with Rust Stable (1.41 as of writing).
 
-In addition to the below methods, you can also get release versions using `cargo install bottom`, or manually building from the [Releases](https://github.com/ClementTsang/bottom/releases) page by downloading and building.
+In addition to the below methods, you can manually build from the [Releases](https://github.com/ClementTsang/bottom/releases) page by downloading and building.
 
-I officially support and test 64-bit variants. I will also build and release 32-bit variants for Linux and Windows, but I'm (currently) not testing whether they work.
+I officially support and test 64-bit versions of [Tier 1](https://forge.rust-lang.org/release/platform-support.html) Rust targets. I will try to build and release 32-bit versions for Linux and Windows, but as of now, I will not be testing 32-bit for validity beyond building.
+
+### Cargo
+
+```bash
+cargo install bottom
+```
 
 ### Linux
 
-Other installation methods based on distros are as follows:
+Installation methods on a per-distro basis:
 
 #### Arch Linux
 
-You can get the release versions from the [AUR](https://aur.archlinux.org/packages/bottom/) by installing `bottom`. For example:
+You can get the release versions from the [AUR](https://aur.archlinux.org/packages/bottom/) by installing `bottom`. For example, using `yay`:
 
 ```bash
 yay bottom
@@ -56,7 +62,7 @@ yay bottom
 
 #### Debian (and anything based on it, like Ubuntu)
 
-A `.deb` file is provided on each [release](https://github.com/ClementTsang/bottom/releases/latest). For example:
+A `.deb` file is provided on each [release](https://github.com/ClementTsang/bottom/releases/latest):
 
 ```bash
 curl -LO https://github.com/ClementTsang/bottom/releases/download/0.2.2/bottom_0.2.2_amd64.deb
@@ -196,6 +202,12 @@ Note that `q` is disabled while in the search widget.
 
 - Scrolling with the mouse will scroll through the currently selected list if the widget is a scrollable table.
 
+## Contribution
+
+Contribution is welcome!  Just submit a PR.
+
+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!
+
 ## Thanks, kudos, and all the like
 
 - This project is very much inspired by both [gotop](https://github.com/cjbassi/gotop) and [gtop](https://github.com/aksakalli/gtop).
@@ -217,4 +229,6 @@ Note that `q` is disabled while in the search widget.
   - [tokio](https://github.com/tokio-rs/tokio)
   - [toml-rs](https://github.com/alexcrichton/toml-rs)
   - [tui-rs](https://github.com/fdehau/tui-rs)
+  - [unicode-segmentation](https://github.com/unicode-rs/unicode-segmentation)
+  - [unicode-width](https://github.com/unicode-rs/unicode-width)
   - [winapi](https://github.com/retep998/winapi-rs)

src/app.rs

@@ -6,9 +6,13 @@ pub mod data_farmer;
 use data_farmer::*;
 
 use crate::{canvas, constants, utils::error::Result};
-
 mod process_killer;
 
+use unicode_segmentation::GraphemeCursor;
+use unicode_width::UnicodeWidthStr;
+
+const MAX_SEARCH_LENGTH: usize = 200;
+
 #[derive(Debug, Clone, Copy)]
 pub enum WidgetPosition {
 	Cpu,
@@ -28,6 +32,12 @@ pub enum ScrollDirection {
 	DOWN,
 }
 
+#[derive(Debug)]
+pub enum SearchDirection {
+	LEFT,
+	RIGHT,
+}
+
 /// AppScrollWidgetState deals with fields for a scrollable app's current state.
 #[derive(Default)]
 pub struct AppScrollWidgetState {
@@ -60,9 +70,9 @@ pub struct AppSearchState {
 	pub is_enabled: bool,
 	current_search_query: String,
 	current_regex: Option<std::result::Result<regex::Regex, regex::Error>>,
-	current_cursor_position: usize,
 	pub is_blank_search: bool,
 	pub is_invalid_search: bool,
+	pub grapheme_cursor: GraphemeCursor,
 }
 
 impl Default for AppSearchState {
@@ -71,9 +81,9 @@ impl Default for AppSearchState {
 			is_enabled: false,
 			current_search_query: String::default(),
 			current_regex: None,
-			current_cursor_position: 0,
 			is_invalid_search: false,
 			is_blank_search: true,
+			grapheme_cursor: GraphemeCursor::new(0, 0, true),
 		}
 	}
 }
@@ -535,7 +545,8 @@ impl App {
 	pub fn get_cursor_position(&self) -> usize {
 		self.process_search_state
 			.search_state
-			.current_cursor_position
+			.grapheme_cursor
+			.cur_cursor()
 	}
 
 	/// One of two functions allowed to run while in a dialog...
@@ -576,23 +587,26 @@ impl App {
 			WidgetPosition::Process => self.start_dd(),
 			WidgetPosition::ProcessSearch => {
 				if self.process_search_state.search_state.is_enabled
-					&& self
+					&& self.get_cursor_position()
-						.process_search_state
+						< self
-						.search_state
+							.process_search_state
-						.current_cursor_position < self
+							.search_state
-						.process_search_state
+							.current_search_query
-						.search_state
+							.len()
-						.current_search_query
-						.len()
 				{
 					self.process_search_state
 						.search_state
 						.current_search_query
-						.remove(
+						.remove(self.get_cursor_position());
-							self.process_search_state
+
-								.search_state
+					self.process_search_state.search_state.grapheme_cursor = GraphemeCursor::new(
-								.current_cursor_position,
+						self.get_cursor_position(),
-						);
+						self.process_search_state
+							.search_state
+							.current_search_query
+							.len(),
+						true,
+					);
 
 					self.update_regex();
 					self.update_process_gui = true;
@@ -619,9 +633,8 @@ impl App {
 
 	pub fn clear_search(&mut self) {
 		if let WidgetPosition::ProcessSearch = self.current_widget_selected {
-			self.process_search_state
+			self.process_search_state.search_state.grapheme_cursor =
-				.search_state
+				GraphemeCursor::new(0, 0, true);
-				.current_cursor_position = 0;
 			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;
@@ -629,26 +642,46 @@ impl App {
 		}
 	}
 
+	pub fn search_walk_forward(&mut self, start_position: usize) {
+		self.process_search_state
+			.search_state
+			.grapheme_cursor
+			.next_boundary(
+				&self.process_search_state.search_state.current_search_query[start_position..],
+				start_position,
+			)
+			.unwrap(); // TODO: [UNWRAP] unwrap in this and walk_back seem sketch
+	}
+
+	pub fn search_walk_back(&mut self, start_position: usize) {
+		self.process_search_state
+			.search_state
+			.grapheme_cursor
+			.prev_boundary(
+				&self.process_search_state.search_state.current_search_query[..start_position],
+				0,
+			)
+			.unwrap();
+	}
+
 	pub fn on_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 && self.get_cursor_position() > 0 {
-				&& self
+				self.search_walk_back(self.get_cursor_position());
-					.process_search_state
+
-					.search_state
-					.current_cursor_position
-					> 0
-			{
-				self.process_search_state
-					.search_state
-					.current_cursor_position -= 1;
 				self.process_search_state
 					.search_state
 					.current_search_query
-					.remove(
+					.remove(self.get_cursor_position());
-						self.process_search_state
+
-							.search_state
+				self.process_search_state.search_state.grapheme_cursor = GraphemeCursor::new(
-							.current_cursor_position,
+					self.get_cursor_position(),
-					);
+					self.process_search_state
+						.search_state
+						.current_search_query
+						.len(),
+					true,
+				);
 
 				self.update_regex();
 				self.update_process_gui = true;
@@ -683,16 +716,7 @@ impl App {
 	pub fn on_left_key(&mut self) {
 		if !self.is_in_dialog() {
 			if let WidgetPosition::ProcessSearch = self.current_widget_selected {
-				if self
+				self.search_walk_back(self.get_cursor_position());
-					.process_search_state
-					.search_state
-					.current_cursor_position
-					> 0
-				{
-					self.process_search_state
-						.search_state
-						.current_cursor_position -= 1;
-				}
 			}
 		} else if self.delete_dialog_state.is_showing_dd && !self.delete_dialog_state.is_on_yes {
 			self.delete_dialog_state.is_on_yes = true;
@@ -702,20 +726,7 @@ impl App {
 	pub fn on_right_key(&mut self) {
 		if !self.is_in_dialog() {
 			if let WidgetPosition::ProcessSearch = self.current_widget_selected {
-				if self
+				self.search_walk_forward(self.get_cursor_position());
-					.process_search_state
-					.search_state
-					.current_cursor_position
-					< self
-						.process_search_state
-						.search_state
-						.current_search_query
-						.len()
-				{
-					self.process_search_state
-						.search_state
-						.current_cursor_position += 1;
-				}
 			}
 		} else if self.delete_dialog_state.is_showing_dd && self.delete_dialog_state.is_on_yes {
 			self.delete_dialog_state.is_on_yes = false;
@@ -725,9 +736,14 @@ impl App {
 	pub fn skip_cursor_beginning(&mut self) {
 		if !self.is_in_dialog() {
 			if let WidgetPosition::ProcessSearch = self.current_widget_selected {
-				self.process_search_state
+				self.process_search_state.search_state.grapheme_cursor = GraphemeCursor::new(
-					.search_state
+					0,
-					.current_cursor_position = 0;
+					self.process_search_state
+						.search_state
+						.current_search_query
+						.len(),
+					true,
+				);
 			}
 		}
 	}
@@ -735,13 +751,17 @@ impl App {
 	pub fn skip_cursor_end(&mut self) {
 		if !self.is_in_dialog() {
 			if let WidgetPosition::ProcessSearch = self.current_widget_selected {
-				self.process_search_state
+				self.process_search_state.search_state.grapheme_cursor = GraphemeCursor::new(
-					.search_state
+					self.process_search_state
-					.current_cursor_position = self
+						.search_state
-					.process_search_state
+						.current_search_query
-					.search_state
+						.len(),
-					.current_search_query
+					self.process_search_state
-					.len();
+						.search_state
+						.current_search_query
+						.len(),
+					true,
+				);
 			}
 		}
 	}
@@ -788,13 +808,12 @@ impl App {
 	}
 
 	pub fn on_char_key(&mut self, caught_char: char) {
-		// Forbid any char key presses when showing a dialog box...
-
 		// Skip control code chars
 		if caught_char.is_control() {
 			return;
 		}
 
+		// Forbid any char key presses when showing a dialog box...
 		if !self.is_in_dialog() {
 			let current_key_press_inst = Instant::now();
 			if current_key_press_inst
@@ -806,21 +825,30 @@ impl App {
 			self.last_key_press = current_key_press_inst;
 
 			if let WidgetPosition::ProcessSearch = self.current_widget_selected {
-				self.process_search_state
+				if UnicodeWidthStr::width(
-					.search_state
+					self.process_search_state
-					.current_search_query
+						.search_state
-					.insert(
+						.current_search_query
+						.as_str(),
+				) <= MAX_SEARCH_LENGTH
+				{
+					self.process_search_state
+						.search_state
+						.current_search_query
+						.insert(self.get_cursor_position(), caught_char);
+
+					self.process_search_state.search_state.grapheme_cursor = GraphemeCursor::new(
+						self.get_cursor_position(),
 						self.process_search_state
 							.search_state
-							.current_cursor_position,
+							.current_search_query
-						caught_char,
+							.len(),
+						true,
 					);
-				self.process_search_state
+					self.search_walk_forward(self.get_cursor_position());
-					.search_state
+					self.update_regex();
-					.current_cursor_position += 1;
+					self.update_process_gui = true;
-
+				}
-				self.update_regex();
-				self.update_process_gui = true;
 			} else {
 				match caught_char {
 					'/' => {

src/canvas.rs

@@ -4,7 +4,7 @@ use crate::{
 	data_conversion::{ConvertedCpuData, ConvertedProcessData},
 	utils::error,
 };
-use std::cmp::max;
+use std::cmp::{max, min};
 use std::collections::HashMap;
 use tui::{
 	backend,
@@ -14,6 +14,8 @@ use tui::{
 	widgets::{Axis, Block, Borders, Chart, Dataset, Marker, Paragraph, Row, Table, Text, Widget},
 	Terminal,
 };
+use unicode_segmentation::UnicodeSegmentation;
+use unicode_width::UnicodeWidthStr;
 
 mod canvas_colours;
 use canvas_colours::*;
@@ -1155,55 +1157,66 @@ impl Painter {
 	fn draw_search_field<B: backend::Backend>(
 		&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;
+		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();
+		let query = app_state.get_current_search_query().as_str();
-		let shrunk_query = if query.len() < width as usize {
+		let grapheme_indices = UnicodeSegmentation::grapheme_indices(query, true).rev(); // Reverse due to us wanting to draw from back -> front
-			query
+		let cursor_position = app_state.get_cursor_position();
+		let right_border = min(UnicodeWidthStr::width(query), width as usize);
+		debug!(
+			"Width: {}, query length: {}",
+			width,
+			UnicodeWidthStr::width(query)
+		);
+
+		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,
+				))
+			}
+
+			res.extend(
+				grapheme_indices
+					.filter_map(|grapheme| {
+						if itx >= right_border {
+							None
+						} else {
+							let styled = if grapheme.0 == cursor_position {
+								Text::styled(grapheme.1, self.colours.currently_selected_text_style)
+							} else {
+								Text::styled(grapheme.1, self.colours.text_style)
+							};
+							itx += UnicodeWidthStr::width(grapheme.1);
+							Some(styled)
+						}
+					})
+					.collect::<Vec<_>>(),
+			);
+
+			res
 		} else {
-			&query[(query.len() - width as usize)..]
+			// 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<_>>()
 		};
 
-		let cursor_position = app_state.get_cursor_position();
+		// I feel like this is most definitely not the efficient way of doing this but eh
-
+		query_with_cursor.reverse();
-		let query_with_cursor: Vec<Text<'_>> =
-			if let app::WidgetPosition::ProcessSearch = app_state.current_widget_selected {
-				if cursor_position >= query.len() {
-					let mut q = vec![Text::styled(
-						shrunk_query.to_string(),
-						self.colours.text_style,
-					)];
-
-					q.push(Text::styled(
-						" ".to_string(),
-						self.colours.currently_selected_text_style,
-					));
-
-					q
-				} else {
-					shrunk_query
-						.chars()
-						.enumerate()
-						.map(|(itx, c)| {
-							if let app::WidgetPosition::ProcessSearch =
-								app_state.current_widget_selected
-							{
-								if itx == cursor_position {
-									return Text::styled(
-										c.to_string(),
-										self.colours.currently_selected_text_style,
-									);
-								}
-							}
-							Text::styled(c.to_string(), self.colours.text_style)
-						})
-						.collect::<Vec<_>>()
-				}
-			} else {
-				vec![Text::styled(
-					shrunk_query.to_string(),
-					self.colours.text_style,
-				)]
-			};
 
 		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

@@ -70,6 +70,15 @@ pub fn get_variable_intrinsic_widths(
 	(resulting_widths, last_index)
 }
 
+#[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
+}
+
 pub fn get_start_position(
 	num_rows: u64, scroll_direction: &app::ScrollDirection, scroll_position_bar: &mut u64,
 	currently_selected_position: u64, is_resized: bool,

src/main.rs

@@ -315,12 +315,18 @@ fn handle_mouse_event(event: MouseEvent, app: &mut App) {
 fn handle_key_event_or_break(
 	event: KeyEvent, app: &mut App, rtx: &std::sync::mpsc::Sender<ResetEvent>,
 ) -> bool {
+	//debug!("KeyEvent: {:?}", event);
+
+	// TODO: [PASTE] Note that this does NOT support some emojis like flags.  This is due to us
+	// catching PER CHARACTER right now WITH A forced throttle!  This means multi-char will not work.
+	// We can solve this (when we do paste probably) while keeping the throttle (mainly meant for movement)
+	// by throttling after *bulk+singular* actions, not just singular ones.
+
 	if event.modifiers.is_empty() {
 		// Required catch for searching - otherwise you couldn't search with q.
 		if event.code == KeyCode::Char('q') && !app.is_in_search_widget() {
 			return true;
 		}
-
 		match event.code {
 			KeyCode::End => app.skip_to_last(),
 			KeyCode::Home => app.skip_to_first(),