-
-
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
Fixes #667 and correct a few details #705
Conversation
This primarily adds the config option requested in #667. I've added back the notion or erroring in case of a show action on an entry with only a password and the safe content option enabled Also I've fixed the way the String() function of the StoreConfig forgot to display the Notification value in its output.
It seems that the Generate action does not use the correct context and has not the correct settings. I suspect this might be related to the newly added #701. I've tried using Store.GetContext, to no avail... |
Codecov Report
@@ Coverage Diff @@
## master #705 +/- ##
==========================================
- Coverage 65.66% 65.59% -0.08%
==========================================
Files 150 150
Lines 8556 8578 +22
==========================================
+ Hits 5618 5626 +8
- Misses 2272 2286 +14
Partials 666 666
Continue to review full report at Codecov.
|
Alright, I've found it: when Setting the password in the store, the correct Store context wasn't retrieved since there was no "SetContext" function doing the same as the "GetContext" does. |
Yes, I was just about the state this but you beat me by a few minutes ;) The PR looks good. Thanks for fixing some existing flaws 👍 |
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. Thanks a lot!
I'm afraid I forgot some tests as Codecov said. |
Alright, I'll just pull them from there. Thanks a lot! |
* Fixes gopasspw#667 and correct a few details This primarily adds the config option requested in gopasspw#667. I've added back the notion or erroring in case of a show action on an entry with only a password and the safe content option enabled Also I've fixed the way the String() function of the StoreConfig forgot to display the Notification value in its output. * Adding tests for autoclip * adding a clip flag back * correcting an old copy-paste mistake * trying to correct the problem * Correcting the error caused by gopasspw#701
This primarily adds the config option requested in #667.
I've added back the notion or erroring in case of a show action on an
entry with only a password and the safe content option enabled.
Also I've fixed the way the String() function of the StoreConfig forgot
to display the Notification value in its output.
I've also handled the merge conflict caused by #703.
Now, this does NOT work for the generate action, for a reason I do not yet fully understand. I hope you'll be able to help me there.
In the show action, the context seems to correctly use the config values set.
However, when running the Generate action,
ctxutil.HasAutoClip(ctx)
returnsfalse
, even ifgopass config autoclip
returnstrue
.As per the default value of AutoClip,
IsAutoClip
will returntrue
isHasAutoClip
isfalse
...This makes no sense to me, it's just like if the Generate action had not loaded the context correctly.