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

Cannot go to caret mode from normal mode #3877

Merged
merged 7 commits into from
Oct 17, 2022
Merged

Conversation

tuanbass
Copy link
Contributor

@tuanbass tuanbass commented Aug 4, 2021

In recent releases, I cannot go to the caret mode (by press "vc") from Normal mode.
So I create this patch to have it back.

@gdh1995 gdh1995 mentioned this pull request Nov 4, 2021
@gdh1995 gdh1995 mentioned this pull request Nov 17, 2021
@twio142
Copy link
Contributor

twio142 commented Dec 12, 2021

Hi, thanks for fixing the caret mode. :)

But there's one little bug here: In caret mode, forward word jumping using w or e acts just like in visual mode, but the selection only jumps forward the first time the key is pressed and then stops there. While in visual mode w and e do act properly.

I suppose it's due to the altering below (starting from line 55):

    // Native word movements behave differently on Linux and Windows, see #1441.  So we implement some of them
    // character-by-character.
    if (granularity === vimword && direction === forward) {
      while (this.nextCharacterIsWordCharacter())
        if (this.extendByOneCharacter(forward) === 0)
          return;
      while (this.getNextForwardCharacter() && !this.nextCharacterIsWordCharacter())
        if (this.extendByOneCharacter(forward) === 0)
          return;
    } else if (granularity === vimword) {
      return this.selection.modify(this.alterMethod, backward, word);
    }

FYI: I'm using Chrome 96 on Mac 10.15

@chuckmasucci
Copy link

Would love to see his merged!

Instead of call directly browser function to scroll, we call it via
setTimeout, so the event loop can be activated
Scrolling is stucked until end of the page reached.
@gdh1995
Copy link
Contributor

gdh1995 commented May 30, 2022

@tuanbass Hello, you have pushed some new commits to the master branch of your repo, while it's about scrolling, but not VisualMode. This affects this PR because the PR is bound to tuanbass:master directly.

@tuanbass
Copy link
Contributor Author

@tuanbass Hello, you have pushed some new commits to the master branch of your repo, while it's about scrolling, but not VisualMode. This affects this PR because the PR is bound to tuanbass:master directly.

Thanks for pointing out.
As the PR was opened for a long time, I have nearly forgotten about it, and tried to fix another bug.
Anyway, I reverted the last commit, and hope the PR will be merged soon.

@roryfahy
Copy link

roryfahy commented Sep 8, 2022

Any idea when this will be merged? Would really benefit from this functionality. Thank you.

@philc philc merged commit 5e856b0 into philc:master Oct 17, 2022
@philc
Copy link
Owner

philc commented Oct 17, 2022

This is great; thanks for the cleanup and the fix @tuanbass !

philc pushed a commit that referenced this pull request Oct 17, 2022
@gdh1995
Copy link
Contributor

gdh1995 commented Oct 17, 2022

Just now I reviewed all my code in #3867, and I found this PR lacks a place:

"/"() {
this.exit();
return new FindMode({returnToViewport: true}).onExit(() => new VisualMode());
},

Here a following .init() for new VisualMode() is also necessary. @philc

@philc
Copy link
Owner

philc commented Oct 18, 2022

Good catch @gdh1995; fixed in 5c00e2e

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.

6 participants