microsoft / microsoft/terminal
Profiles Schema: Invalid command chords validate
@cinnamon-msft is already working on this.
Since Sep 30, 2019.
- 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?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.