-
Notifications
You must be signed in to change notification settings - Fork 300
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
[Vi-Mode] Supports di' and di" commands (quoted text objects) #3791
[Vi-Mode] Supports di' and di" commands (quoted text objects) #3791
Conversation
a93af44
to
2495382
Compare
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.
Before moving forward with more 3-key chord supports, we need to first figure out how to make Get/Set-PSReadLineKeyHandler
support chords with more than 2 keys. Today, _chordDispatchTable
is a 2-level dictionary, and hence it only really supports 2-key chords. Get/Set-PSReadLineKeyHandler
are implemented under that assumption.
By looking at #2059 again, I think I merged it a little too prematurely :)
Today, we have two 3-key chords built-in:
<d,g,g>
-- implemented by havingg
mapped toViDBChord
in theDTable
with aDGTable
that maps the secondg
to the handler.<d,w,i>
--implemented by [vi-mode] Supports 'diw' text-object command. #2059.
There are 2 concerns here:
- Their implementation should be consistent in the way how the 3rd level dictionary is stored.
- Changes to VI key binding data structure are needed to make
Get/Set-PSReadLineKeyHandler
to support chords with 3 keys (are we stopping at 3-key chords for VI mode here? maybe we should and not support chords with more than 3 keys).
I think we should first resolve the above concerns before adding more text object commands.
/// starting from the specified "current" position. | ||
/// </summary> | ||
/// <param name="current">The position in the current logical line.</param> | ||
internal static int GetBeginningOfLogicalLinePos(this StringBuilder buffer, int current) |
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.
It seems GetBeginningOfLogicalLinePos
and GetEndOfLogicalLinePos
shoudl be put in StringBuilderLinewiseExtensions
instead.
@@ -1,4 +1,5 @@ | |||
using System; | |||
using System.Management.Automation; |
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.
Added by mistake? It doesn't seem to be needed by the code changes in this file.
BTW, the
Two problems here:
It's the same for
|
@daxian-dbw I have published #3853 to address your feedback, re-working #2059 in the process. |
PR Summary
Fixes #3790.
PR Checklist
As #2059 has been readily accepted, we have not had the time to discuss the documentation.
This PR, like #2059, wire keybindings to private methods. Please, let me know how to best document this behaviour.
Microsoft Reviewers: Open in CodeFlow