Skip to content

feat: Refactor CLI config-ssh to improve UX #1900

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.

Sign up for GitHub

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

Merged
merged 38 commits into from
Jun 8, 2022
Merged

feat: Refactor CLI config-ssh to improve UX #1900

Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
b7f2b3d
feat: Refactor CLI config-ssh to improve UX
mafredri May 30, 2022
4f03415
chore: Switch to github.com/pkg/diff for diffing
mafredri May 30, 2022
e196920
fix: Output absolute paths for diffs
mafredri May 30, 2022
cdcf6c3
chore: Remove todone
mafredri May 30, 2022
263d823
chore: Rename actions => changes for consistency
mafredri May 30, 2022
18797e7
fix: Remove workspaces check, removing is a legit use-case
mafredri May 30, 2022
9680792
fix: Remove unused else
mafredri May 30, 2022
7a4b8d6
fix: Handle no workspaces case
mafredri May 30, 2022
e552e84
chore: Update diffBytes comment
mafredri May 30, 2022
5250a8a
fix: Cleanup temp file on failure
mafredri May 30, 2022
0749e58
fix: Wrap error
mafredri May 30, 2022
b3f448a
fix: Enable color when terminal is a tty
mafredri May 30, 2022
c5f1983
fix: Improve output, detect if stdout is tty
mafredri May 31, 2022
8d0f72c
fix: Add Include to the top of config, verify semantically
mafredri May 31, 2022
d7838b5
fix: Guard against overwriting unknown ~/.ssh/coder file
mafredri May 31, 2022
f94bc3f
feat: Parse and prompt to re-use previous configuration
mafredri May 31, 2022
00a8c12
chore: Increase indentation
mafredri May 31, 2022
e50c4c3
chore: Remove unused equality func
mafredri May 31, 2022
94c8681
fix: Perform multi line regexp matches
mafredri Jun 1, 2022
1e33886
fix: Only rewrite coder file path if not ~/
mafredri Jun 1, 2022
209c8ff
fix: Allow removing invalid Include coder entry
mafredri Jun 1, 2022
5ed5e0e
Revert "chore: Remove unused equality func"
mafredri Jun 1, 2022
000c9b4
fix: Allow replacing default ssh config file in tests
mafredri Jun 1, 2022
9d64680
feat: Update options prompt, default to using new options
mafredri Jun 1, 2022
a7cb149
fix: Improve regexes
mafredri Jun 1, 2022
a77c791
feat: Add tests
mafredri Jun 1, 2022
5bcd038
fix: Do not prompt for previous opts on first run
mafredri Jun 1, 2022
1632eb4
fix: Fix broken TestConfigSSH test
mafredri Jun 1, 2022
99693ce
fix: Revert multiwriter change usued while testing
mafredri Jun 1, 2022
fb24742
fix: Change diff suggestions for more natural sentence
mafredri Jun 1, 2022
5c785dd
fix: Typo
mafredri Jun 1, 2022
c60762f
fix: Fix linting (gocyclo, revive), improve responsiveness
mafredri Jun 1, 2022
0422e71
chore: Update header comment
mafredri Jun 6, 2022
6e1e1a0
chore: Extract function
mafredri Jun 6, 2022
fda935f
chore: Remove test log statements
mafredri Jun 6, 2022
f1be4c6
Make Include regexp stricter to avoid deleting user data
mafredri Jun 8, 2022
bf4596c
chore: Update the "Changes" text for diff to match non-diff mode
mafredri Jun 8, 2022
093e2ce
chore: Use consistent naming of functions
mafredri Jun 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: Allow replacing default ssh config file in tests
  • Loading branch information
