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

Subword motion ignores line breaks, jumps to next line unexpectedly #17780

Open
1 task done
ribelo opened this issue Sep 13, 2024 · 4 comments
Open
1 task done

Subword motion ignores line breaks, jumps to next line unexpectedly #17780

ribelo opened this issue Sep 13, 2024 · 4 comments
Labels
bug [core label] editor Feedback for code editing, formatting, editor iterations, etc linux linux-wayland Linux Wayland

Comments

@ribelo
Copy link

ribelo commented Sep 13, 2024

Check for existing issues

  • Completed

Describe the bug / provide steps to reproduce it

Description

The Subword motion feature is not working as expected when it encounters end-of-line characters. Instead of treating the end of the line as a boundary, it immediately jumps to the subword end of the next word on the line below.

Steps to Reproduce

  1. Open a file in Zed editor
  2. Place the cursor at the beginning of the last subword in a line
  3. Use the Subword motion command to move forward

Expected Behavior

The cursor should move to the end of the current word and stop at the end of the line.

Actual Behavior

The cursor skips the end-of-line character(s) and jumps to the subword end of the first word on the next line.

Environment

Zed: v0.152.3 (Zed)
OS: Linux Wayland nixos 24.11
Memory: 30.6 GiB
Architecture: x86_64
GPU: AMD Radeon Graphics (RADV REMBRANDT) || radv || Mesa 24.2.2

If applicable, add mockups / screenshots to help explain present your vision of the feature

2024-09-13-091340_wf.mp4

2024-09-13-092026_grim
2024-09-13-092037_grim
2024-09-13-092044_grim

If applicable, attach your Zed.log file to this issue.

Zed.log
@ribelo ribelo added admin read Pending admin review bug [core label] triage Maintainer needs to classify the issue labels Sep 13, 2024
@JosephTLyons JosephTLyons added editor Feedback for code editing, formatting, editor iterations, etc linux linux-wayland Linux Wayland and removed triage Maintainer needs to classify the issue admin read Pending admin review labels Oct 1, 2024
@github-actions github-actions bot added admin read Pending admin review triage Maintainer needs to classify the issue labels Nov 5, 2024
@notpeter notpeter removed triage Maintainer needs to classify the issue admin read Pending admin review labels Nov 5, 2024
@lionel-
Copy link

lionel- commented Nov 12, 2024

I see the same issue on macOS, so not Linux specific.

@0x2CA
Copy link
Contributor

0x2CA commented Dec 24, 2024

This is normal behavior, because it uses multiple lines to search, which simulates the movement of a word.

fn next_subword_start(
    map: &DisplaySnapshot,
    mut point: DisplayPoint,
    ignore_punctuation: bool,
    times: usize,
) -> DisplayPoint {
    let classifier = map
        .buffer_snapshot
        .char_classifier_at(point.to_point(map))
        .ignore_punctuation(ignore_punctuation);
    for _ in 0..times {
        let mut crossed_newline = false;
        let new_point = movement::find_boundary(map, point, FindRange::MultiLine, |left, right| {
            let left_kind = classifier.kind(left);
            let right_kind = classifier.kind(right);
            let at_newline = right == '\n';

            let is_word_start = (left_kind != right_kind) && !left.is_alphanumeric();
            let is_subword_start =
                left == '_' && right != '_' || left.is_lowercase() && right.is_uppercase();

            let found = (!right.is_whitespace() && (is_word_start || is_subword_start))
                || at_newline && crossed_newline
                || at_newline && left == '\n'; // Prevents skipping repeated empty lines

            crossed_newline |= at_newline;
            found
        });
        if point == new_point {
            break;
        }
        point = new_point;
    }
    point
}

@ConradIrwin

@lionel-
Copy link

lionel- commented Jan 6, 2025

hmm it makes subword motions pretty much unusable for me and is inconsistent with non-subword motions. I have a hard time seeing how this could be expected behaviour?

In this screencast I type e twice:

Screen.Recording.2025-01-06.at.11.08.15.mov

@0x2CA
Copy link
Contributor

0x2CA commented Jan 7, 2025

hmm it makes subword motions pretty much unusable for me and is inconsistent with non-subword motions. I have a hard time seeing how this could be expected behaviour?

In this screencast I type e twice:

Screen.Recording.2025-01-06.at.11.08.15.mov

Oh, sorry I misunderstood, indeed there is a problem.

When using e, it will unexpectedly skip many words and may even skip many lines.

The key code is here

pub(crate) fn next_subword_end(
    map: &DisplaySnapshot,
    mut point: DisplayPoint,
    ignore_punctuation: bool,
    times: usize,
    allow_cross_newline: bool,
) -> DisplayPoint {
    let classifier = map
        .buffer_snapshot
        .char_classifier_at(point.to_point(map))
        .ignore_punctuation(ignore_punctuation);
    for _ in 0..times {
        let new_point = next_char(map, point, allow_cross_newline);

        let mut crossed_newline = false;
        let mut need_backtrack = false;
        let new_point =
            movement::find_boundary(map, new_point, FindRange::MultiLine, |left, right| {
                let left_kind = classifier.kind(left);
                let right_kind = classifier.kind(right);
                let at_newline = right == '\n';

                if !allow_cross_newline && at_newline {
                    return true;
                }

                let is_word_end = (left_kind != right_kind) && !right.is_alphanumeric();
                let is_subword_end =
                    left != '_' && right == '_' || left.is_lowercase() && right.is_uppercase();

                let found = !left.is_whitespace() && !at_newline && (is_word_end || is_subword_end);

                if found && (is_word_end || is_subword_end) {
                    need_backtrack = true;
                }

                crossed_newline |= at_newline;
                found
            });
        let mut new_point = map.clip_point(new_point, Bias::Left);
        if need_backtrack {
            *new_point.column_mut() -= 1;
        }
        if point == new_point {
            break;
        }
        point = new_point;
    }
    point
}

If you replace if !allow_cross_newline && at_newline { return true; } with if at_newline { return true; } here, it will behave normally.

I don't quite understand the function of the allow_cross_newline parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug [core label] editor Feedback for code editing, formatting, editor iterations, etc linux linux-wayland Linux Wayland
Projects
None yet
Development

No branches or pull requests

5 participants