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

cylc-bash-complete completes registered suite names when a command takes suite names #2771

Merged
merged 3 commits into from
Sep 24, 2018

Conversation

alpacaxander
Copy link

Currently cylc-bash-complete will complete cylc commands if cylc is the first word.

This modification will complete cylc commands if cylc is the only word and will complete registered suite names when specific commands (which take suite names) are the second word.

suite_cmds is ugly but I felt manually inputting commands that take suite names was better than completing suite names for all commands.

First time contributing so please let me know if I am missing anything.

AlexanderPaulsell added 2 commits September 5, 2018 16:47
…word; will complete registered cylc suite names when a command that takes suite names is the second word
…word; will complete registered cylc suite names when a command that takes suite names is the second word
@hjoliver hjoliver added this to the soon milestone Sep 6, 2018
@hjoliver
Copy link
Member

hjoliver commented Sep 6, 2018

(@matthewrmshin - assigned reviewers who aren't going to be at the workshop next week, but feel free to change).

Copy link
Collaborator

@sadielbartholomew sadielbartholomew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something is amiss at the moment, as I get auto-completion with (further) cylc command keywords instead of suite names after a suite-accepting command, e.g. upon tab completion:

$ cylc run s
scan           set-verbosity  spawn          submit         
scp-transfer   show           start          suite-state    
search         shutdown       stop
$ cylc validate r
register        remote-init     report-timings  run
release         remote-tidy     reset           
reload          remove          restart 

Also forgive me but I am not sure what the difference is between:

complete cylc commands if cylc is the first word

and:

complete cylc commands if cylc is the only word

so it is not clear to me what I am testing for in the first case. Can you please clarify this, perhaps with some command-line examples?

@alpacaxander
Copy link
Author

The difference is that the current version would always complete commands no matter how many words were on the line
eg:

$ cylc run scan register foo s
scan           set-verbosity  spawn          submit         
scp-transfer   show           start          suite-state    
search         shutdown       stop

whereas this will (should) only complete for commands if cylc is the only word on the line

$ cylc r
register        reload          remote-tidy     report-timings  restart
release         remote-init     remove          reset           run
$ cylc run r 

^^^
Last line will not complete with commands starting with r.

I am not sure why yours is behaving like the current way. Here is the behavior I have. The only thing to come to mind is if the file wasn't sourced.

$ cylc
5to6               cycle-point        get-cylc-version   gsummary           ls                 remote-tidy        start
bcast              cycletime          get-directory      gui                ls-checkpoints     remove             stop
broadcast          datetime           get-global-config  help               message            report-timings     submit
browse             diff               get-gui-config     hold               monitor            reset              suite-state
cat-log            documentation      get-site-config    import-examples    nudge              restart            task-message
cat-state          dump               get-suite-config   insert             ping               run                test-battery
checkpoint         edit               get-suite-contact  jobscript          poll               scan               trigger
check-software     email-suite        get-suite-version  jobs-kill          print              scp-transfer       unhold
check-triggering   email-task         gpanel             jobs-poll          profile-battery    search             upgrade-run-dir
check-versions     external-trigger   graph              jobs-submit        register           set-verbosity      validate
compare            ext-trigger        graph-diff         kill               release            show               version
conditions         get-config         grep               list               reload             shutdown           view
cyclepoint         get-contact        gscan              log                remote-init        spawn              warranty
$ cylc r
register        reload          remote-tidy     report-timings  restart
release         remote-init     remove          reset           run
$ cylc run
omniglobe   toZip       tryone      twa         webscrape   webscrape2
$ cylc run t
toZip   tryone  twa
$ cylc run t
$ cylc run -n t
toZip   tryone  twa
$ cylc run -n t

@sadielbartholomew
Copy link
Collaborator

Thanks for clarifying the desired behaviour, & please accept my apologies, as I assumed my .bashrc file was sourcing the cylc-bash-completion under etc/ as checked out from this branch but it was in fact sourcing the equivalent file set for my site i.e. the original.

With this change actually set-up (!) I get the same tab-completion behaviour you outline above, though obviously with my own local suites returned as suggestions.

There are however certain commands that are defined under suite_cmds that are not appropriate for suite auto-completion, e.g. get-site-config, so please go through that listing & check each of these commands accepts a suite name. Otherwise this looks great.

@sadielbartholomew
Copy link
Collaborator

The alias command print-contact is now being removed (#2778) so should be taken out of suite_cmds too.

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me - once @sadielbartholomew's two comments are addressed.

@alpacaxander
Copy link
Author

Ok,
I updated it but I haven't used many of these commands so let me know if there are any other issues.
All I did was go through cylc help all and any command that took SUITE or REG I added to the list.

@sadielbartholomew sadielbartholomew merged commit a3f71aa into cylc:master Sep 24, 2018
@matthewrmshin matthewrmshin modified the milestones: soon, next release Oct 1, 2018
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.

4 participants