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

Set the terminal width in clCfg #179

Closed halloleo closed 3 years ago

halloleo commented 3 years ago

Following up on #176, I'd like to check whether there is a general way to set a (maximal) terminal width in clCfg? (Either in the Nim code via the clCfg structure or via the ~/.config/cligen/config file.)

c-blake commented 3 years ago

First, it bears mention that CLI authors cannot control the vertical scrolling on terminals. So, I do not think it's outside the general model to not let them control horizontal scrolling.

That said, let's discuss what might solve your problem. I think a terminal width override is more idiosyncratic than either a static compile time setting or even a user config setting. Not all users have all their terminal emulators with the same widths (or keep them the same over time). Just hammering the doc string lets you do the static already anyway. Philosophically, I think things like this (and colorization) should really be end-user choices.

Furthermore, at least on Unix-like, shells usually provide a way to trap a SIGWINCH. An easy thing to change inside a shell trap function is an environment variable. $COLUMNS and $LINES are usually already changed for you.

So, what seems like might solve your problem and maybe that of others is maybe a $CLIGEN_WRAP_WIDTH variable. If set that is just taken for textUt.wrap|alignTable purposes no matter what other things say. I could potentially also just re-use $COLUMNS, but it sounds like you have some very fancy terminal setup where even that might not be right. If done like this, it might be even better to add a static/compile-time setting for what the environment variable name is. (This would be a ClCfg addition.)

Now, I just call terminal.terminalWidth in the stdlib (yes, I did write that for the stdlib, but there is still a real question of scope). So, if you (or others) have this problem with the cligen notion of terminal width, well, maybe this should be a patch to the Nim std/terminal? At the moment std/terminal.terminalWidth falls back to $COLUMNS, but it could, e.g. let $COLUMNS override other settings (or $NIM_TERM_WIDTH/global var setting in terminal.nim or even the env var name could be passed as a string to terminalWidth). Or it could even take some new enum specifying what the priority/order of authority for such information should be. So, I think in a perfect world this kind of configurability should be part of the Nim stdlib.

That said, I realize it's often a PITA to get fine-grained things like this into the stdlib and I tend to be more accommodating. So, let me know what you think the best way forward is in light of all these considerations.

halloleo commented 3 years ago

Hey @c-blake, super-thanks for the detailed explanation! :-)

Hammering the doc string only "solves" the wrapping issue for the doc string but not for option help, etc. That's why I had given up on it.

In most shells(*) the end user has certainly an option to use stty cols to set things as desired. I still think the CLI tool creator should have her/his say for a maximum line width to match the regular output of the CLI tool.

$CLIGEN_WRAP_WIDTH would be fine - after all the CLI tool creator can set in tin the nim program before the dispatch, right?


(*) For very annoying problems in the xonsh shell with stty see Why does stty cols 40 have different effects in the two shells bash and xonsh - Stack Overflow.

c-blake commented 3 years ago

A CLauthor has no idea how narrow or wide the CLuser's terminal emulator window is or why. The CLuser may have several widths - 64, 80, 128, all in play at the same time or maximize some window on demand. Wanting each to wrap the same way/behave the same way seems wrong-headed. Using only 64 of 128 available text columns might please you from a run-to-run output consistency point of view, but it is also not very friendly to a CLuser who specifically uses some wide terminal to get less wrapping. You cannot have both statically decided and dynamically flexible widths at the same time.

That said, yeah..I'll add an env.var and yes, you should be able to override what users set that environment variable to statically/at compile-time value before dispatch. I'll say again, I think that is ill-advised/user-hostile and I would encourage you to not clobber what other CLusers may want in the name of always-the-same formatting. The best approach is probably to set a default (by checking if the env.var is set before you clobber it with your personal preferred width).

c-blake commented 3 years ago

Oops. I left the issue number out of my commit message.

So, while I think doing any putEnv override should not really be in your static source code, if you really want to put in a default CLusers can override then you might do before dispatch:

putEnv("CLIGEN_WIDTH", getEnv("CLIGEN_WIDTH", "80"))

where you can change 80 columns to whatever you want to set the default. Then CLusers can set it if they want a different default.

I put the name of the environment variable updaters in my default config file parsers. So, if you are kind enough to your CLusers to allow them to use a config file, then they can actually "sneak around" the above code snippet by changing name to, e.g., "MY_CLIGEN_WIDTH". You could probably use clCfg.widthEnv instead of the string literal in your putEnv(., getEnv()) to adapt to that.

Really I think you should do nothing in your static code before dispatch() and just put CLIGEN_WIDTH=whatever in your personal shell .rc files and let CLusers do the same if they want. Why, if you do not override things on them, they could even have a trap for SIGWINCH that set CLIGEN_WIDTH to say, $((COLUMNS-3)) so that there was always a 3 column "right margin" and similar.

halloleo commented 3 years ago

Cool. I've just updated nimble to cligen@#head and it works pretty well. Thanks for your very quick implementation!

One question: Why did you decide to introduce CLIGEN_WIDTH as an env var and not directly as a ClCfg field?

c-blake commented 3 years ago

Answer: because, as mentioned quite a few times by now, my philosophy is strongly that the end CLuser, aka the reader, should have final say over presentation of help. The Clauthor can control the name of the environment variable. It seems you may benefit a little more from my bias that you seem to be fighting against of giving CLusers control of help presentation.

A Clauthor just cannot know many relevant things unless he knows every single user. Are they old needing giant fonts/few terminal columns? Fully color blind? Only red-green color blind? Work on "night mode" terminals or white background? Both at the same time. Is happy to maximize windows on demand or hates that? Etc., etc. CLuser emotions often "run hot" when asking for help in the first place (how the heck do I make this program do what I want!!?!). Best not to add to their documentation seeking frustration by blocking them from control over how it is presented (how I even supposed to read this? I have impairment or personal issue XYZ!).

Meanwhile, on the other side, if a CLauthor is their only user (as is probably not so rare in Nim and surely the case for me personally for many of my dozens of cligen programs) then the user must also be a computer programmer of some kind. For such, having to configure both ~/.config/cligen/config and the code is not a "big ask".

A middle ground might be that a CLauthor personally knows all their users. Then they can all just negotiate like peacable peoples do. :-)