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

Change behavior of home/end button #21338

Merged
merged 5 commits into from
May 29, 2017
Merged

Change behavior of home/end button #21338

merged 5 commits into from
May 29, 2017

Conversation

misoguy
Copy link
Contributor

@misoguy misoguy commented Feb 24, 2017

Fixes #19644

@msftclas
Copy link

@misoguy,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

@mention-bot
Copy link

@misoguy, thanks for your PR! By analyzing the history of the files in this pull request, we identified @alexandrudima and @egamma to be potential reviewers.

@msftgits
Copy link

Hi, I am closing and re-opening this PR to bump the CLA bot. Sorry for the inconvenience!

@msftgits msftgits closed this Feb 24, 2017
@msftgits msftgits reopened this Feb 24, 2017
@msftclas
Copy link

@misoguy,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@misoguy
Copy link
Contributor Author

misoguy commented Feb 27, 2017

@rebornix How does the PR look? Is there room for improvement? or could this be merged as is?

@misoguy misoguy changed the title [WIP] Change behavior of home/end button Change behavior of home/end button Mar 1, 2017
@misoguy
Copy link
Contributor Author

misoguy commented Mar 2, 2017

@rebornix It'd be nice to get a feedback on this when you get the time :)

@@ -1234,11 +1234,17 @@ export class Cursor extends EventEmitter {
}

private _moveToBeginningOfLine(inSelectionMode: boolean, ctx: IMultipleCursorOperationContext): boolean {
return this._invokeForAll(ctx, (cursorIndex: number, oneCursor: OneCursor, oneCtx: IOneCursorOperationContext) => OneCursorOp.moveToBeginningOfLine(oneCursor, inSelectionMode, oneCtx));
Copy link
Member

Choose a reason for hiding this comment

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

_invokeForAll is for multi cursor, you may want to keep it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After taking a look at the code, the method _cursorMove which I used instead of _invokeForAll uses _invokeForAll internally so I'm guessing it's not a problem?
However, I think we do need to clarify how Home and End command should behave in multi cursor mode. I'll raise this question in the related issue directly.

@alexdima alexdima self-assigned this May 20, 2017
@alexdima alexdima added this to the May 2017 milestone May 29, 2017
@alexdima alexdima merged commit 6a2255e into microsoft:master May 29, 2017
@alexdima
Copy link
Member

Thank you @misoguy ! ❤️

@misoguy
Copy link
Contributor Author

misoguy commented May 30, 2017

@alexandrudima Thank you for merging this PR 👍

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cursor does not go to the end of line when pressing "End" with word wrap enabled
6 participants