New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix AutoClip handling on generate #2024
Conversation
Fixes gopasspw#2023 RELEASE_NOTES=[BUGFIX] Fix AutoClip handling on generate Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -120,7 +120,7 @@ func (s *Action) generateCopyOrPrint(ctx context.Context, c *cli.Context, name, | |||
// 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)) { | |||
if IsClip(ctx) || (s.cfg.AutoClip && ctxutil.IsTerminal(ctx)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I should be replacing the content of my new function out.IsOutputRedirected
by this then.
I'm not sure how ctxutil.IsTerminal(ctx)
behaves when we are piping stuff but in a terminal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now in #2025
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") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we printing this on StdOut
? If so we might want to consider doing that on StdErr
instead in a future PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think this goes to stdout right now.
Fixes gopasspw#2023 RELEASE_NOTES=[BUGFIX] Fix AutoClip handling on generate Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
Fixes #2023
RELEASE_NOTES=[BUGFIX] Fix AutoClip handling on generate
Signed-off-by: Dominik Schulz dominik.schulz@gauner.org