-
Notifications
You must be signed in to change notification settings - Fork 71
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
allow request for new key to confirm entered pass #702
allow request for new key to confirm entered pass #702
Conversation
Maybe move these closures to separate function? |
What is the best file to add the function so that both key.rs and init.rs can access |
|
First, this PR IMO doesn't allow to confirm the password, but enforces it. About the place: Please not in |
@istudyatuni @aawsome what do you think about having a function called prompt_password_twice in rustic/src/helpers that both init.rs and key.rs can call |
I'm afraid to make those decisions in this project 😁 |
As discussed @Aderemi-Adesada on discord, I also think that the best idea is to replace |
rpassword has been replaced with dialoguer in init.rs and key.rs when prompting for new 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.
Beside my remarks, I was wondering if we can remove the dependency on rpassword
completely. Then I remembered some newline handling which needed to be dealt with.
Checking the code I realized that that fix already removed rpassword
when reading from a file - but obviously I have overseen this in src/command/key.rs
. Can you please have a look at read_password_from_reader
in crates/rustic_core/src/repository.rs
and use this in src/command/key.rs
- this should allow to remove rpassword
completely.
Moreover, can you please add an entry in changelog/new.txt
and after your changes squash all commits into 1 commit? Thanks a lot!
b1a006e
to
6f2dfb4
Compare
rpassword have been removed completely and |
@Aderemi-Adesada there are syntax errors in |
I will fix that right now |
3bbd0e5
to
6f2dfb4
Compare
@aawsome syntax error have been fixed |
Can you please rebase to main and squash all commits instead of re-merging master back into your branch? That will keep commit history clean. Thanks a lot! |
c89f198
to
cdbe671
Compare
cdbe671
to
088eabd
Compare
Oh, clicked wrong - actually wanted to run the tests. |
You are welcome |
allow request for new key to prompt user to confirm the password they entered