Allow ',' at the start of OCAMLRUNPARAM #9634
Conversation
| @@ -135,6 +135,7 @@ void caml_parse_ocamlrunparam(void) | |||
| case 'v': scanmult (opt, &caml_verb_gc); break; | |||
| case 'w': scanmult (opt, &caml_init_major_window); break; | |||
| case 'W': scanmult (opt, &caml_runtime_warnings); break; | |||
| case ',': continue; | |||
| } | |||
| while (*opt != '\0'){ | |||
| if (*opt++ == ',') break; | |||
There was a problem hiding this comment.
I don't think we need this while loop anymore, then?
There was a problem hiding this comment.
Well, if I understand correctly, the scanmult call does not update opt, so we still need to skip over the =... part read by it before going on to the next item (which is what the while loop is doing).
There was a problem hiding this comment.
Ah, I had misunderstood indeed. I thought that we were only skipping commas here.
There was a problem hiding this comment.
Approved. This seems minor but I think it could benefit from a Changes entry, in case users have been bitten by this issue before.
@nojb justifies skipping the first comma with the idiom FOO=$FOO,bar, but note that skipping middle commas (also implemented) is also useful to allow the idiom FOO=$FOO1,$FOO2,$FOO3.
|
(Is there no test for OCAMLRUNPARAM parsing in the testsuite?) |
Well, the code would have been exercised by the tests in #9633, but they were broken :) Am not aware of any other test using |
|
This is nice! Thanks! Perhaps the documentation should be updated, too? I'd definitely put everything in only one commmit. |
|
Manual updated. |
|
Merged. |
|
Thanks! |
|
Nicolás Ojeda Bär (2020/06/04 13:32 -0700):
Manual updated.
Thanks!
|
|
Gabriel Scherer (2020/06/04 22:12 -0700):
Merged.
Thanks. To be honest I find things tend to go really fast these days.
The last change on that one (minor, I agree) happened yesterday very
late and the merge rather early this morning. It does not leave people
much time to review and I find it a bit a pity.
|
Currently the idiom
OCAMLRUNPARAM=$OCAMLRUNPARAM,b=1doesn't work if$OCAMLRUNPARAMis empty, as theOCAMLRUNPARAMparsing code is not able to handle the initial comma. This PR makes it so that any number of initial commas, or repeated commas in general are skipped over.