microsoft/Terminal

Profiles Schema: Invalid command chords validate

Open

#2,967 opened on Sep 29, 2019

View on GitHub
 (0 comments) (0 reactions) (1 assignee)C++ (3,212 forks)batch import
Area-SettingsHelp WantedIssue-DocsProduct-Terminal

Repository metrics

Stars
 (35,764 stars)
PR merge metrics
 (Avg merge 27d 19h) (24 merged PRs in 30d)

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