diff --git a/internal/config/config_type.go b/internal/config/config_type.go index 3714698bd..92792e93f 100644 --- a/internal/config/config_type.go +++ b/internal/config/config_type.go @@ -55,6 +55,11 @@ var configOptions = []ConfigOption{ Description: "the path to a unix socket through which to make HTTP connection", DefaultValue: "", }, + { + Key: "browser", + Description: "the web browser to use for opening URLs", + DefaultValue: "", + }, } func ConfigOptions() []ConfigOption { @@ -193,6 +198,15 @@ func NewBlankRoot() *yaml.Node { Kind: yaml.ScalarNode, Value: "", }, + { + HeadComment: "What web browser gh should use when opening URLs. If blank, will refer to environment.", + Kind: yaml.ScalarNode, + Value: "browser", + }, + { + Kind: yaml.ScalarNode, + Value: "", + }, }, }, }, diff --git a/internal/config/config_type_test.go b/internal/config/config_type_test.go index fe5e3f239..bf53aabe4 100644 --- a/internal/config/config_type_test.go +++ b/internal/config/config_type_test.go @@ -52,6 +52,8 @@ func Test_defaultConfig(t *testing.T) { co: pr checkout # The path to a unix socket through which send HTTP connections. If blank, HTTP traffic will be handled by net/http.DefaultTransport. http_unix_socket: + # What web browser gh should use when opening URLs. If blank, will refer to environment. + browser: `) assert.Equal(t, expected, mainBuf.String()) assert.Equal(t, "", hostsBuf.String()) @@ -69,6 +71,10 @@ func Test_defaultConfig(t *testing.T) { assert.Equal(t, len(aliases.All()), 1) expansion, _ := aliases.Get("co") assert.Equal(t, expansion, "pr checkout") + + browser, err := cfg.Get("", "browser") + assert.NoError(t, err) + assert.Equal(t, "", browser) } func Test_ValidateValue(t *testing.T) { @@ -106,4 +112,7 @@ func Test_ValidateKey(t *testing.T) { err = ValidateKey("http_unix_socket") assert.NoError(t, err) + + err = ValidateKey("browser") + assert.NoError(t, err) } diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index a6956faad..40fac08f9 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -29,7 +29,7 @@ func New(appVersion string) *cmdutil.Factory { f.HttpClient = httpClientFunc(f, appVersion) // Depends on Config, IOStreams, and appVersion f.Remotes = remotesFunc(f) // Depends on Config f.BaseRepo = BaseRepoFunc(f) // Depends on Remotes - f.Browser = browser(f) // Depends on IOStreams + f.Browser = browser(f) // Depends on Config, and IOStreams return f } @@ -91,7 +91,26 @@ func httpClientFunc(f *cmdutil.Factory, appVersion string) func() (*http.Client, func browser(f *cmdutil.Factory) cmdutil.Browser { io := f.IOStreams - return cmdutil.NewBrowser(os.Getenv("BROWSER"), io.Out, io.ErrOut) + return cmdutil.NewBrowser(browserLauncher(f), io.Out, io.ErrOut) +} + +// Browser precedence +// 1. GH_BROWSER +// 2. browser from config +// 3. BROWSER +func browserLauncher(f *cmdutil.Factory) string { + if ghBrowser := os.Getenv("GH_BROWSER"); ghBrowser != "" { + return ghBrowser + } + + cfg, err := f.Config() + if err == nil { + if cfgBrowser, _ := cfg.Get("", "browser"); cfgBrowser != "" { + return cfgBrowser + } + } + + return os.Getenv("BROWSER") } func executable() string { diff --git a/pkg/cmd/factory/default_test.go b/pkg/cmd/factory/default_test.go index c7e641aa1..03ea29153 100644 --- a/pkg/cmd/factory/default_test.go +++ b/pkg/cmd/factory/default_test.go @@ -378,6 +378,80 @@ func Test_ioStreams_prompt(t *testing.T) { } } +func Test_browserLauncher(t *testing.T) { + tests := []struct { + name string + env map[string]string + config config.Config + wantBrowser string + }{ + { + name: "GH_BROWSER set", + env: map[string]string{ + "GH_BROWSER": "GH_BROWSER", + }, + wantBrowser: "GH_BROWSER", + }, + { + name: "config browser set", + config: config.NewFromString("browser: CONFIG_BROWSER"), + wantBrowser: "CONFIG_BROWSER", + }, + { + name: "BROWSER set", + env: map[string]string{ + "BROWSER": "BROWSER", + }, + wantBrowser: "BROWSER", + }, + { + name: "GH_BROWSER and config browser set", + env: map[string]string{ + "GH_BROWSER": "GH_BROWSER", + }, + config: config.NewFromString("browser: CONFIG_BROWSER"), + wantBrowser: "GH_BROWSER", + }, + { + name: "config browser and BROWSER set", + env: map[string]string{ + "BROWSER": "BROWSER", + }, + config: config.NewFromString("browser: CONFIG_BROWSER"), + wantBrowser: "CONFIG_BROWSER", + }, + { + name: "GH_BROWSER and BROWSER set", + env: map[string]string{ + "BROWSER": "BROWSER", + "GH_BROWSER": "GH_BROWSER", + }, + wantBrowser: "GH_BROWSER", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.env != nil { + for k, v := range tt.env { + old := os.Getenv(k) + os.Setenv(k, v) + defer os.Setenv(k, old) + } + } + f := New("1") + f.Config = func() (config.Config, error) { + if tt.config == nil { + return config.NewBlankConfig(), nil + } else { + return tt.config, nil + } + } + browser := browserLauncher(f) + assert.Equal(t, tt.wantBrowser, browser) + }) + } +} + func defaultConfig() config.Config { return config.InheritEnv(config.NewFromString(heredoc.Doc(` hosts: diff --git a/pkg/cmd/root/help_topic.go b/pkg/cmd/root/help_topic.go index 9d512c79f..61b812080 100644 --- a/pkg/cmd/root/help_topic.go +++ b/pkg/cmd/root/help_topic.go @@ -44,7 +44,7 @@ var HelpTopics = map[string]map[string]string{ GH_EDITOR, GIT_EDITOR, VISUAL, EDITOR (in order of precedence): the editor tool to use for authoring text. - BROWSER: the web browser to use for opening links. + GH_BROWSER, BROWSER (in order of precedence): the web browser to use for opening links. DEBUG: set to any value to enable verbose output to standard error. Include values "api" or "oauth" to print detailed information about HTTP requests or authentication flow.