Skip to content

Conversation

@JustForFun88
Copy link
Contributor

This PR includes details about Unicode code points in keybindings, as requested in the comment. The changes are based on the improvements introduced in #14020.


Starting from [#14020](https://github.com/nushell/nushell/pull/14020), users can now specify characters in the keycode field using their Unicode code points, such as `char_u003B` for the semicolon (`;`). This provides greater flexibility when configuring keybindings in Nushell.

Previously, it was possible to specify characters by enclosing them in quotes, like keycode: "char_;". However, this approach was inconsistent for all Unicode characters, particularly when distinguishing between uppercase and lowercase Latin letters, although it worked for non-Latin characters. The new feature allows for a more straightforward and universally applicable way to define keybindings using Unicode code points.
Copy link
Member

Choose a reason for hiding this comment

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

This attempt at explaining reasoning makes me even less convinced that this feature should be there.

The example of the ; is a bad explanation because it dances around the point the only limitation before was a poor understanding of the core tenants of nushells string syntax.
That we accept strings without quotes in our record syntax is a courtesy we provide (and an attempt to transfer the logic for (external) command calls) and depends on being free of otherwise confusable characters, you can't include : or , in a record bare string either. If our default config would use strings with quoting for the keycode entries it wouldn't have come up.
We should strive towards making it easier for our users to grasp more of the transferrable base concepts of Nushell they can apply in many other situations than adding more things you have to understand to paper over things that were unclear before.

Saying that you get a benefit in the capitalization is also questionable, the capitalization of non-ASCII alphabetic characters already mattered as well as the capitalization of the ASCII alphabetics without modifiers (CTRL, ALT etc.).
Passing a specific capitalization for the ASCII alphabetics with added modifiers (even through their codepoint id) still doesn't matter because reedline will coerce the ASCII capitalization for the keys with modifier. For all intents and purposes, that we coerce here doesn't really matter if users follow the output of keybindings listen in setting up their keybindings. It shows which modifiers will be active and the resulting binding will work. (we could clarify the behavior when improving our documentation there, but being lax there doesn't have much harm if you want to enforce some binding with specific modifiers anyways.)

Adding things to our documentation that can be misunderstood or are a product of misunderstanding doesn't help. I would rather not have those two sentences.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your feedback. I can remove the two sentences as you've suggested. However, would it be acceptable to mention that the new ability to specify Unicode code points in keycodes can be useful for eliminating confusion in configurations where characters from different Unicode blocks look identical but have different code points? For example, several Cyrillic letters appear identical to Latin letters (cyrillic symbols: а, е, о, р, с, у, х, А, В, Е, К, М, Н, О, Р, С, Т, У, Х), but they have different Unicode values. By specifying the Unicode code points directly, users can accurately define keybindings for these characters without ambiguity.

Would including this explanation be appropriate, or would you prefer that I omit this as well?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah the character confusables sounds like the best motivation for using this feature to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely give an example of specifying Unicode code points. I agree that it's a key value prop here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

Choose a reason for hiding this comment

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

Awesome thanks!

@sholderbach sholderbach merged commit e55a24b into nushell:release-notes-0.99.0 Oct 9, 2024
2 checks passed
@JustForFun88 JustForFun88 deleted the release-notes-0.99.0 branch October 9, 2024 20:44
devyn added a commit that referenced this pull request Oct 16, 2024
* Release notes for `0.99.0`

Please add your new features and breaking changes to the release notes
by opening PRs against the `release-notes-0.99.0` branch.

## TODO
- [ ] look at interesting contributions
- [ ] write all the sections
- [ ] order the sections by interest
- [ ] add the breaking changes
- [ ] detail the breaking changes
- [ ] add the full changelog
- [ ] complete all the `TODO`s inside the release note
- [ ] ... (PRs that need to land before the release, e.g. [deprecations](https://github.com/nushell/nushell/labels/deprecation) or [removals](https://github.com/nushell/nushell/pulls?q=is%3Apr+is%3Aopen+label%3Aremoval-after-deprecation))

* std-lib breaking changes (#1578)

* std-lib breaking changes

* small typo

* Added changes to release notes due to nushell/#14020 (#1583)

* Start 0.99.0 release notes (#1586)

* hall of fame

* added full change log

* removed bump version change

* Finish 0.99.0 release notes (#1588)

* Fix typos

* Finish last PRs.

* TOC and excerpt

* Only Nushell changes

* Fix typo

* just add note about no plugin changes

---------

Co-authored-by: Douglas <[email protected]>
Co-authored-by: JustForFun88 <[email protected]>
Co-authored-by: Jack Wright <[email protected]>
Co-authored-by: Devyn Cairns <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants