c-blake / cligen

Nim library to infer/generate command-line-interfaces / option / argument parsing; Docs at
https://c-blake.github.io/cligen/
ISC License
496 stars 23 forks source link

No interpolation of const values in a literal passed to `help` or `short` of `dispatch` #209

Closed ZoomRmc closed 2 years ago

ZoomRmc commented 2 years ago
import cligen

const aH = "foo"
let bH = "bar"

proc main(aaa: string; bbb: int) =
  quit()

when isMainModule:
  dispatch(main, help = [("aaa", aH), ("bbb", bH)])

Output of nim r test.nim -r:

...
  -a=, --aaa=    string  REQUIRED  aH
  -b=, --bbb=    int     REQUIRED  bH

Expected: compile time error for using variable bH, using "foo" for aaa help. Silently inserting names of the constants/variables is even more surprising.

c-blake commented 2 years ago

The inserting name thing is probably just some strVal "helpfulness". I have to evaluate/translate an expression tree that the macro gets delivered and that parseHelps work is..incomplete.

c-blake commented 2 years ago

Oh, also - as a corollary to evaluating/translating, things that are "semantically similar in Nim" are distinct as seen by a macro. So, {a:b, c:d} delivers a different syntax tree than [(a,b),(c,d)]. I can see that this relates to your https://github.com/c-blake/cligen/issues/208 , though and I am working on some solution for you.

ZoomRmc commented 2 years ago

So, {a:b, c:d} delivers a different syntax tree than [(a,b),(c,d)]. I can see that this relates to your #208 , though and I am working on some solution for you.

That's interesting, considering table literal doesn't exist in the language itself. However, in this case it makes no difference, the behaviour is the same.

c-blake commented 2 years ago

Actually, I might be wrong about delivering a different syntax tree. It may not be possible to de-sugar that any differently. Sorry - working on all your issues at once. :-D

c-blake commented 2 years ago

You need to change your let to const to make your code work, but that is the other error message issue...Still working on that, but presumably it is lower priority... :-)

ZoomRmc commented 2 years ago

You need to change your let to const to make your code work, but that is the other error message issue...Still working on that, but presumably it is lower priority... :-)

I put "let" there to show it's a case worth having an error for. Code works as shown, but shouldn't. It should work with a var changed to a const (see the "Expected" line).

The crux of the issue was const not being treated.

c-blake commented 2 years ago

Well, I took the crux to be the variable name instead of the string... :-) My just committed error message code does not seem to work as well on this snippet as the other issue one...Investigating.

c-blake commented 2 years ago

Well, I committed a check that should work ok. Let me know what you think whenever you get a chance.

ZoomRmc commented 2 years ago

Works perfectly. Thanks a lot!