@mafredri
mafredri committed Jun 8, 2022
commit 000c9b461ecc90a172a0ec28d66c0212644abcf9
28 changes: 14 additions & 14 deletions cli/configssh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,9 @@ var (
// sshCoderConfigOptions represents options that can be stored and read
// from the coder config in ~/.ssh/coder.
type sshCoderConfigOptions struct {
sshConfigFile string
sshOptions []string
}

func (o sshCoderConfigOptions) isZero() bool {
return o.sshConfigFile == sshDefaultConfigFileName && len(o.sshOptions) == 0
sshConfigDefaultFile string
sshConfigFile string
sshOptions []string
}

func (o sshCoderConfigOptions) equal(other sshCoderConfigOptions) bool {
Expand All @@ -75,7 +72,7 @@ func (o sshCoderConfigOptions) equal(other sshCoderConfigOptions) bool {
}

func (o sshCoderConfigOptions) asArgs() (args []string) {
if o.sshConfigFile != sshDefaultConfigFileName {
if o.sshConfigFile != o.sshConfigDefaultFile {
args = append(args, "--ssh-config-file", o.sshConfigFile)
}
for _, opt := range o.sshOptions {
Expand All @@ -85,7 +82,7 @@ func (o sshCoderConfigOptions) asArgs() (args []string) {
}

func (o sshCoderConfigOptions) asList() (list []string) {
if o.sshConfigFile != sshDefaultConfigFileName {
if o.sshConfigFile != o.sshConfigDefaultFile {
list = append(list, fmt.Sprintf("ssh-config-file: %s", o.sshConfigFile))
}
for _, opt := range o.sshOptions {
Expand Down Expand Up @@ -175,7 +172,7 @@ func configSSH() *cobra.Command {
return xerrors.Errorf("unexpected content in %s: remove the file and rerun the command to continue", coderConfigFile)
}
}
lastCoderConfig := sshCoderConfigParseLastOptions(bytes.NewReader(coderConfigRaw))
lastCoderConfig := sshCoderConfigParseLastOptions(bytes.NewReader(coderConfigRaw), coderConfig.sshConfigDefaultFile)

// Only prompt when no arguments are provided and avoid
// prompting in diff mode (unexpected behavior).
Expand Down Expand Up @@ -368,8 +365,10 @@ func configSSH() *cobra.Command {
},
}
cliflag.StringVarP(cmd.Flags(), &coderConfig.sshConfigFile, "ssh-config-file", "", "CODER_SSH_CONFIG_FILE", sshDefaultConfigFileName, "Specifies the path to an SSH config.")
cmd.Flags().StringVar(&coderConfigFile, "ssh-coder-config-file", sshDefaultCoderConfigFileName, "Specifies the path to an Coder SSH config file. Useful for testing.")
_ = cmd.Flags().MarkHidden("ssh-coder-config-file")
cmd.Flags().StringVar(&coderConfig.sshConfigDefaultFile, "test.default-ssh-config-file", sshDefaultConfigFileName, "Specifies the default path to the SSH config file. Useful for testing.")
_ = cmd.Flags().MarkHidden("test.default-ssh-config-file")
cmd.Flags().StringVar(&coderConfigFile, "test.ssh-coder-config-file", sshDefaultCoderConfigFileName, "Specifies the path to an Coder SSH config file. Useful for testing.")
_ = cmd.Flags().MarkHidden("test.ssh-coder-config-file")
cmd.Flags().StringArrayVarP(&coderConfig.sshOptions, "ssh-option", "o", []string{}, "Specifies additional SSH options to embed in each host stanza.")
cmd.Flags().BoolVarP(&showDiff, "diff", "D", false, "Show diff of changes that will be made.")
cmd.Flags().BoolVarP(&skipProxyCommand, "skip-proxy-command", "", false, "Specifies whether the ProxyCommand option should be skipped. Useful for testing.")
Expand Down Expand Up @@ -418,7 +417,7 @@ func sshCoderConfigWriteHeader(w io.Writer, o sshCoderConfigOptions) error {
_, _ = fmt.Fprint(w, sshCoderConfigHeader)
_, _ = fmt.Fprint(w, sshCoderConfigDocsHeader)
_, _ = fmt.Fprint(w, sshCoderConfigOptionsHeader)
if o.sshConfigFile != sshDefaultConfigFileName {
if o.sshConfigFile != o.sshConfigDefaultFile {
_, _ = fmt.Fprintf(w, "# :%s=%s\n", "ssh-config-file", o.sshConfigFile)
}
for _, opt := range o.sshOptions {
Expand All @@ -428,8 +427,9 @@ func sshCoderConfigWriteHeader(w io.Writer, o sshCoderConfigOptions) error {
return nil
}

func sshCoderConfigParseLastOptions(r io.Reader) (o sshCoderConfigOptions) {
o.sshConfigFile = sshDefaultConfigFileName // Default value is not written.
func sshCoderConfigParseLastOptions(r io.Reader, sshConfigDefaultFile string) (o sshCoderConfigOptions) {
o.sshConfigDefaultFile = sshConfigDefaultFile
o.sshConfigFile = sshConfigDefaultFile // Default value is not written.

s := bufio.NewScanner(r)
for s.Scan() {
Expand Down