RFR: Fix parsing of editor commandline in git-pr

Christoph Langer clanger at openjdk.java.net
Tue Feb 11 13:22:23 UTC 2020


On Tue, 11 Feb 2020 09:56:40 GMT, Erik Helin <ehelin at openjdk.org> wrote:

>>> Thanks Christopher! I agree, if it is possible to have quotation marks around the binary in `core.editor`, then we should support that. Looking at your patch, what do you think about doing something perhaps a bit simpler than a regular expression:
>>> 
>>> ```java
>>> var parts = editor.split(" ");
>>> if (parts.length > 0) {
>>>     var binary = parts[0];
>>>     // Binary might be wrapped in quotes on Windows
>>>     if (binary.startsWith(""") && binary.endsWith(""")) {
>>>         parts[0] = binary.substring(1, binary.length() - 1);
>>>     }
>>> }
>>> var pb = new ProcessBuilder(parts);
>>> ```
>> 
>> Hm, I agree that something simpler and more explicit than such a regexp would be desirable. But, as for your suggestion, what if the binary path contains blanks? Then it would be distributed along parts[0...n]. Furthermore, the options for the editor could also be quoted items with blanks in it. So, I guess the regexp would handle all that.
> 
> Ah right, my UNIX brain forgot about paths with spaces in them �� Yes, seems like we need the regular expression here. Just a quick question on the last part of the regex, `\\s*`, will that turn into groups? I wanted to make sure that we execute the editor _with_ all the arguments the user might have added.

> Ah right, my UNIX brain forgot about paths with spaces in them �� Yes, seems like we need the regular expression here. Just a quick question on the last part of the regex, `\\s*`, will that turn into groups? I wanted to make sure that we execute the editor _with_ all the arguments the user might have added.

The \\s* in the end will not turn into a group. It'll just lead to accepting trailing whitespace. I've tested the regex again in a small testprogram with several variations in input. One thing I've added is a \\s* in the beginning to accept leading whitespace - just in case. So the regex basically is:
-optional leading whitespace(non-quoted token|quoted token)optional trailing whitespace-
The group is just formed out of the token.

PR is updated, hopefully the final version :)

-------------

PR: https://git.openjdk.java.net/skara/pull/259


More information about the skara-dev mailing list