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

Terraform command completions #3960

Merged
merged 7 commits into from Apr 17, 2017
Merged

Conversation

@adambyrtek
Copy link
Contributor

@adambyrtek adambyrtek commented Apr 16, 2017

Description

Terraform is an open source declarative infrastructure management tool:
https://www.terraform.io
https://github.com/hashicorp/terraform

This PR adds completions for all basic commands and their flags:

$ terraform help
Usage: terraform [--version] [--help] <command> [args]

The available commands for execution are listed below.
The most common, useful commands are shown first, followed by
less common or more advanced commands. If you're just getting
started with Terraform, stick with the common commands. For the
other commands, please read the help and docs before usage.

Common commands:
    apply              Builds or changes infrastructure
    console            Interactive console for Terraform interpolations
    destroy            Destroy Terraform-managed infrastructure
    env                Environment management
    fmt                Rewrites config files to canonical format
    get                Download and install modules for the configuration
    graph              Create a visual graph of Terraform resources
    import             Import existing infrastructure into Terraform
    init               Initialize a new or existing Terraform configuration
    output             Read an output from a state file
    plan               Generate and show an execution plan
    push               Upload this Terraform module to Atlas to run
    refresh            Update local state file against real resources
    show               Inspect Terraform state or plan
    taint              Manually mark a resource for recreation
    untaint            Manually unmark a resource as tainted
    validate           Validates the Terraform files
    version            Prints the Terraform version

The code was based on Git completions in order to keep things consistent.

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages (N/A)
  • Tests have been added for regressions fixed (N/A)
  • User-visible changes noted in CHANGELOG.md
@@ -0,0 +1,74 @@
function __fish_terraform_needs_command
set cmd (commandline -opc)

This comment has been minimized.

@krader1961

krader1961 Apr 17, 2017
Contributor

In general you should use local variables to avoid modifying a global var by the same name; i.e., set -l cmd....

This comment has been minimized.

@adambyrtek

adambyrtek Apr 17, 2017
Author Contributor

This function is not needed anymore.

@@ -0,0 +1,74 @@
function __fish_terraform_needs_command
set cmd (commandline -opc)
if [ (count $cmd) -eq 1 ]

This comment has been minimized.

@krader1961

krader1961 Apr 17, 2017
Contributor

The idiomatic fish way to do this is if not set -q cmd[2] to avoid the subcommand and test. Also, we prefer test over [. The latter is there strictly to make it easier for people transitioning from POSIX 1003.1 shells.

This comment has been minimized.

@adambyrtek

adambyrtek Apr 17, 2017
Author Contributor

This function is not needed anymore.

@@ -15,60 +15,193 @@ function __fish_terraform_needs_command
return 0
end

function __fish_terraform_using_command

This comment has been minimized.

@krader1961

krader1961 Apr 17, 2017
Contributor

@faho should comment on this since I'm a completion newbie but I thought that functions in general should be using __fish_use_subcommand rather than rolling their own custom version. That there are existing completions that haven't been updated to use the global function is no surprise.

This comment has been minimized.

@adambyrtek

adambyrtek Apr 17, 2017
Author Contributor

Good idea, I replaced this function with __fish_use_subcommand.

This comment has been minimized.

@faho

faho Apr 17, 2017
Member

Well......

The issue with custom versions is that they are usually just

if test "$cmd[2]" = $argv[1]
     return 0
else
    return 1
end

(and maybe some annoying stuff about how $cmd[1] needs to be the name of the tool, which breaks aliases)

However, __fish_use_subcommand isn't much better:

function __fish_use_subcommand -d "Test if a non-switch argument has been given in the current commandline"
    set -l cmd (commandline -poc)
    set -e cmd[1]
    for i in $cmd
        switch $i
            case '-*'
                continue
        end
        return 1
    end
    return 0
end

This will break if

  • options are allowed before the subcommand

  • those options can be used like --option argument, i.e. the argument is another token that doesn't start with "-"

e.g. on something like git --git-dir /some/path.

Which is why the git completions have the __fish_git_needs_command function, that explicitly skips all the options, and skips the next token for options that can take a command.

Currently, I don't know of an easier way to be "fully" correct (you essentially need to recreate the command's argument parsing). __fish_use_subcommand is okay if no argument-taking options are allowed before the subcommand, something like not __fish_seen_subcommand_from $cmds is often good enough. Neither are perfect, though.

