-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Left shift fix 2299 #2300
Left shift fix 2299 #2300
Conversation
@@ -171,11 +171,23 @@ function tokenizeKeySequence(sequence: string): string[] { | |||
let key = ''; | |||
let result: string[] = []; | |||
|
|||
// no close bracket, probably trying to do a left shift, take literal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried walking through this a couple of times, it's probably cause it's the end of the day, but I can't grok this code.
The code might be simpler if instead we walk the string, if we see <
, find the matching >
. If there's no matching <
, then it's a left shift.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what I'm doing is sort of the opposite, I'm looking for another <
following a <
, and then working back instead of looking ahead.
If I see a second <
(i.e. isBracketedKey
is already truthy when I hit a <
) I know whatever was previous was a left shift and subsequent literal character inputs, so I take the currently built key
and tokenize it one character per token.
After the tokenizing loop has run, if isBracketedKey
is truthy I know we also saw an unclosed <
so again I tokenize the currently built key
as one character per token.
I don't mind looking ahead for closing >
instead if you still prefer that. I didn't do that because it's a bit more complicated in the case of keysPressed
like <<<ESC>
where I would have to start counting open <
s and looking for matches or recursing which seemed overcomplicated for a convenience test helper function. That would be starting to get into serious parsing territory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although now that I've explained this I've found a bug, I will update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks for the explanation.
What this PR does / why we need it:
Fixes #2299
Left shift operator with G goto line/end command was not indenting last line. Seems to be because editor.action.outdentLines ignores last line of vscode.Selection when the end position is at the beginning of the last line. Taking end.getLineEnd() in the selection fixes this.
Which issue(s) this PR fixes
#2299