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

Introduce a move command for moving cursor in the editor #9143

Closed
sandy081 opened this issue Jul 12, 2016 · 27 comments
Closed

Introduce a move command for moving cursor in the editor #9143

sandy081 opened this issue Jul 12, 2016 · 27 comments
Assignees
Labels
api feature-request Request for new features or functionality VIM VIM issue
Milestone

Comments

@sandy081
Copy link
Member

sandy081 commented Jul 12, 2016

Introduce a move command for moving cursor at a logical view position in the editor

This will allow the extension writers to place the cursor at a provided position in the editor

Usage:

  • Command Id: cursorMove
  • Arguments: {to: $ViewPosition, select: boolean, by: $By, value: number}
  • Defaults: value = 1. by based on to value.

ViewPosition

  • left: Moves the cursor left by n characters.
  • right: Moves the cursor right by n characters.
  • up: Moves the cursor up by n lines or wrapped lines.
  • down: Moves the cursor down by n lines or wrapped lines.
  • wrappedLineStart: Places the cursor at the start of the current line in the view.
  • wrappedLineFirstNonWhitespaceCharacter: Places the cursor at the first non white space character of the current line in the view.
  • wrappedLineEnd: Places the cursor at the end of the current line in the view.
  • wrappedLineLastNonWhitespaceCharacter: Places the cursor at the last non white space character of the current line in the view.
  • wrappedLineColumnCenter: Places the cursor at the center of the line in the view.
  • viewPortTop: Move the cursor to the first non white space character of Nth line from the top of the view.
  • viewPortCenter: Move the cursor to the first non white space character of the center line of the view.
  • viewPortBottom: Move the cursor to the first non white space character of Nth line from the bottom of the view.

By

  • line - Default for line specific commands
  • wrappedLine
  • character - Default for character specific commands
  • halfLine - Only for character specific commands.

Set select arg to true, to select text for above

@sandy081 sandy081 added the api label Jul 12, 2016
@sandy081 sandy081 added this to the July 2016 milestone Jul 12, 2016
@sandy081 sandy081 self-assigned this Jul 12, 2016
@sandy081
Copy link
Member Author

@rebornix I am going to implement a command name 'move' which can take arguments. Based on the arguments command will move the cursor appropriately. Let me know if you have any specifications so that I can make them part of it.

As a beginning, this will let to move the cursor to

  • first character in the screen line
  • first non white space character in the screen line

@siegebell
Copy link

Is this what you need?

// set anchor=active for just positioning the cursor (no selection)
editor.selections = [new vscode.Selection(anchor, active)]

@sandy081
Copy link
Member Author

// set anchor=active for just positioning the cursor (no selection)
editor.selections = [new vscode.Selection(anchor, active)]

@siegebell No, above will place the cursor as per the editor model. Instead this new command will allow you to move it to a position you wanted on the screen. For eg: If a line is wrapped and if you want to move the cursor to the beginning of the line on the screen (not to the beginning of the complete line) above javascript will not able to do that.

@siegebell
Copy link

@sandy081 Ah, you want screen-relative coordinates. Good idea. But I would find a function that translates screen coordinates into a vscode.Position more useful. Would that also fit the requirements?

@sandy081
Copy link
Member Author

sandy081 commented Jul 12, 2016

@siegebell We can but this will introduce view properties on the text editor model which is not a good idea. Hence we intended to provide commands so that users can execute them.

@rebornix
Copy link
Member

@sandy081 these commands look good to me and really thank you! For now I still need time to check whether they are suitable to Vim integration, whether it's a direct support or a workaround.

@rebornix
Copy link
Member

@sandy081 just played around with these APIs, thank you very much as they make the implementation really simple our side. I bet other extensions can also benefit from these APIs.

There is some unexpected behaviors while running cursorMove, the first thing is lineEnd actually moves the cursor the first character of next screen line. The position line and character are both zero-based, so I'm guesssing maybe we forget to decrease by 1 while calculating.

Another one is lineCenter is just keeping the cursor where it was. Not sure did I miss anything.

@sandy081
Copy link
Member Author

@rebornix I see lineEnd and lineCenter commands are behaving as expected for me. I guess you would be calling these arguments just similar to others. If you can provide any information that can help in reproducing would be great.

@rebornix
Copy link
Member

@sandy081 I used the keywords put in this issue description (up, down), while it's lineUp and lineDown per your code. It was my misunderstand and lineCenter, lineUp and lineDown works perfectly.

But I still have problem with lineEnd, will sync with @sandy081 offline. All others are really awesome.

@sandy081
Copy link
Member Author

@rebornix Yah, I forgot to update the description with updated keywords. Thanks for pointing this.

@sandy081
Copy link
Member Author

@rebornix It would be great if you can give me an updated list of VIM commands (all variations) which are implemented / implementable (with existing API) and which are still not. I have seen the VIM extension road map but I think it is not up to date or does not have all variations.. we can also discuss it offline.

Thanks

@sandy081
Copy link
Member Author

@rebornix Commands H, M and L are supported with selection.

@rebornix
Copy link
Member

@sandy081 I just found that Vim's H, M, L are not referring to Screen Line. They are referring to real line numbers on the gutter. I'm not sure whether it's confusing putting both screen line and real line in the same API all together, what do you think?

