From 6d111b2458638b92f911a177b467350c3fdb0b28 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 16 Sep 2020 14:45:06 +0200 Subject: [PATCH 1/5] Use WSL2 detection to pick browser --- pkg/browser/browser.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/pkg/browser/browser.go b/pkg/browser/browser.go index 67ebca92f..a91bae302 100644 --- a/pkg/browser/browser.go +++ b/pkg/browser/browser.go @@ -1,6 +1,7 @@ package browser import ( + "bufio" "os" "os/exec" "runtime" @@ -30,7 +31,11 @@ func ForOS(goos, url string) *exec.Cmd { r := strings.NewReplacer("&", "^&") args = append(args, "/c", "start", r.Replace(url)) default: - exe = "xdg-open" + if inWsl() { + exe = "wslview" + } else { + exe = "xdg-open" + } args = append(args, url) } @@ -51,3 +56,21 @@ func FromLauncher(launcher, url string) (*exec.Cmd, error) { cmd.Stderr = os.Stderr return cmd, nil } + +// Determine if gh is running inside of WSL2 +func inWsl() bool { + file, err := os.Open("/proc/sys/kernel/osrelease") + if err != nil { + return false + } + defer file.Close() + + scanner := bufio.NewScanner(file) + scanner.Scan() // Read single line + if err := scanner.Err(); err != nil { + return false + } + + os := scanner.Text() + return strings.Contains(os, "microsoft-WSL2") +} From dd1c24a20aa04ad94df62807bc78c7768012d171 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 16 Sep 2020 15:05:42 +0200 Subject: [PATCH 2/5] Use `LookPath` to determine if `xdg-open` exists --- pkg/browser/browser.go | 24 +++++++----------------- pkg/browser/browser_test.go | 20 ++++++++++++++++---- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/pkg/browser/browser.go b/pkg/browser/browser.go index a91bae302..6496966a4 100644 --- a/pkg/browser/browser.go +++ b/pkg/browser/browser.go @@ -1,7 +1,6 @@ package browser import ( - "bufio" "os" "os/exec" "runtime" @@ -31,10 +30,10 @@ func ForOS(goos, url string) *exec.Cmd { r := strings.NewReplacer("&", "^&") args = append(args, "/c", "start", r.Replace(url)) default: - if inWsl() { - exe = "wslview" - } else { + if findExe("xdg-open") { exe = "xdg-open" + } else { + exe = "wslview" } args = append(args, url) } @@ -57,20 +56,11 @@ func FromLauncher(launcher, url string) (*exec.Cmd, error) { return cmd, nil } -// Determine if gh is running inside of WSL2 -func inWsl() bool { - file, err := os.Open("/proc/sys/kernel/osrelease") +var findExe = func(command string) bool { + _, err := exec.LookPath(command) if err != nil { return false + } else { + return true } - defer file.Close() - - scanner := bufio.NewScanner(file) - scanner.Scan() // Read single line - if err := scanner.Err(); err != nil { - return false - } - - os := scanner.Text() - return strings.Contains(os, "microsoft-WSL2") } diff --git a/pkg/browser/browser_test.go b/pkg/browser/browser_test.go index 749d0693e..dd007f950 100644 --- a/pkg/browser/browser_test.go +++ b/pkg/browser/browser_test.go @@ -11,9 +11,10 @@ func TestForOS(t *testing.T) { url string } tests := []struct { - name string - args args - want []string + name string + args args + findExe bool + want []string }{ { name: "macOS", @@ -29,7 +30,17 @@ func TestForOS(t *testing.T) { goos: "linux", url: "https://example.com/path?a=1&b=2", }, - want: []string{"xdg-open", "https://example.com/path?a=1&b=2"}, + findExe: true, + want: []string{"xdg-open", "https://example.com/path?a=1&b=2"}, + }, + { + name: "WSL", + args: args{ + goos: "linux", + url: "https://example.com/path?a=1&b=2", + }, + findExe: false, + want: []string{"wslview", "https://example.com/path?a=1&b=2"}, }, { name: "Windows", @@ -42,6 +53,7 @@ func TestForOS(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + findExe = func(string) bool { return tt.findExe } if cmd := ForOS(tt.args.goos, tt.args.url); !reflect.DeepEqual(cmd.Args, tt.want) { t.Errorf("ForOS() = %v, want %v", cmd.Args, tt.want) } From 7551139caf20e7dbfb8f0b55061ad1bf7d65e766 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 16 Sep 2020 16:04:58 +0200 Subject: [PATCH 3/5] Address PR comments --- pkg/browser/browser.go | 12 ++++-------- pkg/browser/browser_test.go | 4 ++-- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/pkg/browser/browser.go b/pkg/browser/browser.go index 6496966a4..d972542f6 100644 --- a/pkg/browser/browser.go +++ b/pkg/browser/browser.go @@ -30,10 +30,10 @@ func ForOS(goos, url string) *exec.Cmd { r := strings.NewReplacer("&", "^&") args = append(args, "/c", "start", r.Replace(url)) default: - if findExe("xdg-open") { - exe = "xdg-open" - } else { + if findExe("wslview") { exe = "wslview" + } else { + exe = "xdg-open" } args = append(args, url) } @@ -58,9 +58,5 @@ func FromLauncher(launcher, url string) (*exec.Cmd, error) { var findExe = func(command string) bool { _, err := exec.LookPath(command) - if err != nil { - return false - } else { - return true - } + return err == nil } diff --git a/pkg/browser/browser_test.go b/pkg/browser/browser_test.go index dd007f950..97e6b1a13 100644 --- a/pkg/browser/browser_test.go +++ b/pkg/browser/browser_test.go @@ -30,7 +30,7 @@ func TestForOS(t *testing.T) { goos: "linux", url: "https://example.com/path?a=1&b=2", }, - findExe: true, + findExe: false, // wslview does not exist on standard Linux want: []string{"xdg-open", "https://example.com/path?a=1&b=2"}, }, { @@ -39,7 +39,7 @@ func TestForOS(t *testing.T) { goos: "linux", url: "https://example.com/path?a=1&b=2", }, - findExe: false, + findExe: true, // wslview exists on WSL want: []string{"wslview", "https://example.com/path?a=1&b=2"}, }, { From fd1d09dfc21512e5eb69d762de4d61f2afac96ae Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 16 Sep 2020 16:56:47 +0200 Subject: [PATCH 4/5] Clean up linux logic to have better default logic --- pkg/browser/browser.go | 16 ++++++++++------ pkg/browser/browser_test.go | 18 +++++++++--------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/pkg/browser/browser.go b/pkg/browser/browser.go index d972542f6..2408a2e08 100644 --- a/pkg/browser/browser.go +++ b/pkg/browser/browser.go @@ -30,11 +30,7 @@ func ForOS(goos, url string) *exec.Cmd { r := strings.NewReplacer("&", "^&") args = append(args, "/c", "start", r.Replace(url)) default: - if findExe("wslview") { - exe = "wslview" - } else { - exe = "xdg-open" - } + exe = linuxExe() args = append(args, url) } @@ -56,7 +52,15 @@ func FromLauncher(launcher, url string) (*exec.Cmd, error) { return cmd, nil } -var findExe = func(command string) bool { +var linuxExe = func() string { + exe := "xdg-open" + if !findExe("xdg-open") && findExe("wslview") { + exe = "wslview" + } + return exe +} + +func findExe(command string) bool { _, err := exec.LookPath(command) return err == nil } diff --git a/pkg/browser/browser_test.go b/pkg/browser/browser_test.go index 97e6b1a13..11dd80372 100644 --- a/pkg/browser/browser_test.go +++ b/pkg/browser/browser_test.go @@ -11,10 +11,10 @@ func TestForOS(t *testing.T) { url string } tests := []struct { - name string - args args - findExe bool - want []string + name string + args args + exe string + want []string }{ { name: "macOS", @@ -30,8 +30,8 @@ func TestForOS(t *testing.T) { goos: "linux", url: "https://example.com/path?a=1&b=2", }, - findExe: false, // wslview does not exist on standard Linux - want: []string{"xdg-open", "https://example.com/path?a=1&b=2"}, + exe: "xdg-open", + want: []string{"xdg-open", "https://example.com/path?a=1&b=2"}, }, { name: "WSL", @@ -39,8 +39,8 @@ func TestForOS(t *testing.T) { goos: "linux", url: "https://example.com/path?a=1&b=2", }, - findExe: true, // wslview exists on WSL - want: []string{"wslview", "https://example.com/path?a=1&b=2"}, + exe: "wslview", + want: []string{"wslview", "https://example.com/path?a=1&b=2"}, }, { name: "Windows", @@ -53,7 +53,7 @@ func TestForOS(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - findExe = func(string) bool { return tt.findExe } + linuxExe = func() string { return tt.exe } if cmd := ForOS(tt.args.goos, tt.args.url); !reflect.DeepEqual(cmd.Args, tt.want) { t.Errorf("ForOS() = %v, want %v", cmd.Args, tt.want) } From 9058feebec47d343a42d45e29f058ad4c6b9a63b Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 16 Sep 2020 17:19:11 +0200 Subject: [PATCH 5/5] Fix up structure for better testing --- pkg/browser/browser.go | 17 ++++++++++------- pkg/browser/browser_test.go | 10 +++++++++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/pkg/browser/browser.go b/pkg/browser/browser.go index 2408a2e08..c710a3b38 100644 --- a/pkg/browser/browser.go +++ b/pkg/browser/browser.go @@ -52,15 +52,18 @@ func FromLauncher(launcher, url string) (*exec.Cmd, error) { return cmd, nil } -var linuxExe = func() string { +func linuxExe() string { exe := "xdg-open" - if !findExe("xdg-open") && findExe("wslview") { - exe = "wslview" + + _, err := lookPath(exe) + if err != nil { + _, err := lookPath("wslview") + if err == nil { + exe = "wslview" + } } + return exe } -func findExe(command string) bool { - _, err := exec.LookPath(command) - return err == nil -} +var lookPath = exec.LookPath diff --git a/pkg/browser/browser_test.go b/pkg/browser/browser_test.go index 11dd80372..48b91f7c1 100644 --- a/pkg/browser/browser_test.go +++ b/pkg/browser/browser_test.go @@ -1,6 +1,7 @@ package browser import ( + "errors" "reflect" "testing" ) @@ -52,8 +53,15 @@ func TestForOS(t *testing.T) { }, } for _, tt := range tests { + lookPath = func(file string) (string, error) { + if file == tt.exe { + return file, nil + } else { + return "", errors.New("not found") + } + } + t.Run(tt.name, func(t *testing.T) { - linuxExe = func() string { return tt.exe } if cmd := ForOS(tt.args.goos, tt.args.url); !reflect.DeepEqual(cmd.Args, tt.want) { t.Errorf("ForOS() = %v, want %v", cmd.Args, tt.want) }