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

CHG: Permit flags as both prequel or sequel of positional arguments #123

Closed
wants to merge 1 commit into from

Conversation

cjbrigato
Copy link

This make flags:

  • usable either before, after, or both sides of positional arguments
  • breaking if one appears in between two positional arguments
  • repeatable as they'll just overide themself, so there is no problem having same flag used in both sides of positionals.
  • '[options]' in usage a prequel of positional in help message (more often seen in th wild, more natural to some, arguable to many)

Also, this PR change behavior of missing positional argument by exec'ing self plus -h after original error
(This is because it was found really frustrating for discovery that a subcommand with positional argument would be discoverable by iterating over errors or forcing a -h,
a-contrario of nearly all other case that bashly generates.
)

Note :

  • these changes are known to be quite opiniated.
  • The very high robustness and coherence that bashly shows in behavior and code probably makes this PR unable to qualify for further treatment
    More than enough to not take any offense in case this PR is thrown away without any other care.
    (Deep thanks for your very valuable,trustable and elegant work.)

- makes '[options]' in usage a prequel of positional in help message (more often seen in th wild, more natural to some, arguable to many)
- without breaking their original behavior

This make flags:
- usable either before, after, or both sides of positional arguments
- breaking if one appears in between two positional arguments
- repeatable as they'll just overide themself, so there is no problem having same flag used in both  sides of positionals.

Also, this PR change behavior of missing positional argument:
- original error message is kept
- then a full help for subcommand is printed (exec fallback to `$0 $action -h`

This is because it was found really frustrating for discovery that a subcommand with positional argument would be discoverable by iterating over errors or forcing a -h,
a-contrario of nearly all other case that bashly generates.

(Note :
- these changes are known to be quite opiniated.
- The very high robustness and coherence that bashly shows in behavior and code probably makes this PR unable to qualify for further treatment
For these reason, I would not be offended if this PR is thrown away without feedback)
@DannyBen
Copy link
Owner

Thank you Colin for this PR and for your kind words.

I would need to take a deeper look, and if we will be moving on with this change, fix the current tests and implement new ones for the new use cases.

@DannyBen
Copy link
Owner

Also - I do not understand this comment of yours:

This is because it was found really frustrating for discovery that a subcommand with positional argument would be discoverable by iterating over errors or forcing a -h,
a-contrario of nearly all other case that bashly generates.

Can you give me a step by step reproduction, so I can see why it is frustrating?

@DannyBen
Copy link
Owner

DannyBen commented Oct 15, 2021

I tested the PR locally, and many of the basic bashly tests fail, for the right reasons. This PR as is, breaks some things.

I am also not a big fan of showing [options] before the positional arguments, to me it feels much less natural. I always look at a command line, like a sentence in English. download SOURCE TARGET [options], and also, "optional" is semantically less important than the required, so first required, then optional, makes more sense from cognitive perspective, in my opinion of course.

@cjbrigato
Copy link
Author

cjbrigato commented Oct 15, 2021

This is what I meant when I said opinionated. It's even more a case of precedence than opinion on my side, since semantically your way actually feels more natural. Still it won't clear strong kinetic memory and continues to be a bit of pain. Might be the time to break some habits for me.

And since it breaks much test case, let's throw this away.

Thanks for your time :)

(I'm closing with a longer comment on the "frustration" thing, in case it is of interest)

However, after trying again the scenario, definitely this is heavy frustration to have no help/usage fallback on empty/erroneous usage of complex case. (Warning : This is definitely a case of Golang ecosystem induced frustration; yet might be interesting)

e.g:

cbrigato@ci:~/frusty$ cat src/bashly.yml                                                                                                                                                                                                                                        
name: frusty                                                                                                                                                                                                                                                                    
help: Apex of frustiness inner core essential self awareness                                                                                                                                                                                                                    
version: 0.1.0                                                                                                                                                                                                                                                                  
                                                                                                                                                                                                                                                                                
commands:                                                                                                                                                                                                                                                                       
- name: benchmarks                                                                                                                                                                                                                                                              
  short: b                                                                                                                                                                                                                                                                      
  help: common benchmark scenario for our super project                                                                                                                                                                                                                         
  commands:                                                                                                                                                                                                                                                                     
  - name: microservicea                                                                                                                                                                                                                                                         
    short: msa                                                                                                                                                                                                                                                                  
    help: msa oriented benchmarks                                                                                                                                                                                                                                               
    commands:                                                                                                                                                                                                                                                                   
    - name: jobs                                                                                                                                                                                                                                                                
      short: j                                                                                                                                                                                                                                                                  
      help: benchmark /jobs endpoint (critical for e.g.  BO,Mobile APP...)                                                                                                                                                                                                      
      args:                                                                                                                                                                                                                                                                     
      - name: status                                                                                                                                                                                                                                                            
        required: true                                                                                                                                                                                                                                                          
        help: status of jobs for filter                                                                                                                                                                                                                                         
        allowed: [job_done,job_canceled,job_in_progress,pending_realization,pending_answer,pending_payment]                                                                                                                                                                     
      - name: per-page                                                                                                                                                                                                                                                          
        required: true                                                                                                                                                                                                                                                          
        help: per page job recollection limit                                                                                                                                                                                                                                   
      flags:                                                                                                                                                                                                                                                                    
      - long: --users                                                                                                                                                                                                                                                           
        short: -u                                                                                                                                                                                                                                                               
        arg: users                                                                                                                                                                                                                                                              
        required: true                                                                                                                                                                                                                                                          
        help: quoted or escaped, space separated list of users mail that will run each test (for jwt-dependant cases)                                                                                                                                                           
      - long: --page                                                                                                                                                                                                                                                            
        short: -g                                                                                                                                                                                                                                                               
        arg: page                                                                                                                                                                                                                                                               
        help: an offset of "per-page" unit base that we often see alongside pagination concepts \:)                                                                                                                                                                             
        default: 1                                                                                                                                                                                                                                                              
    - name: incidents                                                                                                                                                                                                                                                           
      short: i                                                                                                                                                                                                                                                                  
      help: benchmark /incident endpoint (critical for e.g QA, partners...) 

