c-blake / cligen

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

Problem displaying hyphens instead of underscores #100

Closed pb-cdunn closed 5 years ago

pb-cdunn commented 5 years ago

I've tried a few ideas without luck:

 19           help = {
 20             "aln-fn": "BAM alignment, sorted on 'coordinate'",
 21             "ref-fn": "FASTA reference",
 22           },
        ... ../../.git/NIMBLE/pkgs/cligen-0.9.19/cligen.nim(68, 15) Error: main has no param matching `help` key "aln-fn"
 19           help = {
 20             "aln_fn": "BAM alignment, sorted on 'coordinate'",
 21             "ref_fn": "FASTA reference",
 22           },
  --aln_fn=      string  REQUIRED  BAM alignment, sorted on 'coordinate'
  --ref_fn=      string  REQUIRED  FASTA reference

I want

--aln-fn= ...
--ref-fn= ...

I think I'm using the tip of master.

pb-cdunn commented 5 years ago

Oh!

  -a=, --aln-fn=  string  REQUIRED  BAM alignment, sorted on 'coordinate'
  -r=, --ref-fn=  string  REQUIRED  FASTA reference

But I get underscores when I drop the short-flags:

short = {"": '\0'},

Weird!

c-blake commented 5 years ago

Slightly less weird if you look at the git log -p. ;-) I have to switch off the help row formatting depending on whether there is a short option (probably for obvious reasons - no "short=,"). This bug should also have happened if you had a first-char collision and the long option which missed out on a short had an '_' in it. Turning off all shorts just makes them all miss out.

pb-cdunn commented 5 years ago

No solution?

c-blake commented 5 years ago

test/PerParam.nim works for me. You have to use the _ version in the help keys, but the CLI user visible stuff should be kebab case.

pb-cdunn commented 5 years ago

Yeah, it works for me too, but not if I remove all the short options. Is that what you see too?

pb-cdunn commented 5 years ago

https://github.com/c-blake/cligen/blob/master/test/PerParam.nim

yes, that works for me.

I guess I live with extra short options. Not really a problem.

c-blake commented 5 years ago

You can still drop the short options. Your version of cligen just needs to include the fix for this issue ( https://github.com/c-blake/cligen/commit/c0e07eb08b09185055a4aaa1771179ac77fbefb0 ). What I meant by test/PerParam.nim working for me was if I hack it to say '\0' instead of 'z' for "al_pha" then I get the kebab case version when running --help. Here, I'll go ahead and add a backdoor test case.

Right now matching of parameter names and long option keys is all based on the literal spelling/casing in the Nim code. So, you do have to specify them with underscores in both the parameter list and in the help table.

There are quite a few places in the dispatchGen interface where CLI authors give the names of parameters (help and short keys, positional, suppress, mandatory, implicitDefault). None of them are really doing normalized identifier comparisons. They should probably be "just-Nim"-normalized rather than optionNormalize()d since the kebab case variant is not allowed in Nim code. The Nim style insensitivity kind of doubles up work for me, and then the new helpCase triples it up in some places. You need the spelling used for compares, for presentation as compiler errors, and now for presentation to CLI users.

Anyway, ArgcvtParams should maybe grow a parNorm field and helpCase should maybe only be called from within cligen.nim. That might simplify CLI author-override of whatever our default helpCase rules are. (Not everyone hates snake_case.) Then helpCase could be just like mergeParams - a proc the CLI author can define anywhere between import cligen and dispatch - instead of needing to be in a separate include file with Nim search path shenanigans. I hadn't really settled on that, but wanted to get you your feature quickly. I should fix this before I punch a release. (Doubt I'll fix all the unnormalized ident comparisons in those 6 places, though.)

c-blake commented 5 years ago

Oh, and while I only updated test/PerParam.nim to test the single dropped short option case, I manually tested short={ "": '\0" } to drop all of them and the kebab case help seemed to work there for me, too.

pb-cdunn commented 5 years ago

Wunderbar! That worked. Sorry, I didn't notice your fix. You're too quick!

c-blake commented 5 years ago

Ok. Glad to hear it's working for you.

pb-cdunn commented 5 years ago

Can you remind me how to get dashes instead of underscores in help? (I thought that would be the default, no? --help-syntax is correct.)

./pb consensus -h
consensus [optional-params]
  Options(opt-arg sep :|=|spc):
  -h, --help                                    print this cligen-erated help
  --help-syntax                                 advanced: prepend,plurals,..
  -m=, --min_cov=               int     6       minimum coverage to break the consensus
  --min_cov_aln=                int     10       ...

It's this way whether I drop short or not. I'm at 6c7e08c.

pb-cdunn commented 5 years ago

Oh! I see. I need to specify help messages with dashes on the option-name:

"min-cov": "minimum coverage ..."
c-blake commented 5 years ago

Yeah. 2nd bullet in RELEASE_NOTES.md for v0.9.29.