Fixe arg parsing when one-character argument is followed by ;

[serd2011]

Summary of the Pull Request

Changes the way _addCommandsForArg determines if the delimiter was at the beginning of the argument so that it accounts for the fact that match includes the last character of the string before it.

PR Checklist

• Closes --profile: 1 required TEST missing #13277

Validation Steps Performed

wt -p "u"; nt -p "u" does not cause an error

[zadjii-msft]

I'd love a test for this in src\cascadia\LocalTests_TerminalApp\CommandlineTest.cpp, but the LocalTests are kinda finicky to get running so I won't cry if this merges without one.

[serd2011]

Would it be ok add them to ParseTrickyCommandlines or ParseSimpleCommandline?
Is testing that one-character argument parsed correctly enough or are there more test cases that need to be added?

[zadjii-msft]

ParseTrickyCommandlines seems fine to me ; the wt -p "u"; nt -p "u" test case seems good enough of a test to me 😄

[lhecker]

(Future improvement:) Looking at _commandDelimiterRegex, I get the feeling that this code would be a lot more robust if we'd just find(L';') and check if the preceding character is != L'\\'.

[DHowett]

(Future improvement:) Looking at _commandDelimiterRegex, I get the feeling that this code would be a lot more robust if we'd just find(L';') and check if the preceding character is != L'\\'.

Yes and no.

What if the character before the \ is \? \\; is not the same as \;!

[lhecker]

What if the character before the \ is \? \\; is not the same as \;!

The regex is currently (^;|[^\\];) and described as

Either a ; at the start of a line, or a ; preceded by any non-\ char.

There's nothing in the regex that would handle such a case. If we handle it, it must be elsewhere or not at all...

[DHowett]

This vaguely worries me, actually. Why are we using a regex to match a character followed by ; when what we want to know is whether there is a ;? Is it just because we need to match anything except a \?

Leonard's right, we should be using a more direct/implementation-specific string parser rather than a regex. It would give us control over exactly when we match and when we "vend". :)

[ghost]

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[ghost]

🎉Windows Terminal Preview v1.16.252 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download