Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

Make waypoint config set command pipeable #674

Merged
merged 2 commits into from
Oct 26, 2020

Conversation

dominikbraun
Copy link
Contributor

@dominikbraun dominikbraun commented Oct 22, 2020

Fixes #561.

This change allows the user to pass arguments to waypoint config set using pipes:

$ cat .env | waypoint config set

The linked issue also proposes a -raw flag for config get, but this flag apparently already exists:

f.BoolVar(&flag.BoolVar{
Name: "raw",
Target: &c.raw,
Usage: "Output the value for the named variable only (disables prefix matching)",
})

However, waypoint config get -raw doesn't print anything to my console. Maybe this is a separate issue.

Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

This looks pretty good! Thank you! I requested some slight changes.


scanner := bufio.NewScanner(os.Stdin)
for scanner.Scan() {
configArgs = append(configArgs, scanner.Text())
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: I would change this a bit to set configArgs := c.args earlier up and then just append to that here. That way we don’t need the somewhat awkward else fall through.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely, that's a more elegant solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I just noticed that I'm now directly appending the config variables to c.args without creating a new slice configArgs at all - but you actually suggested to initialize configArgs with c.Args and still append the variables to configArgs.

Which way would you prefer?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think its fine as-is since we verify len(c.args) == 0 to guard entry into that path anyways. Thanks!

@mitchellh mitchellh merged commit dbf76ad into hashicorp:main Oct 26, 2020
@mitchellh mitchellh added this to the 0.1.4 milestone Oct 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI: enhance the config command experience
2 participants