microsoft / microsoft/terminal

Profiles Schema: Invalid command chords validate

Open
#2,967 0 comments 0 reactions 1 assignee View on GitHub

@cinnamon-msft is already working on this.

Since Sep 30, 2019.

Area-Settings Help Wanted Issue-Docs Product-Terminal
Dominant language
C++
Stars
105k
Forks
9.6k
Avg merge
3d 17h
Merged PRs (30d)
29

Description

First of all, I'm very happy to see that parts of the schema that I suggested in #1003 made it into a release, but in the review process I think that an important feature of the (allegedly, very long) regular expression to validate the keys strings was lost: It will now incorrectly validates key chords that won't parse properly. This is because the strings recognized in KeyChordSerialization.cpp are not spelled out explicitly.

May I suggest that the expression is changed to (the <key> group is new, and the last + in <modifier> is required):

^(?<modifier>(ctrl|alt|shift)(?:\+(ctrl|alt|shift)(?<!\2))?(?:\+(ctrl|alt|shift)(?<!\2|\3))?\+)?(?<key>[^\s+]|backspace|tab|enter|esc|space|pgup|pgdn|end|home|left|up|right|down|insert|delete|numpad_(?:[0-9]|multiply|plus|minus|period|divide)|f[1-9]|f1[0-9]|f2[0-4]|plus)$

Note that the problem with my original expression is fixed - it now allows single |} etc. keys, as it should. However, it will not validate ctrl++ or ctrlplus, which should be ctrl+plus instead. Also, modifiers without keys will not validate (e.g. ctrl).

Another improvement would be to list out the allowed words in the description, of course. This is a tricky value to get right without documentation as a crutch, but at least proper validation can help you catch errors early.

Maybe @oising, who improved on the original regexp, would like to comment?

Edit: Link to example at Regex101

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.