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

use ^H to delete word left in cmd.exe #98494

Merged
merged 1 commit into from
Jul 13, 2020
Merged

Conversation

connorskees
Copy link
Contributor

This resolves #98404 by conditionally changing which escape sequence is sent for ctrl+bksp based on the currently focused shell type.

Before:
recording (5)

After:
recording (4)

From the blame it looks like the best reviewer would be @Tyriar

I was unable to get around an issue in which the keybinding would not register for cmd.exe until the user unfocuses the terminal, either by clicking out of it, closing it (ctrl+backtick), or minimizing vscode. It only occurs when there are two bindings to the key -- when removing the other (^W) binding this issue no longer occurs.

Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

@connorskees nice, seems to work 👍

Comment on lines +178 to +185
if (platform.isWindows) {
// Delete word left: ctrl+h
// Windows cmd.exe requires ^H to delete full word left
registerSendSequenceKeybinding(String.fromCharCode('H'.charCodeAt(0) - 64), {
when: ContextKeyExpr.equals(KEYBINDING_CONTEXT_TERMINAL_SHELL_TYPE_KEY, WindowsShellType.CommandPrompt),
primary: KeyMod.CtrlCmd | KeyCode.Backspace,
});
}
Copy link
Member

Choose a reason for hiding this comment

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

@TylerLeonhardt any idea why ctrl+H deletes a word left for me in both pwsh and windows powershell on windows terminal but in VS Code only works in pwsh (windows powershell will delete a character left)?

Copy link
Member

Choose a reason for hiding this comment

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

hmm what versions of PSReadLine are available in those?

gmo PSReadLine

Copy link
Member

Choose a reason for hiding this comment

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

It says 2.0.0 for both in Windows PowerShell.

Copy link
Member

Choose a reason for hiding this comment

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

What edit mode are you using?:

Get-PSReadLineOption | % EditMode

cc @daxian-dbw

Copy link
Member

@Tyriar Tyriar Jun 8, 2020

Choose a reason for hiding this comment

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

Windows on both, I wonder if Windows Terminal does something clever here, the keybindings don't show anything bound to ctrl+h:

    "keybindings":
    [
        // Copy and paste are bound to Ctrl+Shift+C and Ctrl+Shift+V in your defaults.json.
        // These two lines additionally bind them to Ctrl+C and Ctrl+V.
        // To learn more about selection, visit https://aka.ms/terminal-selection
        { "command": {"action": "copy", "singleLine": false }, "keys": "ctrl+c" },
        { "command": "paste", "keys": "ctrl+v" },

        // Press Ctrl+Shift+F to open the search box
        { "command": "find", "keys": "ctrl+shift+f" },

        // Press Alt+Shift+D to open a new pane.
        // - "split": "auto" makes this pane open in the direction that provides the most surface area.
        // - "splitMode": "duplicate" makes the new pane use the focused pane's profile.
        // To learn more about panes, visit https://aka.ms/terminal-panes
        { "command": { "action": "splitPane", "split": "auto", "splitMode": "duplicate" }, "keys": "alt+shift+d" }
    ]

Copy link
Member

@TylerLeonhardt TylerLeonhardt Jun 8, 2020

Choose a reason for hiding this comment

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

Wait sorry, can you run this:

Get-Module PSReadLine | % PrivateData | % PSData

In Windows PowerShell, it won't tell you if it's a prerelease... only in pwsh will it tell you.

Copy link
Member

Choose a reason for hiding this comment

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

On both:

Name                           Value
----                           -----
Prerelease                     beta2

Copy link
Member

@daxian-dbw daxian-dbw Jun 8, 2020

Choose a reason for hiding this comment

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

In Windows Terminal, [Console]::ReadKey() shows Ctrl+Backspace for ctrl+h, while on the legacy console host, it shows Ctrl+H.
ctrl+Backspace is bound to BackwardKillWord in Windows edit mode, and hence it deletes a word in Windows Terminal.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @daxian-dbw, found the wt PR microsoft/terminal#3935

@zadjii-msft I followed the code in microsoft/terminal#3935 and end up hitting INPUT_RECORDs, is this support only possible once conpty gets microsoft/terminal#6309 (which I'm guessing is 12 months or something)?

@Tyriar Tyriar added this to the June 2020 milestone May 26, 2020
@Tyriar Tyriar modified the milestones: June 2020, July 2020 Jun 29, 2020
@Tyriar Tyriar merged commit 909356f into microsoft:master Jul 13, 2020
Tyriar added a commit that referenced this pull request Jul 13, 2020
@Tyriar
Copy link
Member

Tyriar commented Jul 13, 2020

Ok I added a named constant in 75e134a, thanks for the PR and input and sorry about the delay to get this merged 🙂

Tyriar pushed a commit that referenced this pull request Jul 14, 2020
this resolves an issue introduced in #98494 in which ctrl+bksp would not
delete a whole word when the currently open terminal was cmd.exe
@github-actions github-actions bot locked and limited conversation to collaborators Aug 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.

[Integrated Terminal] Allow ctrl+backspace to delete entire word in cmd.exe
5 participants