#this is example case of utility that CI and QA/humans will use, we implement shortcuts for scenarios of end to end cases and benchmarks (let say)

when being even the least profane of the specific cases, there is deep contrast from start to end of discovery, or usage:

  • not aware of hidden flags that are required if we don't rely immediately or soon enough to full help
  • help subcommand is no alias of subcommand -h
  • no "global flags" avaible
  • no relation of overidding name-reated arguments / avoiding "required" arguments by usage of environment variables (or i failed to find how to pass an arg requirement by providing an environment variable within bashly scope)

Common scenarios of frustration we received for feedback are nearly all giving the impression that bashly is at same time producing "so much user friendly" cli while suddenly lacking "very common features". I must say this is quite uncommon to have this contrast observable.

if ever you got time, that asciinema shows pretty much how our guys use cli software (iterative discovery is always expected, even in failing case)

Asciinma Demo

if we compare with docker, for example, which is the case that probably makes much precedence, these pain points are never felt. It is quite hard to "fail" a docker command or have to rely on manually going to full help.

@DannyBen
Copy link
Owner

Well - thanks for the asciinema demo, it helped me understand your flow, and there are a few points I want to mention:

  1. I agree, this current PR is not ready and combines several different concerns.
  2. I see a case for allowing the options to precede the positional arguments. I have opened Allow flags to precede required positional arguments #124 to track this.
  3. Although I do not want to argue with someone's frustration - I observed that at least a part of it could have been avoided by not being afraid to explicitly ask for help, instead of continuing to guess wrong. If you through some cli command subcommand -h here and there, your workflow will be much more fluent.
  4. In addition - although bashly can support a complex set of subcommands - from user interface perspective, perhaps it wouldn't be a bad idea to consider separating to different CLI tools. The beauty of bashly, is that you are only writing your own code - everything else comes for free, so separating to different CLIs might be easier.
  5. In regards to having ./cli help COMMAND available out of the box: It is by design that bashly tries to not impose too many rules. It is a top priority for me to keep bashly simple, both in terms of how long it takes for users to learn to work with it, and how complex is the code base. However, adding this functionality on your own is trivial (demonstrated below).
  6. As for your expectations from environment variables (assuming I got it right), the reason environment variables are not automatically associated with arguments, is that it is not always the desired effect, and if it is, it is quite easy to achieve it. Also example below.

Adding cli help COMMAND

# In your src/bashly.yml
commands:
- name: help
  short: h
  help: Show a help for a command

  args:
  - name: command
    required: true
    help: Command to show help for
# in your src/help_command.sh
cmd="${args[command]}"
./cli "$cmd" --help

# or, call the internal function directly
long_usage=1
"cli_${cmd}_usage"

Allowing argument to be provided with an environment variable

# In your src/bashly.yml
environment_variables:
  - name: APIKEY
    help: Your API key

flags:
  - long: --apikey
    short: -a
    arg: APIKEY
    help: Your API key (can also be provided with API_KEY environment variable)
# in your src/*_command.sh
apikey="${args[--apikey]:=$APIKEY}"
inspect_args

# You can add any validation here or reverse the order, or do whatever
# is desired by your use case

