Skip to content

Commit c646867

Browse files
committed
fix: Improve output, detect if stdout is tty
1 parent deaee03 commit c646867

File tree

2 files changed

+41
-16
lines changed

2 files changed

+41
-16
lines changed

cli/configssh.go

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,15 @@ func configSSH() *cobra.Command {
103103
// TODO(mafredri): Check coderConfigFile for previous options
104104
// coderConfigFile.
105105

106+
out := cmd.OutOrStdout()
107+
if showDiff {
108+
out = cmd.OutOrStderr()
109+
}
110+
binaryFile, err := currentBinPath(out)
111+
if err != nil {
112+
return err
113+
}
114+
106115
// Only allow not-exist errors to avoid trashing
107116
// the users SSH config.
108117
configRaw, err := os.ReadFile(sshConfigFile)
@@ -136,11 +145,6 @@ func configSSH() *cobra.Command {
136145
configModified = append(configModified, []byte(sep+sshConfigIncludeStatement+"\n")...)
137146
}
138147

139-
binaryFile, err := currentBinPath(cmd)
140-
if err != nil {
141-
return err
142-
}
143-
144148
root := createConfig(cmd)
145149
var errGroup errgroup.Group
146150
type workspaceConfig struct {
@@ -240,13 +244,13 @@ func configSSH() *cobra.Command {
240244
if showDiff {
241245
if len(changes) > 0 {
242246
// Write to stderr to avoid dirtying the diff output.
243-
_, _ = fmt.Fprint(cmd.ErrOrStderr(), "Changes:\n\n")
247+
_, _ = fmt.Fprint(out, "Changes:\n\n")
244248
for _, change := range changes {
245-
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "* %s\n", change)
249+
_, _ = fmt.Fprintf(out, "* %s\n", change)
246250
}
247251
}
248252

249-
color := isTTY(cmd)
253+
color := isTTYOut(cmd)
250254
for _, diffFn := range []func() ([]byte, error){
251255
func() ([]byte, error) { return diffBytes(sshConfigFile, configRaw, configModified, color) },
252256
func() ([]byte, error) { return diffBytes(coderConfigFile, coderConfigRaw, buf.Bytes(), color) },
@@ -257,6 +261,7 @@ func configSSH() *cobra.Command {
257261
}
258262
if len(diff) > 0 {
259263
filesDiffer = true
264+
// Always write to stdout.
260265
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "\n%s", diff)
261266
}
262267
}
@@ -270,9 +275,9 @@ func configSSH() *cobra.Command {
270275
IsConfirm: true,
271276
})
272277
if err != nil {
273-
return err
278+
return nil
274279
}
275-
_, _ = fmt.Fprint(cmd.OutOrStdout(), "\n")
280+
_, _ = fmt.Fprint(out, "\n")
276281

277282
if !bytes.Equal(configRaw, configModified) {
278283
err = writeWithTempFileAndMove(sshConfigFile, bytes.NewReader(configRaw))
@@ -289,10 +294,10 @@ func configSSH() *cobra.Command {
289294
}
290295

291296
if len(workspaces) > 0 {
292-
_, _ = fmt.Fprintln(cmd.OutOrStdout(), "You should now be able to ssh into your workspace")
293-
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "For example, try running:\n\n\t$ ssh coder.%s\n\n", workspaces[0].Name)
297+
_, _ = fmt.Fprintln(out, "You should now be able to ssh into your workspace.")
298+
_, _ = fmt.Fprintf(out, "For example, try running:\n\n\t$ ssh coder.%s\n\n", workspaces[0].Name)
294299
} else {
295-
_, _ = fmt.Fprint(cmd.OutOrStdout(), "You don't have any workspaces yet, try creating one with:\n\n\t$ coder create <workspace>\n\n")
300+
_, _ = fmt.Fprint(out, "You don't have any workspaces yet, try creating one with:\n\n\t$ coder create <workspace>\n\n")
296301
}
297302
return nil
298303
},
@@ -349,7 +354,7 @@ func writeWithTempFileAndMove(path string, r io.Reader) (err error) {
349354

350355
// currentBinPath returns the path to the coder binary suitable for use in ssh
351356
// ProxyCommand.
352-
func currentBinPath(cmd *cobra.Command) (string, error) {
357+
func currentBinPath(w io.Writer) (string, error) {
353358
exePath, err := os.Executable()
354359
if err != nil {
355360
return "", xerrors.Errorf("get executable path: %w", err)
@@ -365,24 +370,26 @@ func currentBinPath(cmd *cobra.Command) (string, error) {
365370
// correctly. Check if the current executable is in $PATH, and warn the user
366371
// if it isn't.
367372
if err != nil && runtime.GOOS == "windows" {
368-
cliui.Warn(cmd.OutOrStdout(),
373+
cliui.Warn(w,
369374
"The current executable is not in $PATH.",
370375
"This may lead to problems connecting to your workspace via SSH.",
371376
fmt.Sprintf("Please move %q to a location in your $PATH (such as System32) and run `%s config-ssh` again.", binName, binName),
372377
)
378+
_, _ = fmt.Fprint(w, "\n")
373379
// Return the exePath so SSH at least works outside of Msys2.
374380
return exePath, nil
375381
}
376382

377383
// Warn the user if the current executable is not the same as the one in
378384
// $PATH.
379385
if filepath.Clean(pathPath) != filepath.Clean(exePath) {
380-
cliui.Warn(cmd.OutOrStdout(),
386+
cliui.Warn(w,
381387
"The current executable path does not match the executable path found in $PATH.",
382388
"This may cause issues connecting to your workspace via SSH.",
383389
fmt.Sprintf("\tCurrent executable path: %q", exePath),
384390
fmt.Sprintf("\tExecutable path in $PATH: %q", pathPath),
385391
)
392+
_, _ = fmt.Fprint(w, "\n")
386393
}
387394

388395
return binName, nil

cli/root.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,24 @@ func isTTY(cmd *cobra.Command) bool {
203203
return isatty.IsTerminal(file.Fd())
204204
}
205205

206+
// isTTYOut returns whether the passed reader is a TTY or not.
207+
// This accepts a reader to work with Cobra's "OutOrStdout"
208+
// function for simple testing.
209+
func isTTYOut(cmd *cobra.Command) bool {
210+
// If the `--force-tty` command is available, and set,
211+
// assume we're in a tty. This is primarily for cases on Windows
212+
// where we may not be able to reliably detect this automatically (ie, tests)
213+
forceTty, err := cmd.Flags().GetBool(varForceTty)
214+
if forceTty && err == nil {
215+
return true
216+
}
217+
file, ok := cmd.OutOrStdout().(*os.File)
218+
if !ok {
219+
return false
220+
}
221+
return isatty.IsTerminal(file.Fd())
222+
}
223+
206224
func usageTemplate() string {
207225
// usageHeader is defined in init().
208226
return `{{usageHeader "Usage:"}}

0 commit comments

Comments
 (0)