From 1969423b5c20784cb1b50a1e6e6434ef14a37fbb Mon Sep 17 00:00:00 2001 From: Christopher Allen Lane Date: Sun, 15 Feb 2026 06:54:42 -0500 Subject: [PATCH] fix: respect $VISUAL and $EDITOR env vars at runtime Previously, env vars were only consulted during config generation and baked into conf.yml. At runtime, the config file value was always used, making it impossible to override the editor via environment variables. Now the precedence is: $VISUAL > $EDITOR > conf.yml > auto-detect. Closes #589 Co-Authored-By: Claude Opus 4.6 --- cmd/cheat/config.go | 2 +- internal/config/config.go | 13 +++-- internal/config/config_test.go | 91 ++++++++++++++++++++++++++-------- internal/config/new_test.go | 10 ++++ 4 files changed, 90 insertions(+), 26 deletions(-) diff --git a/cmd/cheat/config.go b/cmd/cheat/config.go index 899e45e..1ed521f 100644 --- a/cmd/cheat/config.go +++ b/cmd/cheat/config.go @@ -3,7 +3,7 @@ package main // configs returns the default configuration template func configs() string { return `--- -# The editor to use with 'cheat -e '. Defaults to $EDITOR or $VISUAL. +# The editor to use with 'cheat -e '. Overridden by $VISUAL or $EDITOR. editor: EDITOR_PATH # Should 'cheat' always colorize output? diff --git a/internal/config/config.go b/internal/config/config.go index fb8a08e..d87bfb5 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -107,10 +107,17 @@ func New(_ map[string]interface{}, confPath string, resolve bool) (Config, error } conf.Cheatpaths = validPaths - // trim editor whitespace - conf.Editor = strings.TrimSpace(conf.Editor) + // determine the editor: env vars override the config file value, + // following standard Unix convention (see #589) + if v := os.Getenv("VISUAL"); v != "" { + conf.Editor = v + } else if v := os.Getenv("EDITOR"); v != "" { + conf.Editor = v + } else { + conf.Editor = strings.TrimSpace(conf.Editor) + } - // if an editor was not provided in the configs, attempt to choose one + // if an editor was still not determined, attempt to choose one // that's appropriate for the environment if conf.Editor == "" { if conf.Editor, err = Editor(); err != nil { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index fc56a38..9795118 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -4,7 +4,6 @@ import ( "os" "path/filepath" "reflect" - "runtime" "testing" "github.com/davecgh/go-spew/spew" @@ -17,6 +16,16 @@ import ( // TestConfig asserts that the configs are loaded correctly func TestConfigSuccessful(t *testing.T) { + // clear env vars so they don't override the config file value + oldVisual := os.Getenv("VISUAL") + oldEditor := os.Getenv("EDITOR") + os.Unsetenv("VISUAL") + os.Unsetenv("EDITOR") + defer func() { + os.Setenv("VISUAL", oldVisual) + os.Setenv("EDITOR", oldEditor) + }() + // initialize a config conf, err := New(map[string]interface{}{}, mock.Path("conf/conf.yml"), false) if err != nil { @@ -76,40 +85,78 @@ func TestConfigFailure(t *testing.T) { } } -// TestEmptyEditor asserts that envvars are respected if an editor is not -// specified in the configs -func TestEmptyEditor(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("Editor() returns notepad on Windows before checking env vars") +// TestEditorEnvOverride asserts that $VISUAL and $EDITOR override the +// config file value at runtime (regression test for #589) +func TestEditorEnvOverride(t *testing.T) { + // save and clear the environment variables + oldVisual := os.Getenv("VISUAL") + oldEditor := os.Getenv("EDITOR") + defer func() { + os.Setenv("VISUAL", oldVisual) + os.Setenv("EDITOR", oldEditor) + }() + + // with no env vars, the config file value should be used + os.Unsetenv("VISUAL") + os.Unsetenv("EDITOR") + conf, err := New(map[string]interface{}{}, mock.Path("conf/conf.yml"), false) + if err != nil { + t.Fatalf("failed to init configs: %v", err) + } + if conf.Editor != "vim" { + t.Errorf("expected config file editor: want: vim, got: %s", conf.Editor) } - // clear the environment variables - os.Setenv("VISUAL", "") - os.Setenv("EDITOR", "") + // $EDITOR should override the config file value + os.Setenv("EDITOR", "nano") + conf, err = New(map[string]interface{}{}, mock.Path("conf/conf.yml"), false) + if err != nil { + t.Fatalf("failed to init configs: %v", err) + } + if conf.Editor != "nano" { + t.Errorf("$EDITOR should override config: want: nano, got: %s", conf.Editor) + } - // initialize a config + // $VISUAL should override both $EDITOR and the config file value + os.Setenv("VISUAL", "emacs") + conf, err = New(map[string]interface{}{}, mock.Path("conf/conf.yml"), false) + if err != nil { + t.Fatalf("failed to init configs: %v", err) + } + if conf.Editor != "emacs" { + t.Errorf("$VISUAL should override all: want: emacs, got: %s", conf.Editor) + } +} + +// TestEditorEnvFallback asserts that env vars are used as fallback when +// no editor is specified in the config file +func TestEditorEnvFallback(t *testing.T) { + // save and clear the environment variables + oldVisual := os.Getenv("VISUAL") + oldEditor := os.Getenv("EDITOR") + defer func() { + os.Setenv("VISUAL", oldVisual) + os.Setenv("EDITOR", oldEditor) + }() + + // set $EDITOR and assert it's used when config has no editor + os.Unsetenv("VISUAL") + os.Setenv("EDITOR", "foo") conf, err := New(map[string]interface{}{}, mock.Path("conf/empty.yml"), false) if err != nil { - t.Errorf("failed to initialize test: %v", err) - } - - // set editor, and assert that it is respected - os.Setenv("EDITOR", "foo") - conf, err = New(map[string]interface{}{}, mock.Path("conf/empty.yml"), false) - if err != nil { - t.Errorf("failed to init configs: %v", err) + t.Fatalf("failed to init configs: %v", err) } if conf.Editor != "foo" { - t.Errorf("failed to respect editor: want: foo, got: %s", conf.Editor) + t.Errorf("failed to respect $EDITOR: want: foo, got: %s", conf.Editor) } - // set visual, and assert that it overrides editor + // set $VISUAL and assert it takes precedence over $EDITOR os.Setenv("VISUAL", "bar") conf, err = New(map[string]interface{}{}, mock.Path("conf/empty.yml"), false) if err != nil { - t.Errorf("failed to init configs: %v", err) + t.Fatalf("failed to init configs: %v", err) } if conf.Editor != "bar" { - t.Errorf("failed to respect editor: want: bar, got: %s", conf.Editor) + t.Errorf("failed to respect $VISUAL: want: bar, got: %s", conf.Editor) } } diff --git a/internal/config/new_test.go b/internal/config/new_test.go index a87750c..6a1edd9 100644 --- a/internal/config/new_test.go +++ b/internal/config/new_test.go @@ -7,6 +7,16 @@ import ( ) func TestNewTrimsWhitespace(t *testing.T) { + // clear env vars so they don't override the config file value + oldVisual := os.Getenv("VISUAL") + oldEditor := os.Getenv("EDITOR") + os.Unsetenv("VISUAL") + os.Unsetenv("EDITOR") + defer func() { + os.Setenv("VISUAL", oldVisual) + os.Setenv("EDITOR", oldEditor) + }() + // Create a temporary config file with whitespace in editor and pager tmpDir := t.TempDir() configPath := filepath.Join(tmpDir, "config.yml")