🐛 Fix highlighting of optional variadic argument metavars#1508
🐛 Fix highlighting of optional variadic argument metavars#1508BenjyWiener wants to merge 3 commits intofastapi:masterfrom
Conversation
|
I was taking another look, and I'm wondering if there's a specific reason the opening and closing brackets are only matched at the beginning/end of the string. If not, it might make more sense to just remove the |
|
The dots indicate that multiple arguments are accepted (hence the |
| @pytest.mark.parametrize("input_text", ["[ARGS]", "[ARGS]..."]) | ||
| def test_metavar_highlighter(input_text: str): |
There was a problem hiding this comment.
This test succeeds on master with the normal [ARGS] but fails with [ARGS].... Succeeds with this PR.
svlandeg
left a comment
There was a problem hiding this comment.
Confirmed the bug though it was hardly noticeable on my console 🥲
To write a regression test for this, I decided to move MetavarHighlighter to the module level, I think it's nicer anyway to define it together with the other highlighters. Then we can test for the highlighting directly, which works fine without the 3 dots but indeed fails as the regex doesn't consider the possibility of seeing 3 dots before the end of the string.
As @BenjyWiener mentioned, another option would be to remove the $. It just feels that these start and end markers are put in the regex on purpose, so I think the current solution is fine.
| highlights = [ | ||
| r"^(?P<metavar_sep>(\[|<))", | ||
| r"(?P<metavar_sep>\|)", | ||
| r"(?P<metavar_sep>(\]|>))(\.\.\.)?$", |
There was a problem hiding this comment.
Tiangolo: notice how this code was moved, but also changed to add the optional 3 dots at the end of the regex.
Due to a mistake in one of the
metavar_sepregexes, a closing bracket followed by '...' does not get the correct highlighting in the arguments help panel.Minimum reproducible example:
Current
--helpoutput (note the closing bracket of[ARGS]...in theArgumentspanel):With this PR: