Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hiding options #127

Closed
pb-cdunn opened this issue Oct 30, 2019 · 17 comments
Closed

Hiding options #127

pb-cdunn opened this issue Oct 30, 2019 · 17 comments

Comments

@pb-cdunn
Copy link

I'd like to be able to hide options from the user.

(The use-case is to maintain backwards-compatibility with another tool. I want to parse and ignore a large number of options that I do not need anymore.)

I see a field called suppress, but that removes the option completely. (And honestly, I don't see a use-case for that. If you don't want an option at all, you should remove it from your function signature. If that is impossible, then wrap your function.)

For now, I will have to keep the old options and just mark them as IGNORED.

@c-blake
Copy link
Owner

c-blake commented Oct 30, 2019

I just added a way to make them not make it to the help table..Make help[param] == "SUPRESS" - see release notes. Also, the use case of regular suppress is to allow Nim API calls to have parameters not exported to the CLI. While I realize that most people just ignore my advice to keep important functionality Nim importable/callable, I do try to make it easier when it is not hard for me to do.

@c-blake
Copy link
Owner

c-blake commented Oct 30, 2019

I realize the semantics aka "level of hiding" in this new feature may not be quite what everyone might want. It will still need a param, for example. It sounds like you may want something like a new zombie or fake or phantom param to dispatch. To turn your own "hack up your signature" approach on you, you can combine that hacked sig with dummy params and the new "SUPRESS" to get what you want..hidden options accepted/parsed including with whatever values but not in the help and unused by the proc dispatch wraps.

@c-blake c-blake closed this as completed Oct 30, 2019
@c-blake
Copy link
Owner

c-blake commented Oct 31, 2019

Oh, and to expand on the "level of hiding" and workarounds/shenanigans, one wrinkle is the "type" and parsability of arguments. To match syntax, vestigial bool options probably need to stay bool, but anything else could be string and the parse should always succeed. Unless you want the parse to fail for bad values to zombie options in which case you might leave their original type in place. And if you care more about those formerly bool options not having value parse errors than you care about being able to continue to participate in "same argv[] slot folding like -abc" then you'll have to make those string, as well.

To both preserve the syntax including bool/noarg or not, still allow use by a CL user but have that be a true no-op, and disallow help table presentation would probably require a new cligen feature. That's kind of a big list of desiderata, though -- especially for some "temporary phase" of a program's existence.

@pb-cdunn
Copy link
Author

I'll try this pretty soon. Thanks.

I think it should be "HIDE" instead of "SUPPRESS", so we don't overload the word "suppress".

@c-blake
Copy link
Owner

c-blake commented Oct 31, 2019

Hmm. I had been thinking I was economizing on pseudo-keyword-type entities, but you may be right. What was going through my head is help["param"]=="SUPPRESS" just "suppresses the help part", but nothing else. It's not really "hidden" from the parser. "NOHELP" might be better as being most explicit about what is going on. Of course, the CL author can change the string by setting clCfg.hTabSuppress. So, we're really only talking about the default (but that is a worthy topic). I'm tagging @luav since I added the hiding/help table suppression based upon his feature request in nim-lang/Nim#12425

@c-blake
Copy link
Owner

c-blake commented Oct 31, 2019

Oh, and I should say that if we are going to change it, it would be best to do so before I stamp the 0.9.41 release whenever that is. I don't know if @luav (or anyone) has even started using it yet.

@pb-cdunn
Copy link
Author

Either "HIDE" or "NOHELP" would be fine. Could you show a small example? I don't understand what "param" should be. (Note that I use multi-dispatch.)

@c-blake
Copy link
Owner

c-blake commented Oct 31, 2019

test/PerParam.nim shows how to use it. With dispatchMulti it would be wherever you are setting the help strings there. (There is not a way, presently, to have an phantom whole subcommand.)

@c-blake
Copy link
Owner

c-blake commented Oct 31, 2019

(I often say "param" to refer to the parameter of the API call being wrapped by dispatch.)

@luav
Copy link

luav commented Nov 1, 2019

test/PerParam.nim shows how to use it

@c-blake The example is pretty helpful but I would unlikely be able to find it without reading this conversation. It would be convenient to name the examples by their features, e.g. param_suppress-hide-nohelp.nim. Also, it would be helpful for (new) users to see a table (or sections in the manual / readme.md) of the library features with their respective examples, or at least a page rendering doc comments for the /test/ dir.

And one more thing concerning the parameter hiding implementation: it might be more safe to use string references allowing nil value, which could be a unique indicator for the parameter help hiding instead of some reserved word (SUPPRESS, HIDE, NOHELP) to prevent occasional collision with the actual content of the help string given by the user.

c-blake added a commit that referenced this issue Nov 1, 2019
@c-blake
Copy link
Owner

c-blake commented Nov 1, 2019

I agree there are many features. If you want to write a "locator table" then I would include it. To me the biggest obstacle (to any approach) is that there are a great many "synonyms" by which various people might look for some feature.

Nim used to have nil strings, but then got rid of them as less user friendly. So, I'm not sure others would agree with your idea. Instead, I changed the magic string to be "CLIGEN-NOHELP". If you are using that string to mean something else then you can probably handle the complexity of looking at the RELEASE_NOTES.md.

@c-blake
Copy link
Owner

c-blake commented Nov 1, 2019

(Oh, and if you didn't notice from the git commit, I made a new test/OmitInHelpTable.nim program.)

@c-blake
Copy link
Owner

c-blake commented Nov 1, 2019

I even show how to change the string if you want in that test/demo program now.

@pb-cdunn
Copy link
Author

pb-cdunn commented Nov 2, 2019

It works for me, but test/OmitInHelpTable.nim is wrong. As you say here, it must be CLIGEN-NOHELP, not cligen-NOHELP. (I tried both.)

@c-blake
Copy link
Owner

c-blake commented Nov 2, 2019

test/OmitInHelpTable.nim changes the default "CLIGEN-NOHELP" to "cligen-NOHELP" with a comment about the usual default on the change line. I'm not sure how you tried "CLIGEN-NOHELP", but both work in that test/OmitInHelpTable.nim (well, if you comment out the hTabSuppress setting the ALL-CAPS version works).

@pb-cdunn
Copy link
Author

pb-cdunn commented Nov 2, 2019

clCfg.hTabSuppress = "cligen-NOHELP" #default is "CLIGEN-NOHELP" -- Oh! Ok, I understand now. Nice.

@c-blake
Copy link
Owner

c-blake commented Nov 2, 2019

Cool. As mentioned above, if you keep the Nim types of your no-longer-used parameters the same then they will still have "the old errors" if some CL user gives them a bad value. That could be cast as either a bug or as a feature, but it's definitely the easier of the two implementation strategies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants