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 chord instead of label #218107

Merged
merged 3 commits into from
Jun 25, 2024
Merged

use chord instead of label #218107

merged 3 commits into from
Jun 25, 2024

Conversation

amunger
Copy link
Contributor

@amunger amunger commented Jun 25, 2024

fix #217901

Dispatch cords are strings built from hard coded values, whereas labels are configured from an OS specific mapping.

@amunger amunger marked this pull request as ready for review June 25, 2024 17:05
@amunger amunger enabled auto-merge (squash) June 25, 2024 17:05
@vscodenpa vscodenpa added this to the June 2024 milestone Jun 25, 2024
const keybinding = keybindings.find(kb => kb.getLabel() === 'Shift+Enter');
const keybinding = keybindings.find(kb => {
const chords = kb.getDispatchChords();
return chords.length === 1 && chords[0] === 'shift+Enter';
Copy link
Member

Choose a reason for hiding this comment

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

Are these localized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no

const keybinding = keybindings.find(kb => kb.getLabel() === 'Shift+Enter');
const keybinding = keybindings.find(kb => {
const chords = kb.getDispatchChords();
return chords.length === 1 && chords[0] === 'shift+Enter';
Copy link
Member

Choose a reason for hiding this comment

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

Don't you need a capital s?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's hard coded to 'shift'

let keybinding = keybindings.find(kb => {
const chords = kb.getDispatchChords();
return chords.length === 1 && chords[0] === 'Enter';
});
if (keybinding) {
return keybinding;
}
keybinding = this.keybindingService.lookupKeybindings('python.execInREPLEnter')
Copy link
Member

Choose a reason for hiding this comment

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

Why are we looking up python. commands in core?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

python shadows the command for the REPL, this hint doesn't work correctly if we don't look it up, and the point is to make this clear for python/jupyter users

amunger and others added 2 commits June 25, 2024 10:51
…entWidget.ts

Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
@amunger amunger merged commit e9b254e into main Jun 25, 2024
6 checks passed
@amunger amunger deleted the aamunger/execHint branch June 25, 2024 18:30
bricefriha pushed a commit to bricefriha/vscode that referenced this pull request Jun 26, 2024
aaronchucarroll pushed a commit to aaronchucarroll/vscode that referenced this pull request Jul 10, 2024
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Aug 9, 2024
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.

enabling interactiveWindow.executeWithShiftEnter does not update hint
3 participants