Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: fix search scrolling with wider Unicode characters. #938

Merged
merged 3 commits into from
Dec 31, 2022
Merged

bug: fix search scrolling with wider Unicode characters. #938

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0a6328c
remove arbitrary search limit
ClementTsang Dec 6, 2022
689fed6
bug: fix scroll width calculation for search
ClementTsang Dec 27, 2022
8b6a336
add some basic tests
ClementTsang Dec 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [#805](https://github.com/ClementTsang/bottom/pull/805): Fix bottom keeping devices awake in certain scenarios.
- [#825](https://github.com/ClementTsang/bottom/pull/825): Use alternative method of getting parent PID in some cases on macOS devices to avoid needing root access.
- [#916](https://github.com/ClementTsang/bottom/pull/916): Fix possible gaps with widget layout spacing.
- [#938](https://github.com/ClementTsang/bottom/pull/938): Fix search scrolling with wider Unicode characters.

## Changes

Expand Down
165 changes: 37 additions & 128 deletions src/app.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use layout_manager::*;
pub use states::*;
use typed_builder::*;
use unicode_segmentation::{GraphemeCursor, UnicodeSegmentation};
use unicode_width::{UnicodeWidthChar, UnicodeWidthStr};

use crate::widgets::{ProcWidget, ProcWidgetMode};
use crate::{
Expand All @@ -32,8 +31,6 @@ pub mod states;

use frozen_state::FrozenState;

const MAX_SEARCH_LENGTH: usize = 200; // FIXME: Remove this limit, it's unnecessary.

#[derive(Debug, Clone)]
pub enum AxisScaling {
Log,
Expand Down Expand Up @@ -504,23 +501,21 @@ impl App {
{
if is_in_search_widget {
if proc_widget_state.proc_search.search_state.is_enabled
&& proc_widget_state.get_search_cursor_position()
&& proc_widget_state.cursor_char_index()
< proc_widget_state
.proc_search
.search_state
.current_search_query
.len()
{
let current_cursor = proc_widget_state.get_search_cursor_position();
proc_widget_state
.search_walk_forward(proc_widget_state.get_search_cursor_position());
let current_cursor = proc_widget_state.cursor_char_index();
proc_widget_state.search_walk_forward();

let _removed_chars: String = proc_widget_state
let _ = proc_widget_state
.proc_search
.search_state
.current_search_query
.drain(current_cursor..proc_widget_state.get_search_cursor_position())
.collect();
.drain(current_cursor..proc_widget_state.cursor_char_index());

proc_widget_state.proc_search.search_state.grapheme_cursor =
GraphemeCursor::new(
Expand Down Expand Up @@ -552,22 +547,21 @@ impl App {
{
if is_in_search_widget
&& proc_widget_state.proc_search.search_state.is_enabled
&& proc_widget_state.get_search_cursor_position() > 0
&& proc_widget_state.cursor_char_index() > 0
{
let current_cursor = proc_widget_state.get_search_cursor_position();
proc_widget_state
.search_walk_back(proc_widget_state.get_search_cursor_position());
let current_cursor = proc_widget_state.cursor_char_index();
proc_widget_state.search_walk_back();

let removed_chars: String = proc_widget_state
// Remove the indices in between.
let _ = proc_widget_state
.proc_search
.search_state
.current_search_query
.drain(proc_widget_state.get_search_cursor_position()..current_cursor)
.collect();
.drain(proc_widget_state.cursor_char_index()..current_cursor);

proc_widget_state.proc_search.search_state.grapheme_cursor =
GraphemeCursor::new(
proc_widget_state.get_search_cursor_position(),
proc_widget_state.cursor_char_index(),
proc_widget_state
.proc_search
.search_state
Expand All @@ -576,11 +570,6 @@ impl App {
true,
);

proc_widget_state
.proc_search
.search_state
.char_cursor_position -= UnicodeWidthStr::width(removed_chars.as_str());

proc_widget_state.proc_search.search_state.cursor_direction =
CursorDirection::Left;

Expand Down Expand Up @@ -684,19 +673,9 @@ impl App {
.get_mut_widget_state(self.current_widget.widget_id - 1)
{
if is_in_search_widget {
let prev_cursor = proc_widget_state.get_search_cursor_position();
proc_widget_state
.search_walk_back(proc_widget_state.get_search_cursor_position());
if proc_widget_state.get_search_cursor_position() < prev_cursor {
let str_slice = &proc_widget_state
.proc_search
.search_state
.current_search_query
[proc_widget_state.get_search_cursor_position()..prev_cursor];
proc_widget_state
.proc_search
.search_state
.char_cursor_position -= UnicodeWidthStr::width(str_slice);
let prev_cursor = proc_widget_state.cursor_char_index();
proc_widget_state.search_walk_back();
if proc_widget_state.cursor_char_index() < prev_cursor {
proc_widget_state.proc_search.search_state.cursor_direction =
CursorDirection::Left;
}
Expand Down Expand Up @@ -753,20 +732,9 @@ impl App {
.get_mut_widget_state(self.current_widget.widget_id - 1)
{
if is_in_search_widget {
let prev_cursor = proc_widget_state.get_search_cursor_position();
proc_widget_state.search_walk_forward(
proc_widget_state.get_search_cursor_position(),
);
if proc_widget_state.get_search_cursor_position() > prev_cursor {
let str_slice = &proc_widget_state
.proc_search
.search_state
.current_search_query
[prev_cursor..proc_widget_state.get_search_cursor_position()];
proc_widget_state
.proc_search
.search_state
.char_cursor_position += UnicodeWidthStr::width(str_slice);
let prev_cursor = proc_widget_state.cursor_char_index();
proc_widget_state.search_walk_forward();
if proc_widget_state.cursor_char_index() > prev_cursor {
proc_widget_state.proc_search.search_state.cursor_direction =
CursorDirection::Right;
}
Expand Down Expand Up @@ -932,10 +900,7 @@ impl App {
.len(),
true,
);
proc_widget_state
.proc_search
.search_state
.char_cursor_position = 0;

proc_widget_state.proc_search.search_state.cursor_direction =
CursorDirection::Left;
}
Expand All @@ -954,30 +919,14 @@ impl App {
.get_mut(&(self.current_widget.widget_id - 1))
{
if is_in_search_widget {
proc_widget_state.proc_search.search_state.grapheme_cursor =
GraphemeCursor::new(
proc_widget_state
.proc_search
.search_state
.current_search_query
.len(),
proc_widget_state
.proc_search
.search_state
.current_search_query
.len(),
true,
);
proc_widget_state
let query_len = proc_widget_state
.proc_search
.search_state
.char_cursor_position = UnicodeWidthStr::width(
proc_widget_state
.proc_search
.search_state
.current_search_query
.as_str(),
);
.current_search_query
.len();

proc_widget_state.proc_search.search_state.grapheme_cursor =
GraphemeCursor::new(query_len, query_len, true);
proc_widget_state.proc_search.search_state.cursor_direction =
CursorDirection::Right;
}
Expand Down Expand Up @@ -1008,11 +957,11 @@ impl App {
// Traverse backwards from the current cursor location until you hit non-whitespace characters,
// then continue to traverse (and delete) backwards until you hit a whitespace character. Halt.

// So... first, let's get our current cursor position using graphemes...
let end_index = proc_widget_state.get_char_cursor_position();
// So... first, let's get our current cursor position in terms of char indices.
let end_index = proc_widget_state.cursor_char_index();

// Then, let's crawl backwards until we hit our location, and store the "head"...
let query = proc_widget_state.get_current_search_query();
let query = proc_widget_state.current_search_query();
let mut start_index = 0;
let mut saw_non_whitespace = false;

Expand All @@ -1032,12 +981,11 @@ impl App {
}
}

let removed_chars: String = proc_widget_state
let _ = proc_widget_state
.proc_search
.search_state
.current_search_query
.drain(start_index..end_index)
.collect();
.drain(start_index..end_index);

proc_widget_state.proc_search.search_state.grapheme_cursor = GraphemeCursor::new(
start_index,
Expand All @@ -1049,11 +997,6 @@ impl App {
true,
);

proc_widget_state
.proc_search
.search_state
.char_cursor_position -= UnicodeWidthStr::width(removed_chars.as_str());

proc_widget_state.proc_search.search_state.cursor_direction = CursorDirection::Left;

proc_widget_state.update_query();
Expand Down Expand Up @@ -1113,40 +1056,24 @@ impl App {
.widget_states
.get_mut(&(self.current_widget.widget_id - 1))
{
if is_in_search_widget
&& proc_widget_state.is_search_enabled()
&& UnicodeWidthStr::width(
proc_widget_state
.proc_search
.search_state
.current_search_query
.as_str(),
) <= MAX_SEARCH_LENGTH
{
if is_in_search_widget && proc_widget_state.is_search_enabled() {
proc_widget_state
.proc_search
.search_state
.current_search_query
.insert(proc_widget_state.get_search_cursor_position(), caught_char);
.insert(proc_widget_state.cursor_char_index(), caught_char);

proc_widget_state.proc_search.search_state.grapheme_cursor =
GraphemeCursor::new(
proc_widget_state.get_search_cursor_position(),
proc_widget_state.cursor_char_index(),
proc_widget_state
.proc_search
.search_state
.current_search_query
.len(),
true,
);
proc_widget_state
.search_walk_forward(proc_widget_state.get_search_cursor_position());

proc_widget_state
.proc_search
.search_state
.char_cursor_position +=
UnicodeWidthChar::width(caught_char).unwrap_or(0);
proc_widget_state.search_walk_forward();

proc_widget_state.update_query();
proc_widget_state.proc_search.search_state.cursor_direction =
Expand Down Expand Up @@ -2724,22 +2651,10 @@ impl App {
.widget_states
.get_mut(&(self.current_widget.widget_id - 1))
{
let curr_width = UnicodeWidthStr::width(
proc_widget_state
.proc_search
.search_state
.current_search_query
.as_str(),
);
let paste_width = UnicodeWidthStr::width(paste.as_str());
let num_runes = UnicodeSegmentation::graphemes(paste.as_str(), true).count();

if is_in_search_widget
&& proc_widget_state.is_search_enabled()
&& curr_width + paste_width <= MAX_SEARCH_LENGTH
{
let paste_char_width = paste.len();
let left_bound = proc_widget_state.get_search_cursor_position();
if is_in_search_widget && proc_widget_state.is_search_enabled() {
let left_bound = proc_widget_state.cursor_char_index();

let curr_query = &mut proc_widget_state
.proc_search
Expand All @@ -2752,15 +2667,9 @@ impl App {
GraphemeCursor::new(left_bound, curr_query.len(), true);

for _ in 0..num_runes {
let cursor = proc_widget_state.get_search_cursor_position();
proc_widget_state.search_walk_forward(cursor);
proc_widget_state.search_walk_forward();
}

proc_widget_state
.proc_search
.search_state
.char_cursor_position += paste_char_width;

proc_widget_state.update_query();
proc_widget_state.proc_search.search_state.cursor_direction =
CursorDirection::Right;
Expand Down
2 changes: 1 addition & 1 deletion src/app/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ pub fn parse_query(
if content == "=" {
// Check next string if possible
if let Some(queue_next) = query.pop_front() {
// TODO: [Query, ???] Need to consider the following cases:
// TODO: [Query] Need to consider the following cases:
// - (test)
// - (test
// - test)
Expand Down
Loading