Now all that said - if you still feel that some features can be improved to reduce your frustration level, I suggest you open a separate issue (one for each suggestion), and outline the suggestion as clearly as possible, hopefully with a sample minimal YAML and an "expected" and "actual" behavior, and we can discuss each separately, and perhaps find ways to improve.

I hope it makes sense, and that I haven't missed any of your issues.

@cjbrigato
Copy link
Author

@DannyBen I have no word to qualify how incredible and unexpected is your answer, not even trying to tell how much precious and rare material and attitude you're delivering here.

I see a case for allowing the options to precede the positional arguments. I have opened Consider allowing flags to precede the positional arguments #124 to track this.

This is highly appreciated, but It definitely makes me curious about the case that you are seeing there that is enough to justifiy this feature : As insightful was your first semantic-powered argument, I'd definitely hear your thought once again :)

3 , 4 , ...

This whole PR shows that what I needed was maybe more an issue than anything else.
But i'm very glad It happend, because the simplicity of implementing these feature in your example is hitting hard enough to make definitive adoption of bashly (and most of your toolset, in fact, as they nearly all show the same consistent, pleasant, and easy to understand paradigm, up to the elegance of the underlying ruby code).

In the end, only limitation is the flags position, everything else was available by using bashly the right way.

About the frustration... I have none, this was a bit of theatrically enhanced impersonation of a some end users here.
But definitely you're right : " it could have been avoided by not being afraid to explicitly ask for help"
This was my very point : Most of the crew there seems to be expecting that any failure should be immediately compensated by pertinent advice and insight. The "hidden" nature of the required flag, it's position, in contrast with the rest of the flow is enough to break their try and error iterations to the point that there was real minute of nervousness while not being able to immediately recall that -h flag is the route to go. They are not that afraid to ask for help. They are used to being driven somewhere explicit , hopefully new and informative at every iteration, be it success or failure.
I like the reminder this story was on these bad habits we have.

Thanks again, definitely looking for #124 !

@DannyBen
Copy link
Owner

Warning: Wall of text ahead


Colin, first of all, thank you for your kind words. For me, it is a great pleasure discussing features and expectations with someone like you.

On top of that, even if not all requests can be easily implemented, at least I learn what is it that people are looking for, and you hopefully learn why they are missing, and how they can be overcome with a slight change of perspective or design.

but It definitely makes me curious about the case that you are seeing there

Well, originally when starting to develop bashly, I wanted to keep things simple, since as you can imagine, generating a valid bash script from Ruby already had its challenges. This is why I kept the command line parsing logic simple - it was (and still is) easier to first consider positional arguments, and only afterwards, parse the rest.

The reason I said that I can understand the reasoning behind the desire to have support for options before positional arguments, is that this is - as you mentioned - mostly the standard in other command line frameworks, so if we can support it without complicating the code so much, it is a good feature to have.

... and most of your toolset

I am curious - which other tools your are considering for your workflow?

this was a bit of theatrically enhanced impersonation of a some end users here.

Heh :) - I see.

Well - I can totally relate. Not everybody is using the command line in the same way, and it helped me see another way that people may be expecting it to work.

Now, although I always strive to have my tools behave as people expect them to, at the end of the day - as I am sure you know - its a balancing act between the code complexity and the benefits of the feature.

I am sure that bashly does not solve all problems, but I hope that at least it provides the ability to develop bash scripts that almost feel like real command line tools.

Most of the crew there seems to be expecting that any failure should be immediately compensated by pertinent advice and insight

As I understand, you are developing these scripts for other colleagues to use.

If this is indeed the case, then I would like to reiterate a few recommendations that I am implementing with my team:

  1. Develop small, single-concern tools, rather than a large bash script that handles all of your team's needs. This is much easier to maintain, and to test.
  2. If you are concerned about how to distribute these tools to your team, and you don't have any other solution - I can tell you I am using rush - which is a bash script (developed with bashly of course) that lets you run scripts (think of it as "install packages") from a GitHub repo. So your users can just run rush benchmark and get the latest version of your benchmark "package".
  3. If you are like me, obsessive about testing and want to test your bash scripts, you might want to take a look at approvals.bash which is a tiny approval-testing tool I am using for all my bash tests.
  4. Lastly - if you haven't discovered this bashly feature yet - you should know that you can generate autocomplete for your scripts, which might make the discovery process a little more friendly.

Thanks again, definitely looking for #124

Sure thing, my pleasure.

Just keep in mind that I am not yet sure that #124 can be easily developed. As we discovered, just moving the required_arg_filter lower in the chain, did not solve the problem. I promise to try to solve it elegantly at least 3 times before giving up, but I cannot promise I will succeed. :)

If you have other thoughts or issues, or even if you just want to share a success or failure story of your generated scripts, feel free to open new tickets.

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

Successfully merging this pull request may close these issues.

None yet

2 participants