-
-
Notifications
You must be signed in to change notification settings - Fork 501
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
Minor generate and show fixes #2145
Conversation
Fixes gopasspw#2140 RELEASE_NOTES=n/a Signed-off-by: Dominik Schulz <[email protected]>
8f74944
to
6690405
Compare
@@ -111,7 +111,7 @@ func keyAndLength(args argList) (string, string) { | |||
func (s *Action) generateCopyOrPrint(ctx context.Context, c *cli.Context, name, key, password string) error { | |||
entry := name | |||
if key != "" { | |||
entry += ":" + key | |||
entry += " " + key |
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.
This should have broken some test 😢
Why not keep the entry:key format?
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.
Because it does not work. IIRC we did never support that format.
@@ -220,7 +220,7 @@ func clamp(min, max, value int) int { | |||
} | |||
|
|||
func (s *Action) generatePasswordForRule(ctx context.Context, c *cli.Context, length, name, domain string, rule pwrules.Rule) (string, error) { | |||
out.Printf(ctx, "Using password rules for %s ...", domain) | |||
out.Noticef(ctx, "Using password rules for %s ...", domain) |
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 rephrase that to make it more obvious it's due to the domain name?
Something like :
"The domain %s appears to have specific password rules, using them."
@@ -231,6 +231,7 @@ func (s *Action) generatePasswordForRule(ctx context.Context, c *cli.Context, le | |||
if err != nil { |
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.
Shouldn't we reject invalid lengths instead of just clamping them below?
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. But either we make the implementation much more convoluted (loops etc.) or we abort the execution and possibly disrupt the user. Not sure either. But not clamping or checking them also sounds wrong.
Fixes gopasspw#2140 RELEASE_NOTES=n/a Signed-off-by: Dominik Schulz <[email protected]>
Fixes #2140
Addresses a few minor issues found when trying to reproduce #2140.
RELEASE_NOTES=n/a