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.

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

Find: fix cursor position and match highlight when tabs, multi-byte UTF-8 characters are present #18

Merged
merged 1 commit into from
Apr 17, 2020

Conversation

ldang0
Copy link
Contributor

@ldang0 ldang0 commented Mar 16, 2020

To reproduce, edit a file with the following lines:

中文 and English 混合文字
第二行文字,包含	Tab 和空格

and find and or English and Tab respectively.

src/editor.rs Outdated
@@ -653,13 +653,13 @@ impl<'a> Editor<'a> {
for _ in 0..num_rows {
current = (current + if forward { 1 } else { num_rows - 1 }) % num_rows;
let row = &mut self.rows[current];
if let Some(rx) = row.render.find(&query) {
if let Some(cx) = row.render.find(&query) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

str::find returns byte index instead of character index.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cx usually represents the byte index in row.chars, not row.render.

Copy link
Owner

@ilai-deutel ilai-deutel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your new contribution. Unfortunately, these changes break cursor position when tabs are present (see this comment).

Perhaps fixing #19 first would simplify this problem?

src/row.rs Outdated
@@ -60,7 +60,7 @@ impl Row {
let (mut cx, mut rx) = (0, 0);
for c in String::from_utf8_lossy(&self.chars).chars() {
// The number of bytes used to store the character
let n_bytes = c.len_utf8();
let n_bytes = if c == '\t' { tab - (rx % tab) } else { c.len_utf8() };
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is right. If you type <tab> a, the cursor will misplaced (it cannot be moved to the a or after, even after pressing the End key).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Need to rework this fix.

src/row.rs Outdated
@@ -188,7 +188,8 @@ impl Row {
pub(crate) fn draw(&self, offset: usize, max_len: usize, buffer: &mut String) {
let mut current_hl_type = HLType::Normal;
let chars = self.render.chars().skip(offset).take(max_len);
for (c, (i, mut hl_type)) in chars.zip(self.hl.iter().enumerate().skip(offset)) {
let mut cx = self.render.chars().take(offset).map(|c| c.len_utf8()).sum();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to use rx2cx to compute cx: let cx = self.rx2cx[i] (you can rename i to rx if you want).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unformately, in this case, i is not rx. For example,

use unicode_width::UnicodeWidthChar;

fn main() {
    let s = "中文";
    s.chars().for_each(|c| println!("{}, {}", c, c.width().unwrap_or(1)));
}

We got

中, 2
文, 2

So for , its rx supposes to be 2 instead of 1.

src/editor.rs Outdated
self.cursor.y = current as usize;
self.cursor.x = row.rx2cx[rx];
self.cursor.x = cx;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.cursor.x should be the cursor position on the screen, not in bytes.

src/editor.rs Outdated
@@ -653,13 +653,13 @@ impl<'a> Editor<'a> {
for _ in 0..num_rows {
current = (current + if forward { 1 } else { num_rows - 1 }) % num_rows;
let row = &mut self.rows[current];
if let Some(rx) = row.render.find(&query) {
if let Some(cx) = row.render.find(&query) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cx usually represents the byte index in row.chars, not row.render.

@ilai-deutel ilai-deutel changed the title Fix find Find: fix cursor position and match highlight when tabs, multi-byte UTF-8 characters are present Mar 18, 2020
@ilai-deutel
Copy link
Owner

@ldang0 It looks like your PR #23 fixed the cursor position after find issue (:tada:), but not the match highlight during find issue.

Are you still planning on working on this PR? I think I will cut a release when this issue is fixed.

@ldang0
Copy link
Contributor Author

ldang0 commented Apr 15, 2020

Are you still planning on working on this PR? I think I will cut a release when this issue is fixed.

Yes.

Recently I am very busy on other stuffs.

@ilai-deutel ilai-deutel merged commit 9a3003c into ilai-deutel:master Apr 17, 2020
@ldang0 ldang0 deleted the fix-find branch April 17, 2020 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants