microsoft/Terminal

Profiles Schema: Invalid command chords validate

Open

#2,967 opened on 2019年9月29日

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
 (平均マージ 27d 19h) (30d で 24 merged PRs)

説明

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

コントリビューターガイド