std/cmdline.commandLineParams NimScript fix#25573
std/cmdline.commandLineParams NimScript fix#25573ZoomRmc wants to merge 1 commit intonim-lang:develfrom
std/cmdline.commandLineParams NimScript fix#25573Conversation
Now consistently returns only the script arguments, skipping compiler arguments.
| result = @[] | ||
| for i in 1..paramCount(): | ||
| result.add(paramStr(i)) | ||
| when defined(nimscript): |
There was a problem hiding this comment.
The compiler as it evaluates paramCount and paramStr at compile-time should do these fixups.
There was a problem hiding this comment.
Well, maybe it should but it doesn't. These procs are builtins for NimScript, as far as I see. Not sure what you want.
This code is not new, it's just moved copied from parseopt to a more fitting place.
There was a problem hiding this comment.
I want you to fix the root cause which is in compiler/scriptconfig.nim, lines:
cbconf paramStr:
setResult(a, os.paramStr(int a.getInt 0))
cbconf paramCount:
setResult(a, os.paramCount())
There was a problem hiding this comment.
First of all, I wouldn't know how to, I found the source but I'm not very confident at that level.
Secondly, wouldn't this break scripts inspecting the command line launched intrinsically from config.nims on compilation?
# in config.nims:
for i in 1..paramCount():
if paramStr(i) == "-foo":
raise newException(Defect, "Build with 'foo' is not supported")|
So, continuing the discussion above. As we have an additional case of implicitly run scripts, I think it would be beneficial to leave the The |
|
@Araq I'd like to proceed with this, but I think my concerns above are legit. To repeat myself, two things need clarifications:
|
cmdline.commandLineParams()now returns only the script arguments, skipping compiler arguments.Closes #23554
The logic is extracted from
parseopt.initOptParserand is now duplicated. Though there it triggers only when there are no arguments provided so double reads are an edge case:Read
commandLineParams()from a NimScript and get none -> pass them explicitly toinitOptParser-> Whole argv reread again and stripped.Should be fixed if #25571 or an alternative is merged.