This comment has been minimized.

@adambyrtek

adambyrtek Apr 17, 2017
Author Contributor

In this case this should be fine since options alowed before Terraform subcommand don't take any arguments (e.g. terraform --help plan). I get your point about Git completions thought.


### apply
complete -f -c terraform -n '__fish_terraform_needs_command' -a apply -d 'Builds or changes infrastructure'
complete -f -c terraform -n '__fish_terraform_using_command apply' -o backup -d 'Path to backup the existing state file'

This comment has been minimized.

@krader1961

krader1961 Apr 17, 2017
Contributor

Similar to my previous comment I believe the current best practice is to use __fish_seen_subcommand_from rather than define a custom function which does essentially the same thing.

This comment has been minimized.

@adambyrtek

adambyrtek Apr 17, 2017
Author Contributor

Done

@adambyrtek
Copy link
Contributor Author

@adambyrtek adambyrtek commented Apr 17, 2017

@krader1961 Thanks for a quick turnaround and helpful comments! Please take another look. By the way, looks like the Git completion I based this one on needs the same changes. I might look into that later today.

complete -f -c terraform -l help -d 'Show help'

### apply
complete -f -c terraform -n '__fish_use_subcommand' -a apply -d 'Builds or changes infrastructure'

This comment has been minimized.

@faho

faho Apr 17, 2017
Member

The imperative form ("Build or change") is more common here, and also a couple of characters shorter.

This comment has been minimized.

@adambyrtek

adambyrtek Apr 17, 2017
Author Contributor

Done for high-level commands. Just so you know, the descriptions were based directly on terraform --help.

@@ -16,6 +16,7 @@
- Command substitutions now have access to the terminal, allowing tools like `fzf` to work in them (#1362, #3922).
- `bg`s argument parsing has been reworked. It now fails for invalid arguments but allows non-existent jobs (#3909).
- `read` now supports the `--silent` flag to hide the characters typed (#838).
- Added completions for `terraform`.

This comment has been minimized.

@faho

faho Apr 17, 2017
Member

Note: Usually before a release we'll go through and add all added completions, since it's easier to add that line once, especially when there are multiple PRs that add completions in flight at the same time (merge conflicts).

There's a magical git incarnation for it (as always) - git diff --name-only --diff-filter=A 2.5.0 share/completions.

This comment has been minimized.

@adambyrtek

adambyrtek Apr 17, 2017
Author Contributor

Sounds good, removed.

@adambyrtek
Copy link
Contributor Author

@adambyrtek adambyrtek commented Apr 17, 2017

@faho I've made the changes you requested.

@faho faho merged commit e7d6864 into fish-shell:master Apr 17, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@faho
Copy link
Member

@faho faho commented Apr 17, 2017

Merged, thanks!

@faho faho added the completions label Apr 17, 2017
@faho faho added this to the 2.6.0 milestone Apr 17, 2017
@adambyrtek adambyrtek deleted the adambyrtek:terraform-completion branch Apr 17, 2017
@adambyrtek
Copy link
Contributor Author

@adambyrtek adambyrtek commented Apr 17, 2017

@faho You're welcome, it's a pleasure with maintainers being so helpful and responsive :)

develop7 added a commit to develop7/fish-shell that referenced this pull request Apr 17, 2017
* Basic Terraform completion supporting all commands

* Option completion for Terraform commands

* Search command line in reverse order

* CHANGELOG entry

* Fix `terraform untaint` completion

* Use common completion functions to handle subcommands

* Use imperative form and remove CHANGELOG changes
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.