@sandy081
Copy link
Member Author

@rebornix above commands are moving to Screen line not to real line. Can you please check by having a very small wrapping size. I think the issue you are seeing is if the screen line above top screen line is little visible then H command jumps to that. Also there is a similar issue for bottom of the view - #9542. It looks to me an issue while computing the lines layout. Since Alex is on vacation we have to wait on that.

Otherwise, H, M and L commands are moving to screen line. Please double check with a line wrapped into more than 2 lines.

@sandy081
Copy link
Member Author

sandy081 commented Jul 21, 2016

@rebornix

  • Re-Implemented viewPortTop, viewPortBottom and viewPortCenter commands by real lines instead of wrapped lines.
  • Implemented left and right commands to move cursor left and right respectively by 1, n or halfLine characters
  • Changed the key word names

Adopt to the above changes and let me know the feedback.

Thanks

@sandy081
Copy link
Member Author

sandy081 commented Jul 25, 2016

@rebornix Please note the small change done to the API for consistency and clean

  • Renamed argument parameter amount to value.

Please adopt to these changes

@aminroosta
Copy link

vkkkkkk

@sandy081 cursorMove does not respect already selected text.
Vjjjj ing works find but Vkkkkkk ing will break selected code.

    await vscode.commands.executeCommand("cursorMove", {
     to: "up",
     by: "wrappedLine",
     select: true,
     value: count
   });

I've spent a couple of hours on this, it seems that activeTextEditor.slection is not handled properly.

@sandy081 Please give us the same API without moving the cursor, Vim extensions are masters on how to move the cursor themselves.

@aminroosta
Copy link

@sandy081 This is how we can practically use your api, moveDown record the position then moveUp. This makes life hard and moving around slow and if you move pass the first line or the last line some wired visual feedback can be seen (you can see cursor moving up and then down and them up) ❗

Let's face it, no other plugin is going to use these commands other than VIM extensions 😉
And we already know that no internal editor stuff needs to be exposed for folding info.

These makes both vim extension writer life and vscode team easier.
So i'm going to ask once more hoping that you reconsider this wrong design approach 😄

@johnfn
Copy link
Contributor

johnfn commented Sep 4, 2016

@sandy081

I would like to chime in along with @aminroosta here.

Unfortunately, the current cursorMove API is not powerful enough to implement most core features of Vim. Simply moving the cursor is not enough - we actually need the position. Without the position, there's no practical way to use the value any of the visual modes (which effectively take the position, then apply some transformations on it, and then create a new selection object). This means that visual mode, visual line mode and visual block mode cannot be implemented using this API.

Again, we are all super appreciative of all the work that you guys do on such a great editor. Let us know if there's anything we can do to help!

@sandy081
Copy link
Member Author

sandy081 commented Sep 5, 2016

@aminroosta

@sandy081 cursorMove does not respect already selected text.
Vjjjj ing works find but Vkkkkkk ing will break selected code.

It is working for me very smoothly. Please see the attached video. I call the following command just like you did

vscode.commands.executeCommand("cursorMove", {
     to: "up",
     by: "wrappedLine",
     select: true,
     value: count
   });

sep-05-2016 18-15-05

Unfortunately, the current cursorMove API is not powerful enough to implement most core features of Vim.

@rebornix, In regards to @johnfn comment, may I know what is missing here? I remember that this can be used to implement most of cursor movement functionality in Vim. In fact, I implemented in our Vim sample extension.

@aminroosta
Copy link

@sandy081 Thanks for taking time and checking this issue. So there should be something wrong with my code. If your code is available online on github please let me know.

@sandy081
Copy link
Member Author

sandy081 commented Sep 6, 2016

@aminroosta here is our Vim sample extension.

@aminroosta
Copy link

aminroosta commented Sep 6, 2016

@sandy081 vkkkk (lower case v) is perfectly fine, i'm having a hard time with Vkkkk (upper case V) that selects lines.

@aminroosta
Copy link

vkkkk

@sandy081 It seems only lower case vk can be implemented with cursorMove, i can't fine any information in the documentation (first comment above) that suggests how to implement Vkkk ing.

@sandy081
Copy link
Member Author

sandy081 commented Sep 6, 2016

@aminroosta Just to note that the sample vim extension does not have all complete vim features. This is an example/reference implementation showing how some of the vim features can be implemented using the editor commands.

With regards to upper case V, sample supports moving to visual mode only on lower case v. You can refer to this to implement the other.

@rebornix might help you in how to consume this.

@rebornix
Copy link
Member

rebornix commented Sep 6, 2016

@sandy081 thanks for the tip.

@aminroosta I'm adding these command sin my separate PR but it does hacking to the view updating phase so it's still WIP. I'll let you know if I solve the issues you met.

@rebornix
Copy link
Member

rebornix commented Sep 6, 2016

I see the problem, I only used cursorMove for screen lines moving but not regular lines, that might describe I don't see any issues @aminroosta ran into. But @aminroosta is doing the right thing, implementing all Vim cursor moving commands by cursorMove instead of doing things in different ways. Let me give them a check real quick.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api feature-request Request for new features or functionality VIM VIM issue
Projects
None yet
Development

No branches or pull requests

5 participants