From ebf8de2b1fbdad521333f5f3458020450dfde672 Mon Sep 17 00:00:00 2001 From: David Dworken Date: Sun, 23 Oct 2022 19:29:29 -0700 Subject: [PATCH] Refactor to enable control-r by default on upgrade + pave the way for prompts in the future --- .errcheck_excludes.txt | 1 + client/client_test.go | 42 ++++++++++-------------------------------- client/hctx/hctx.go | 8 +++++--- client/lib/lib.go | 31 ++++++++++--------------------- go.mod | 1 + go.sum | 1 + 6 files changed, 28 insertions(+), 56 deletions(-) diff --git a/.errcheck_excludes.txt b/.errcheck_excludes.txt index a147585..47e5326 100644 --- a/.errcheck_excludes.txt +++ b/.errcheck_excludes.txt @@ -3,3 +3,4 @@ os.Setenv (*os.File).Close (io.ReadCloser).Close +(*github.com/gofrs/flock.Flock).Unlock diff --git a/client/client_test.go b/client/client_test.go index e0ab0a7..1488607 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -146,7 +146,7 @@ func TestParameterized(t *testing.T) { t.Run("testMultipleUsers/"+tester.ShellName(), func(t *testing.T) { testMultipleUsers(t, tester) }) t.Run("testConfigGetSet/"+tester.ShellName(), func(t *testing.T) { testConfigGetSet(t, tester) }) t.Run("testControlR/"+tester.ShellName(), func(t *testing.T) { testControlR(t, tester, tester.ShellName()) }) - t.Run("testUpgradePromptControlR/"+tester.ShellName(), func(t *testing.T) { testUpgradePromptControlR(t, tester) }) + t.Run("testHandleUpgradedFeatures/"+tester.ShellName(), func(t *testing.T) { testHandleUpgradedFeatures(t, tester) }) // TODO: Add a test for multi-line history entries } t.Run("testControlR/fish", func(t *testing.T) { testControlR(t, bashTester{}, "fish") }) @@ -1589,18 +1589,20 @@ func clearControlRSearchFromConfig(t *testing.T) { configContents = []byte(strings.ReplaceAll(string(configContents), "enable_control_r_search", "something-else")) homedir, err := os.UserHomeDir() shared.Check(t, err) - err = os.WriteFile(path.Join(homedir, shared.HISHTORY_PATH, shared.CONFIG_PATH), configContents, 0o600) + err = os.WriteFile(path.Join(homedir, shared.HISHTORY_PATH, shared.CONFIG_PATH), configContents, 0o644) shared.Check(t, err) } -func testUpgradePromptControlR(t *testing.T, tester shellTester) { +func testHandleUpgradedFeatures(t *testing.T, tester shellTester) { // Setup defer shared.BackupAndRestore(t)() installHishtory(t, tester, "") - // Install, and there is no prompt since the config mentions control-r - tester.RunInteractiveShell(t, `/tmp/client install`) - tester.RunInteractiveShell(t, `hishtory disable`) + // Install, and there is no prompt since the config already mentions control-r + _, err := tester.RunInteractiveShellRelaxed(t, `/tmp/client install`) + shared.Check(t, err) + _, err = tester.RunInteractiveShellRelaxed(t, `hishtory disable`) + shared.Check(t, err) // Ensure that the config doesn't mention control-r clearControlRSearchFromConfig(t) @@ -1611,39 +1613,15 @@ func testUpgradePromptControlR(t *testing.T, tester shellTester) { t.Fatalf("unexpected config-get output: %#v", out) } - // And install again, this time we should get a prompt + // And install again, this time it will get set to true by default clearControlRSearchFromConfig(t) - out, err := tester.RunInteractiveShellRelaxed(t, `yes | /tmp/client install`) - shared.Check(t, err) - expected := "The new version of hishtory supports binding to your shell's control-R key binding for searching. Do you want to enable this? [y/N] Enabled the control-R key binding, please restart your shell for this to take effect\n" - if !strings.HasPrefix(out, expected) { - t.Fatalf("hishtory install mismatch, out=%#v", out) - } + tester.RunInteractiveShell(t, ` /tmp/client install`) // Now it should be enabled out = tester.RunInteractiveShell(t, `hishtory config-get enable-control-r`) if out != "true" { t.Fatalf("unexpected config-get output: %#v", out) } - - // Clear it again, and this time we'll respond no - clearControlRSearchFromConfig(t) - out = tester.RunInteractiveShell(t, `hishtory config-get enable-control-r`) - if out != "false" { - t.Fatalf("unexpected config-get output: %#v", out) - } - clearControlRSearchFromConfig(t) - out, err = tester.RunInteractiveShellRelaxed(t, `echo n | /tmp/client install`) - shared.Check(t, err) - if strings.Contains(out, expected) || strings.Contains(out, "Enabled the control-R key binding") { - t.Fatalf("hishtory install mismatch, out=%#v", out) - } - - // Now it should be disabled - out = tester.RunInteractiveShell(t, `hishtory config-get enable-control-r`) - if out != "false" { - t.Fatalf("unexpected config-get output: %#v", out) - } } func TestFish(t *testing.T) { diff --git a/client/hctx/hctx.go b/client/hctx/hctx.go index 3657059..b020183 100644 --- a/client/hctx/hctx.go +++ b/client/hctx/hctx.go @@ -207,13 +207,15 @@ func SetConfig(config ClientConfig) error { if err != nil { return err } - err = os.WriteFile(path.Join(homedir, shared.HISHTORY_PATH, shared.CONFIG_PATH+".tmp"), serializedConfig, 0o600) + configPath := path.Join(homedir, shared.HISHTORY_PATH, shared.CONFIG_PATH) + stagedConfigPath := configPath + ".tmp" + err = os.WriteFile(stagedConfigPath, serializedConfig, 0o644) if err != nil { return fmt.Errorf("failed to write config: %v", err) } - err = os.Rename(path.Join(homedir, shared.HISHTORY_PATH, shared.CONFIG_PATH+".tmp"), path.Join(homedir, shared.HISHTORY_PATH, shared.CONFIG_PATH)) + err = os.Rename(stagedConfigPath, configPath) if err != nil { - return fmt.Errorf("failed to replace config file with the rewritten version: %v", err) + return fmt.Errorf("failed to replace config file with the updated version: %v", err) } return nil } diff --git a/client/lib/lib.go b/client/lib/lib.go index 0c35329..2e8b359 100644 --- a/client/lib/lib.go +++ b/client/lib/lib.go @@ -547,7 +547,7 @@ func Install() error { if err != nil { return err } - err = promptOnUpgradedFeatures() + err = handleUpgradedFeatures() if err != nil { return err } @@ -559,35 +559,23 @@ func Install() error { return nil } -func promptOnUpgradedFeatures() error { +func handleUpgradedFeatures() error { configConents, err := hctx.GetConfigContents() if err != nil { - // No config, so this is a new install and thus there is nothing to prompt on + // No config, so this is a new install and thus there is nothing to do return nil } if strings.Contains(string(configConents), "enable_control_r_search") { - // control-r search is already configured, so there is nothing to prompt on - return nil - } - fmt.Printf("The new version of hishtory supports binding to your shell's control-R key binding for searching. Do you want to enable this? [y/N] ") - reader := bufio.NewReader(os.Stdin) - resp, err := reader.ReadString('\n') - CheckFatalError(err) - if strings.TrimSpace(resp) != "y" { - // They don't want it, so nothing to do + // control-r search is already configured, so there is nothing to do return nil } + // Enable control-r search config, err := hctx.GetConfig() if err != nil { return err } config.ControlRSearchEnabled = true - err = hctx.SetConfig(config) - if err != nil { - return err - } - fmt.Printf("Enabled the control-R key binding, please restart your shell for this to take effect\n") - return nil + return hctx.SetConfig(config) } func configureFish(homedir, binaryPath string) error { @@ -853,14 +841,15 @@ func Update(ctx *context.Context) error { cmd.Stderr = &stderr err = cmd.Run() if err != nil { - return fmt.Errorf("failed to chmod +x the update (out=%#v, err=%#v): %v", stdout.String(), stderr.String(), err) + return fmt.Errorf("failed to chmod +x the update (stdout=%#v, stderr=%#v): %v", stdout.String(), stderr.String(), err) } cmd = exec.Command("/tmp/hishtory-client", "install") - stdout = bytes.Buffer{} + cmd.Stdout = os.Stdout stderr = bytes.Buffer{} + cmd.Stdin = os.Stdin err = cmd.Run() if err != nil { - return fmt.Errorf("failed to install update (out=%#v, err=%#v): %v", stdout.String(), stderr.String(), err) + return fmt.Errorf("failed to install update (stderr=%#v): %v", stderr.String(), err) } fmt.Printf("Successfully updated hishtory from v0.%s to %s\n", Version, downloadData.Version) return nil diff --git a/go.mod b/go.mod index 8736a40..7a29372 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/fatih/color v1.13.0 github.com/glebarez/sqlite v1.4.7 github.com/go-test/deep v1.0.8 + github.com/gofrs/flock v0.8.1 github.com/google/go-cmp v0.5.9 github.com/google/uuid v1.3.0 github.com/lib/pq v1.10.4 diff --git a/go.sum b/go.sum index 7a4920a..82bcc50 100644 --- a/go.sum +++ b/go.sum @@ -814,6 +814,7 @@ github.com/godbus/dbus v0.0.0-20190422162347-ade71ed3457e/go.mod h1:bBOAhwG1umN6 github.com/godbus/dbus v4.1.0+incompatible/go.mod h1:/YcGZj5zSblfDWMMoOzV4fas9FZnQYTkDnsGvmh2Grw= github.com/godbus/dbus/v5 v5.0.3/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= +github.com/gofrs/flock v0.8.1 h1:+gYjHKf32LDeiEEFhQaotPbLuUXjY5ZqxKgXy7n59aw= github.com/gofrs/flock v0.8.1/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU= github.com/gofrs/uuid v4.0.0+incompatible h1:1SD/1F5pU8p29ybwgQSwpQk+mwdRrXCYuPhW6m+TnJw= github.com/gofrs/uuid v4.0.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM=