RFR: Fix parsing of editor commandline in git-pr

Christoph Langer clanger at openjdk.java.net
Tue Feb 11 07:44:13 UTC 2020


On Tue, 11 Feb 2020 07:37:21 GMT, Erik Helin <ehelin at openjdk.org> wrote:

>> Hi,
>> 
>> finally I got back to this.
>> 
>>> As for the regular expression, could you please add a comments above with a few examples.
>> 
>> I've done that.
>> 
>>> Also, do you really need the quotation marks around the path to `notepad.exe` for `core.editor`? Won't the following suffice:
>>> 
>>> ```
>>> core.editor = C:\Program Files\Notepad++\notepad++.exe -multiInst -notabbar -nosession -noPlugin
>> 
>> Probably yes, but that's how I found my git configuration without manual changes that I'm aware of. So it should work. And I think, even if I remove the quotation marks, it still wouldn't work.
>> 
>> Please review :)
> 
> 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:
> 
> 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);

> 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.

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

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


More information about the skara-dev mailing list