-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
Hotkey for Windows shell context menu #17710
Conversation
Hi @mlewand, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
ac6c098
to
c90b603
Compare
Rebased the branch onto latest master so that we have nice and green tests :) Also I'll close and reopen the PR as I signed CLA yesterday and it still did not take effect... |
Hi @mlewand, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
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'd be more comfortable with this if we simply had a shellNameShort
property in package.json
, leaving all the magic out of it.
Also, have you verified that simply adding a &
before a letter works?
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'd be more comfortable with this if we simply had a shellNameShort
property in package.json
, leaving all the magic out of it.
Also, have you verified that simply adding a &
before a letter works?
@joaomoreno thanks for the R. Yea I verified this change and it works well.
Do you want me to put |
Sorry, I meant |
Ahh, now it makes perfect sense 🙂 I'll take a look on that in a spare time. |
…Windows shell, allowing to provide a custom hotkey.
c90b603
to
e02ccc6
Compare
@joaomoreno all right the code has been pushed. I have amended the former commit, to keep the history shorter. |
@joaomoreno any chances of getting it in some time soon? It's a pretty trivial fix, while pretty important for Windows keyboard users. |
Good job! |
First of all thanks for a great editor, it's high time to give you some love back. 🙂
Here's the pull request to fix #15939, which essentially requests for a hotkey in Windows shell context menu.
It adds a
"win32ShellHotkey"
property in product.json, allowing for hotkey customization.I suggest setting default value to "o" with following reason:
The cool thing is that if you completely skip
"win32ShellHotkey"
property in product.json instead of crushing it simply won't cause an error in build process but will create no hotkey at all. And if you set it to empty, it will use the first letter fromproduct.nameShort
.Also I think it should be case insensitive, so if you set it doesn't matter if you set it to
"o"
or"O"
.The only thing that I did not implement on purpose is handling for case when
product.win32ShellHotkey
character is not present inproduct.nameShort
. Ideally we'd want to append it like: "Open with Code - OSS (y)" - but I wanted to keep it clean, and avoid unnecessary logic. Anyway, if that's something you'd like to get, just let me know.