From 86e4d47b375050f06d1e181f7e1a08019b2d644c Mon Sep 17 00:00:00 2001 From: Kenny Pitt Date: Tue, 20 Sep 2022 11:36:06 -0400 Subject: [PATCH 1/3] Remove support for the `autoclip` option --- docs/commands/config.md | 4 ++-- docs/commands/generate.md | 6 +----- docs/config.md | 5 ++--- docs/features.md | 5 +---- internal/action/config_test.go | 9 +++------ internal/action/copy_test.go | 4 ---- internal/action/find_test.go | 2 -- internal/action/generate.go | 13 ++----------- internal/action/generate_test.go | 28 ---------------------------- internal/action/insert_test.go | 4 ---- internal/config/config.go | 1 - internal/config/config_test.go | 8 ++++---- internal/config/io_test.go | 5 +---- internal/config/legacy.go | 1 - tests/config_test.go | 6 ++---- tests/gptest/unit.go | 3 +-- tests/show_test.go | 7 ------- 17 files changed, 19 insertions(+), 92 deletions(-) diff --git a/docs/commands/config.md b/docs/commands/config.md index 09c92cd383..20f550e663 100644 --- a/docs/commands/config.md +++ b/docs/commands/config.md @@ -8,8 +8,8 @@ Note: To manage mounts use `gopass mounts`. ``` $ gopass config -$ gopass config autoclip -$ gopass config autoclip false +$ gopass config autoimport +$ gopass config autoimport false ``` ## Flags diff --git a/docs/commands/generate.md b/docs/commands/generate.md index a923105655..d084087316 100644 --- a/docs/commands/generate.md +++ b/docs/commands/generate.md @@ -22,7 +22,7 @@ $ gopass generate entry key [length] Flag | Aliases | Description ---- | ------- | ----------- -`--clip` | `-c` | Copy the generated password into the clipboard. Default: Value of `autoclip` +`--clip` | `-c` | Copy the generated password into the clipboard. Default: false. `--print` | `-p` | Print the generated password to the terminal. Default: false. `--force` | `-f` | Force overwriting an existing entry. `--edit` | `-e` | Generate a password and open the entry for editing in `$EDITOR`. @@ -43,10 +43,6 @@ Generator | Description `memorable` | Generate a memorable password. The length argument specifies the minimum lenght of characters. Please note that the password might be longer if not all necessary rules were satisfied by the minimum length solution. `external` | Use the external generator from `$GOPASS_EXTERNAL_PWGEN` -## Relevant configuration options - -* `autoclip` only applies to `generate`. If set the generated password is automatically copied to the clipboard - unless `--clip` is explicitly set to `--clip=false` - ## Templates When creating a new entry gopass will look for the most specific template diff --git a/docs/config.md b/docs/config.md index 888cb3f407..1a436f517b 100644 --- a/docs/config.md +++ b/docs/config.md @@ -45,15 +45,14 @@ During start up, gopass will look for a configuration file at `$HOME/.config/gop All configuration options are also available for reading and writing through the sub-command `gopass config`. * To display all values: `gopass config` -* To display a single value: `gopass config autoclip` -* To update a single value: `gopass config autoclip false` +* To display a single value: `gopass config autoimport` +* To update a single value: `gopass config autoimport false` * As many other sub-commands this command accepts a `--store` flag to operate on a given sub-store, provided the sub-store is a remote one. This is a list of available options: | **Option** | **Type** | Description | | ---------------- | -------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `autoclip` | `bool` | Always copy the password created by `gopass generate`. Only applies to generate. | | `autoimport` | `bool` | Import missing keys stored in the pass repository without asking. | | `cliptimeout` | `int` | How many seconds the secret is stored when using `-c`. | | `exportkeys` | `bool` | Export public keys of all recipients to the store. | diff --git a/docs/features.md b/docs/features.md index c0c4cab5bd..16e372a692 100644 --- a/docs/features.md +++ b/docs/features.md @@ -142,9 +142,7 @@ The generated password for golang.org/gopher is: Eech4ahRoy2oowi0ohl ``` -The `generate` command will ask for any missing arguments, like the name of the secret or the length. By default the password is copied to clipboard. If you don't want the password to be copied, but displayed instead, use the `-p` flag to print it. - -By default the password is copied to clipboard, but you can disable this using the `AutoClip` option, which, when set to`false`, will neither display, nor print the password. This is overridden by the `-p` or `-c` flags. +The `generate` command will ask for any missing arguments, like the name of the secret or the length. Use the `-p` flag to print the generated password, or the `-c` flag to copy it to the clipboard. ### Edit a secret @@ -410,7 +408,6 @@ gopass allows editing the config from the command-line. This is similar to how g ```bash $ gopass config askformore: false -autoclip: true autoimport: false cliptimeout: 10 noconfirm: false diff --git a/internal/action/config_test.go b/internal/action/config_test.go index 98d3b1dbea..692ca51f03 100644 --- a/internal/action/config_test.go +++ b/internal/action/config_test.go @@ -37,8 +37,7 @@ func TestConfig(t *testing.T) { //nolint:paralleltest c := gptest.CliCtx(ctx, t) assert.NoError(t, act.Config(c)) - want := `autoclip: true -autoimport: true + want := `autoimport: true cliptimeout: 45 exportkeys: true nopager: false @@ -74,8 +73,7 @@ parsing: true defer buf.Reset() act.printConfigValues(ctx) - want := `autoclip: true -autoimport: true + want := `autoimport: true cliptimeout: 45 exportkeys: true nopager: true @@ -107,8 +105,7 @@ parsing: true defer buf.Reset() act.ConfigComplete(gptest.CliCtx(ctx, t)) - want := `autoclip -autoimport + want := `autoimport cliptimeout exportkeys nopager diff --git a/internal/action/copy_test.go b/internal/action/copy_test.go index f5e17e2a95..6c2e7db0a0 100644 --- a/internal/action/copy_test.go +++ b/internal/action/copy_test.go @@ -27,8 +27,6 @@ func TestCopy(t *testing.T) { //nolint:paralleltest require.NoError(t, err) require.NotNil(t, act) - act.cfg.AutoClip = false - buf := &bytes.Buffer{} out.Stdout = buf stdout = buf @@ -110,8 +108,6 @@ func TestCopyGpg(t *testing.T) { //nolint:paralleltest require.NoError(t, err) require.NotNil(t, act) - act.cfg.AutoClip = false - buf := &bytes.Buffer{} out.Stdout = buf stdout = buf diff --git a/internal/action/find_test.go b/internal/action/find_test.go index bdb948e4ff..224f24effa 100644 --- a/internal/action/find_test.go +++ b/internal/action/find_test.go @@ -30,8 +30,6 @@ func TestFind(t *testing.T) { //nolint:paralleltest require.NoError(t, err) require.NotNil(t, act) - act.cfg.AutoClip = false - buf := &bytes.Buffer{} out.Stdout = buf stdout = buf diff --git a/internal/action/generate.go b/internal/action/generate.go index 2d0340a348..7dfd5973c2 100644 --- a/internal/action/generate.go +++ b/internal/action/generate.go @@ -133,20 +133,11 @@ func (s *Action) generateCopyOrPrint(ctx context.Context, c *cli.Context, name, out.OKf(ctx, "Password for entry %q generated", entry) - // copy to clipboard if: - // - explicitly requested with -c - // - autoclip=true, but only if output is not being redirected. - if IsClip(ctx) || (s.cfg.AutoClip && ctxutil.IsTerminal(ctx)) { + // copy to clipboard if explicitly requested with -c + if IsClip(ctx) { if err := clipboard.CopyTo(ctx, name, []byte(password), s.cfg.ClipTimeout); err != nil { return exit.Error(exit.IO, err, "failed to copy to clipboard: %s", err) } - // if autoclip is on and we're not printing the password to the terminal - // at least leave a notice that we did indeed copy it. - if s.cfg.AutoClip && !c.Bool("print") { - out.Print(ctx, "Copied to clipboard") - - return nil - } } if !c.Bool("print") { diff --git a/internal/action/generate_test.go b/internal/action/generate_test.go index 6cc66bd7ed..221dec0c1a 100644 --- a/internal/action/generate_test.go +++ b/internal/action/generate_test.go @@ -39,8 +39,6 @@ func TestGenerate(t *testing.T) { //nolint:paralleltest require.NoError(t, err) require.NotNil(t, act) - act.cfg.AutoClip = false - buf := &bytes.Buffer{} out.Stdout = buf out.Stderr = buf @@ -153,32 +151,6 @@ func TestGenerate(t *testing.T) { //nolint:paralleltest buf.Reset() }) - // generate --force foobar 24 w/ autoclip and output redirection - t.Run("generate --force foobar 24", func(t *testing.T) { //nolint:paralleltest - ov := act.cfg.AutoClip - defer func() { - act.cfg.AutoClip = ov - }() - act.cfg.AutoClip = true - ctx := ctxutil.WithTerminal(ctx, false) - assert.NoError(t, act.Generate(gptest.CliCtxWithFlags(ctx, t, map[string]string{"force": "true"}, "foobar", "24"))) - assert.Contains(t, buf.String(), "Not printing secrets by default") - buf.Reset() - }) - - // generate --force foobar 24 w/ autoclip and no output redirection - t.Run("generate --force foobar 24", func(t *testing.T) { //nolint:paralleltest - ov := act.cfg.AutoClip - defer func() { - act.cfg.AutoClip = ov - }() - act.cfg.AutoClip = true - ctx := ctxutil.WithTerminal(ctx, true) - assert.NoError(t, act.Generate(gptest.CliCtxWithFlags(ctx, t, map[string]string{"force": "true"}, "foobar", "24"))) - assert.Contains(t, buf.String(), "Copied to clipboard") - buf.Reset() - }) - // generate --force foobar w/ pw length set via env variable (42 chars) t.Run("generate --force foobar", func(t *testing.T) { //nolint:paralleltest t.Setenv("GOPASS_PW_DEFAULT_LENGTH", "42") diff --git a/internal/action/insert_test.go b/internal/action/insert_test.go index e02e3b114a..27f053ca2a 100644 --- a/internal/action/insert_test.go +++ b/internal/action/insert_test.go @@ -27,8 +27,6 @@ func TestInsert(t *testing.T) { //nolint:paralleltest require.NoError(t, err) require.NotNil(t, act) - act.cfg.AutoClip = false - buf := &bytes.Buffer{} out.Stdout = buf color.NoColor = true @@ -132,8 +130,6 @@ func TestInsertStdin(t *testing.T) { //nolint:paralleltest require.NoError(t, err) require.NotNil(t, act) - act.cfg.AutoClip = false - buf := &bytes.Buffer{} ibuf := &bytes.Buffer{} out.Stdout = buf diff --git a/internal/config/config.go b/internal/config/config.go index f2bb931826..505ac0be93 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -17,7 +17,6 @@ var ( // Config is the current config struct. type Config struct { - AutoClip bool `yaml:"autoclip"` // decide whether passwords are automatically copied or not. AutoImport bool `yaml:"autoimport"` // import missing public keys w/o asking. ClipTimeout int `yaml:"cliptimeout"` // clear clipboard after seconds. ExportKeys bool `yaml:"exportkeys"` // automatically export public keys of all recipients. diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 37e11d8ab2..aec82a5278 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -20,7 +20,7 @@ func TestNewConfig(t *testing.T) { //nolint:paralleltest cfg := config.New() cs := cfg.String() - assert.Contains(t, cs, `&config.Config{AutoClip:false, AutoImport:false, ClipTimeout:45, ExportKeys:true, NoPager:false,`) + assert.Contains(t, cs, `&config.Config{AutoImport:false, ClipTimeout:45, ExportKeys:true, NoPager:false,`) assert.Contains(t, cs, `Mounts:map[string]string{},`) cfg = &config.Config{ @@ -30,7 +30,7 @@ func TestNewConfig(t *testing.T) { //nolint:paralleltest }, } cs = cfg.String() - assert.Contains(t, cs, `&config.Config{AutoClip:false, AutoImport:false, ClipTimeout:0, ExportKeys:false, NoPager:false,`) + assert.Contains(t, cs, `&config.Config{AutoImport:false, ClipTimeout:0, ExportKeys:false, NoPager:false,`) assert.Contains(t, cs, `Mounts:map[string]string{"bar":"", "foo":""},`) } @@ -39,8 +39,8 @@ func TestSetConfigValue(t *testing.T) { //nolint:paralleltest defer u.Remove() cfg := config.New() - assert.NoError(t, cfg.SetConfigValue("autoclip", "true")) + assert.NoError(t, cfg.SetConfigValue("autoimport", "true")) assert.NoError(t, cfg.SetConfigValue("cliptimeout", "900")) assert.NoError(t, cfg.SetConfigValue("path", "/tmp")) - assert.Error(t, cfg.SetConfigValue("autoclip", "yo")) + assert.Error(t, cfg.SetConfigValue("autoimport", "yo")) } diff --git a/internal/config/io_test.go b/internal/config/io_test.go index 923cd04e27..c6df8abac6 100644 --- a/internal/config/io_test.go +++ b/internal/config/io_test.go @@ -38,7 +38,6 @@ mounts: foo/sub: /home/johndoe/.password-store-foo-sub work: /home/johndoe/.password-store-work`, want: &Config{ - AutoClip: true, AutoImport: false, ClipTimeout: 45, ExportKeys: true, @@ -52,8 +51,7 @@ mounts: }, }, { name: "N+1", - cfg: `autoclip: true -autoimport: false + cfg: `autoimport: false cliptimeout: 45 exportkeys: true nopager: false @@ -63,7 +61,6 @@ mounts: foo/sub: /home/johndoe/.password-store-foo-sub work: /home/johndoe/.password-store-work`, want: &Config{ - AutoClip: true, AutoImport: false, ClipTimeout: 45, ExportKeys: true, diff --git a/internal/config/legacy.go b/internal/config/legacy.go index 862073a0c1..8575601fff 100644 --- a/internal/config/legacy.go +++ b/internal/config/legacy.go @@ -22,7 +22,6 @@ type Pre1150 struct { // Config converts the Pre1127 config to the current config struct. func (c *Pre1150) Config() *Config { cfg := &Config{ - AutoClip: c.AutoClip, AutoImport: c.AutoImport, ClipTimeout: c.ClipTimeout, ExportKeys: c.ExportKeys, diff --git a/tests/config_test.go b/tests/config_test.go index 146c7dc6a6..d3f92bedd7 100644 --- a/tests/config_test.go +++ b/tests/config_test.go @@ -15,8 +15,7 @@ func TestBaseConfig(t *testing.T) { //nolint:paralleltest out, err := ts.run("config") assert.NoError(t, err) - wanted := `autoclip: false -autoimport: true + wanted := `autoimport: true cliptimeout: 45 exportkeys: false nopager: false @@ -65,8 +64,7 @@ func TestMountConfig(t *testing.T) { //nolint:paralleltest _, err = ts.run("config") assert.NoError(t, err) - wanted := `autoclip: false -autoimport: true + wanted := `autoimport: true cliptimeout: 45 exportkeys: false nopager: false diff --git a/tests/gptest/unit.go b/tests/gptest/unit.go index d352007125..e9f9fcff21 100644 --- a/tests/gptest/unit.go +++ b/tests/gptest/unit.go @@ -12,8 +12,7 @@ import ( ) const ( - gopassConfig = `autoclip: true -autoimport: true + gopassConfig = `autoimport: true cliptimeout: 45 parsing: true` ) diff --git a/tests/show_test.go b/tests/show_test.go index 9a44552ed4..4788fdb3ab 100644 --- a/tests/show_test.go +++ b/tests/show_test.go @@ -50,11 +50,4 @@ func TestShow(t *testing.T) { //nolint:paralleltest assert.NoError(t, err) assert.Equal(t, goldenQr, out) }) - - t.Run("show w/o autoclip", func(t *testing.T) { //nolint:paralleltest - _, err = ts.run("config autoclip false") - assert.NoError(t, err) - _, err = ts.run("show fixed/secret") - assert.NoError(t, err) - }) } From ccab0ac84b225dd111600d4e369408ad87eddc19 Mon Sep 17 00:00:00 2001 From: Kenny Pitt Date: Tue, 20 Sep 2022 15:12:45 -0400 Subject: [PATCH 2/3] Remove `--alsoclip` option --- docs/commands/show.md | 2 -- internal/action/commands.go | 5 ----- internal/action/context.go | 34 --------------------------------- internal/action/context_test.go | 18 ----------------- internal/action/find.go | 1 - internal/action/show.go | 9 ++------- internal/action/show_test.go | 24 ----------------------- 7 files changed, 2 insertions(+), 91 deletions(-) diff --git a/docs/commands/show.md b/docs/commands/show.md index 99aa9dfafc..d1d543247b 100644 --- a/docs/commands/show.md +++ b/docs/commands/show.md @@ -22,7 +22,6 @@ $ gopass show entry --password Flag | Aliases | Description ---- | ------- | ----------- `--clip` | `-c` | Copy the password value into the clipboard and don't show the content. -`--alsoclip` | `-C` | Copy the password value into the clipboard and show the content. `--qr` | | Encode the password field as a QR code and print it. Note: When combining with `-c`/`-C` the unencoded password is copied. Not the QR code. `--password` | `-o` | Display only the password. For use in scripts. Takes precedence over other flags. `--revision` | `-r` | Display a specific revision of the entry. Use an exact version identifier from `gopass history` or the special `-` syntax. Does not work with native (e.g. git) refs. @@ -42,7 +41,6 @@ TODO: We need to specify the expectations around new lines. * When no flag is set the `show` command will display the full content of the secret and will parse it to support key-value lookup and YAML entries. * The `--noparsing` flag will disable all parsing of the output, this can help debugging YAML secrets for example, where `key: 0123` actually parses into octal for 83. * The `--clip` flag will copy the value of the `Password` field to the clipboard and doesn't display any part of the secret. -* The `--alsoclip` option will copy the value of the `Password` field but also display the secret content depending on the `safecontent` setting, i.e. obstructing the `Password` field if `safecontent` is `true` or just displaying it if not. * The `--qr` flags operates complementary to other flags. It will *additionally* format the value of the `Password` entry as a QR code and display it. Other than that it will honor the other options, e.g. `gopass show --qr` will display the QR code *and* the whole secret content below. One special case is the `-o` flag, this flag doesn't make a lot of sense in combination, so if both `--qr` and `-o` are given only the QR code will be displayed. * Arbitrary git refs are not supported as arguments to the `--revision` flag. Using those might work, but this is explicitly not supported and bug reports will be closed as `wont-fix`. The main issue with using arbitrary git refs is that git versions a whole repository, not single files. So the revision `HEAD^` might not have any changes for a given entry. Thus we only support specifc revisions obtained from `gopass history` or our custom syntax `-N` where N is an integer identifying a specific commit before `HEAD` (cf. `HEAD~N`). diff --git a/internal/action/commands.go b/internal/action/commands.go index 45c4254476..f17832eb3b 100644 --- a/internal/action/commands.go +++ b/internal/action/commands.go @@ -22,11 +22,6 @@ func ShowFlags() []cli.Flag { Aliases: []string{"c"}, Usage: "Copy the password value into the clipboard", }, - &cli.BoolFlag{ - Name: "alsoclip", - Aliases: []string{"C"}, - Usage: "Copy the password and show everything", - }, &cli.BoolFlag{ Name: "qr", Usage: "Print the password as a QR Code", diff --git a/internal/action/context.go b/internal/action/context.go index 8056c320cc..79b5820031 100644 --- a/internal/action/context.go +++ b/internal/action/context.go @@ -10,8 +10,6 @@ const ( ctxKeyPrintQR ctxKeyRevision ctxKeyKey - ctxKeyOnlyClip - ctxKeyAlsoClip ctxKeyPrintChars ) @@ -31,38 +29,6 @@ func IsClip(ctx context.Context) bool { return bv } -// WithAlsoClip returns a context with the value for alsoclip (copy to -// clipboard and print to stdout) set. -func WithAlsoClip(ctx context.Context, clip bool) context.Context { - return context.WithValue(ctx, ctxKeyAlsoClip, clip) -} - -// IsAlsoClip returns the value for alsoclip of the dfeault (false). -func IsAlsoClip(ctx context.Context) bool { - bv, ok := ctx.Value(ctxKeyAlsoClip).(bool) - if !ok { - return false - } - - return bv -} - -// WithOnlyClip returns a context with the value for clip (for copy to clipboard) -// set. -func WithOnlyClip(ctx context.Context, clip bool) context.Context { - return context.WithValue(ctx, ctxKeyOnlyClip, clip) -} - -// IsOnlyClip returns the value of clip or the default (false). -func IsOnlyClip(ctx context.Context) bool { - bv, ok := ctx.Value(ctxKeyOnlyClip).(bool) - if !ok { - return false - } - - return bv -} - // WithPasswordOnly returns a context with the value of password only set. func WithPasswordOnly(ctx context.Context, pw bool) context.Context { return context.WithValue(ctx, ctxKeyPasswordOnly, pw) diff --git a/internal/action/context_test.go b/internal/action/context_test.go index 857a08f3c0..2f3899e9b3 100644 --- a/internal/action/context_test.go +++ b/internal/action/context_test.go @@ -63,21 +63,3 @@ func TestWithKey(t *testing.T) { assert.Equal(t, "", GetKey(ctx)) assert.Equal(t, "foo", GetKey(WithKey(ctx, "foo"))) } - -func TestWithOnlyClip(t *testing.T) { - t.Parallel() - - ctx := context.Background() - - assert.False(t, IsOnlyClip(ctx)) - assert.True(t, IsOnlyClip(WithOnlyClip(ctx, true))) -} - -func TestWithAlsoClip(t *testing.T) { - t.Parallel() - - ctx := context.Background() - - assert.False(t, IsAlsoClip(ctx)) - assert.True(t, IsAlsoClip(WithAlsoClip(ctx, true))) -} diff --git a/internal/action/find.go b/internal/action/find.go index 118f96c2b6..d29900ba40 100644 --- a/internal/action/find.go +++ b/internal/action/find.go @@ -29,7 +29,6 @@ func (s *Action) Find(c *cli.Context) error { func (s *Action) findCmd(c *cli.Context, cb showFunc, fuzzy bool) error { ctx := ctxutil.WithGlobalFlags(c) if c.IsSet("clip") { - ctx = WithOnlyClip(ctx, c.Bool("clip")) ctx = WithClip(ctx, c.Bool("clip")) } diff --git a/internal/action/show.go b/internal/action/show.go index 1b104a13ce..c4f32074b3 100644 --- a/internal/action/show.go +++ b/internal/action/show.go @@ -23,7 +23,7 @@ import ( func showParseArgs(c *cli.Context) context.Context { ctx := ctxutil.WithGlobalFlags(c) if c.IsSet("clip") { - ctx = WithOnlyClip(ctx, c.Bool("clip")) + ctx = WithClip(ctx, c.Bool("clip")) } if c.IsSet("qr") { @@ -38,10 +38,6 @@ func showParseArgs(c *cli.Context) context.Context { ctx = WithRevision(ctx, c.String("revision")) } - if c.IsSet("alsoclip") { - ctx = WithAlsoClip(ctx, c.Bool("alsoclip")) - } - if c.IsSet("noparsing") { ctx = ctxutil.WithShowParsing(ctx, !c.Bool("noparsing")) } @@ -61,7 +57,6 @@ func showParseArgs(c *cli.Context) context.Context { } ctx = WithPrintChars(ctx, iv) } - ctx = WithClip(ctx, IsOnlyClip(ctx) || IsAlsoClip(ctx)) return ctx } @@ -232,7 +227,7 @@ func (s *Action) showGetContent(ctx context.Context, sec gopass.Secret) (string, fullBody := string(sec.Bytes()) // first line of the secret only. - if IsPrintQR(ctx) || IsOnlyClip(ctx) { + if IsPrintQR(ctx) || IsClip(ctx) { return pw, "", nil } if IsPasswordOnly(ctx) { diff --git a/internal/action/show_test.go b/internal/action/show_test.go index 2ba50269ac..214c5dd3b1 100644 --- a/internal/action/show_test.go +++ b/internal/action/show_test.go @@ -184,18 +184,6 @@ func TestShowAutoClip(t *testing.T) { //nolint:paralleltest stderrBuf.Reset() }) - // gopass show -C foo - // -> Copy to clipboard AND print - t.Run("gopass show -C foo", func(t *testing.T) { //nolint:paralleltest - c := gptest.CliCtxWithFlags(ctx, t, map[string]string{"alsoclip": "true"}, "foo") - assert.NoError(t, act.Show(c)) - assert.Contains(t, stderrBuf.String(), "WARNING") - assert.Contains(t, stdoutBuf.String(), "secret") - assert.Contains(t, stdoutBuf.String(), "second") - stdoutBuf.Reset() - stderrBuf.Reset() - }) - // gopass show foo // -> Copy to clipboard t.Run("gopass show foo", func(t *testing.T) { //nolint:paralleltest @@ -217,18 +205,6 @@ func TestShowAutoClip(t *testing.T) { //nolint:paralleltest stdoutBuf.Reset() stderrBuf.Reset() }) - - // gopass show -C foo - // -> Copy to clipboard AND print - t.Run("gopass show -C foo", func(t *testing.T) { //nolint:paralleltest - c := gptest.CliCtxWithFlags(ctx, t, map[string]string{"alsoclip": "true"}, "foo") - assert.NoError(t, act.Show(c)) - assert.Contains(t, stderrBuf.String(), "WARNING") - assert.Contains(t, stdoutBuf.String(), "secret") - assert.Contains(t, stdoutBuf.String(), "second") - stdoutBuf.Reset() - stderrBuf.Reset() - }) } func TestShowHandleRevision(t *testing.T) { //nolint:paralleltest From 743b4ffedf34ddfe419adbca151a30b1c218f758 Mon Sep 17 00:00:00 2001 From: Kenny Pitt Date: Tue, 20 Sep 2022 18:32:51 -0400 Subject: [PATCH 3/3] Remove `--clip` option for `find` command When invoked as `gopass find`, there is no code-path where a value could be copied to the clipboard, so the `--clip` option is always ignored. --- docs/commands/find.md | 13 ++----------- internal/action/commands.go | 15 +++------------ internal/action/find.go | 25 +++++++++---------------- internal/action/find_test.go | 16 ++++++++-------- internal/action/show.go | 9 ++++----- 5 files changed, 26 insertions(+), 52 deletions(-) diff --git a/docs/commands/find.md b/docs/commands/find.md index 28a2c91ec5..4954cc6531 100644 --- a/docs/commands/find.md +++ b/docs/commands/find.md @@ -1,8 +1,7 @@ # `find` command -The `find` command will attempt to do a simple substring match on the names of all secrets. -If there is a single match it will directly invoke `show` and display the result. -If there are multiple matches a selection will be shown. +The `find` command does a simple substring match on the names of all secrets, +and lists the matching secrets. Note: The find command will not fall back to a fuzzy search. @@ -10,12 +9,4 @@ Note: The find command will not fall back to a fuzzy search. ``` $ gopass find entry -$ gopass find -f entry -$ gopass find -c entry ``` - -## Flags - -Flag | Aliases | Description ----- | ------- | ----------- -`--clip` | `-c` | Copy the password into the clipboard. diff --git a/internal/action/commands.go b/internal/action/commands.go index f17832eb3b..91838eda0e 100644 --- a/internal/action/commands.go +++ b/internal/action/commands.go @@ -297,22 +297,13 @@ func (s *Action) GetCommands() []*cli.Command { { Name: "find", Usage: "Search for secrets", - ArgsUsage: "[needle]", + ArgsUsage: "", Description: "" + - "This command will first attempt a simple pattern match on the name of the " + - "secret. If there is an exact match it will be shown directly; if there are " + - "multiple matches, a selection will be shown.", + "List all secrets that match the specified search pattern.", Before: s.IsInitialized, - Action: s.FindNoFuzzy, + Action: s.Find, Aliases: []string{"search"}, BashComplete: s.Complete, - Flags: []cli.Flag{ - &cli.BoolFlag{ - Name: "clip", - Aliases: []string{"c"}, - Usage: "Copy the password into the clipboard", - }, - }, }, { Name: "fsck", diff --git a/internal/action/find.go b/internal/action/find.go index d29900ba40..0fcd85d79c 100644 --- a/internal/action/find.go +++ b/internal/action/find.go @@ -16,27 +16,20 @@ import ( "github.com/urfave/cli/v2" ) -// FindNoFuzzy runs find without fuzzy search. -func (s *Action) FindNoFuzzy(c *cli.Context) error { - return s.findCmd(c, nil, false) -} - -// Find runs find. +// Find runs the find command action, without fuzzy search. func (s *Action) Find(c *cli.Context) error { - return s.findCmd(c, s.show, true) -} + if !c.Args().Present() { + return exit.Error(exit.Usage, nil, "Usage: %s find ", s.Name) + } -func (s *Action) findCmd(c *cli.Context, cb showFunc, fuzzy bool) error { ctx := ctxutil.WithGlobalFlags(c) - if c.IsSet("clip") { - ctx = WithClip(ctx, c.Bool("clip")) - } - if !c.Args().Present() { - return exit.Error(exit.Usage, nil, "Usage: %s find ", s.Name) - } + return s.find(ctx, c, c.Args().First(), nil, false) +} - return s.find(ctx, c, c.Args().First(), cb, fuzzy) +// FindFuzzy runs a fuzzy find of the specified name and attempts to show the result. +func (s *Action) FindFuzzy(c *cli.Context, name string) error { + return s.find(c.Context, c, name, s.show, true) } // see action.show - context, cli context, name, key, rescurse. diff --git a/internal/action/find_test.go b/internal/action/find_test.go index 224f24effa..a3f2137cfe 100644 --- a/internal/action/find_test.go +++ b/internal/action/find_test.go @@ -46,25 +46,25 @@ func TestFind(t *testing.T) { //nolint:paralleltest // find c := gptest.CliCtx(ctx, t) - if err := act.Find(c); err == nil || err.Error() != fmt.Sprintf("Usage: %s find ", actName) { + if err := act.Find(c); err == nil || err.Error() != fmt.Sprintf("Usage: %s find ", actName) { t.Errorf("Should fail: %s", err) } - // find fo + // find fo (with fuzzy search) c = gptest.CliCtxWithFlags(ctx, t, nil, "fo") - assert.NoError(t, act.Find(c)) + assert.NoError(t, act.FindFuzzy(c, "fo")) assert.Contains(t, strings.TrimSpace(buf.String()), "Found exact match in \"foo\"\nsecret") buf.Reset() // find fo (no fuzzy search) c = gptest.CliCtxWithFlags(ctx, t, nil, "fo") - assert.NoError(t, act.FindNoFuzzy(c)) + assert.NoError(t, act.Find(c)) assert.Equal(t, strings.TrimSpace(buf.String()), "foo") buf.Reset() // find yo - c = gptest.CliCtx(ctx, t, "yo") - assert.Error(t, act.Find(c)) + c = gptest.CliCtx(ctx, t) + assert.Error(t, act.FindFuzzy(c, "yo")) buf.Reset() // add some secrets @@ -76,8 +76,8 @@ func TestFind(t *testing.T) { //nolint:paralleltest buf.Reset() // find bar - c = gptest.CliCtx(ctx, t, "bar") - assert.NoError(t, act.Find(c)) + c = gptest.CliCtx(ctx, t) + assert.NoError(t, act.FindFuzzy(c, "bar")) assert.Equal(t, "bar/baz\nbar/zab", strings.TrimSpace(buf.String())) buf.Reset() diff --git a/internal/action/show.go b/internal/action/show.go index c4f32074b3..c024d2deb7 100644 --- a/internal/action/show.go +++ b/internal/action/show.go @@ -64,6 +64,9 @@ func showParseArgs(c *cli.Context) context.Context { // Show the content of a secret file. func (s *Action) Show(c *cli.Context) error { name := c.Args().First() + if name == "" { + return exit.Error(exit.Usage, nil, "Usage: %s show [name]", s.Name) + } ctx := showParseArgs(c) @@ -81,10 +84,6 @@ func (s *Action) Show(c *cli.Context) error { // show displays the given secret/key. func (s *Action) show(ctx context.Context, c *cli.Context, name string, recurse bool) error { - if name == "" { - return exit.Error(exit.Usage, nil, "Usage: %s show [name]", s.Name) - } - if s.Store.IsDir(ctx, name) && !s.Store.Exists(ctx, name) { return s.List(c) } @@ -272,7 +271,7 @@ func (s *Action) showHandleError(ctx context.Context, c *cli.Context, name strin out.Warningf(ctx, "Entry %q not found. Starting search...", name) c.Context = ctx - if err := s.Find(c); err != nil { + if err := s.FindFuzzy(c, name); err != nil { return exit.Error(exit.NotFound, err, "%s", err) }