-
-
Notifications
You must be signed in to change notification settings - Fork 543
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
Adds password prompt for register and login #424
Adds password prompt for register and login #424
Conversation
There are also a few commits since the last PR I raised. I'm not sure why they appear but I think since you squash and merge that should appear on the final tree. Do let me know incase I need to do something else to fix this. |
Thanks! I'll have to test it to see how rpassword works, but it looks good. Can you keep keep the original argument mode as well however? At least username and email via flags seems reasonable to me |
Sure will look into that. I did not do that because I expected it to be confusing to have a partial prompt and options based input. But let me add that back and then decide. |
Please leave |
2586b29
to
e214cf8
Compare
So if I were to leave |
It would not be the same functionality. Our impl before didn't handle passwords in the prompt safely |
Okay, so while the password prompt would be handled safely, then option of adding the password with |
Yeah I think this is expected |
Yup, this is expected behaviour :)
…On Tue, May 24, 2022 at 13:44, Conrad Ludgate ***@***.***> wrote:
Yeah I think this is expected
—
Reply to this email directly, view it on GitHub
<#424 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMWYN3WPKRJPJRWJIFVOSYDVLTFLFANCNFSM5WWSWCKA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Thanks, I've updated the PR. Although I'm not happy with the code that I've written and feel that this could be refactored better. Do let me know your suggestions. |
I have some ideas for a refactor too. But that can be in a separate PR if you want. I'm testing out the password behaviour locally but the code looks good. Much appreciated |
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.
Works well. Ty
06ac958 Show current version on server index (#436) 706b1af Disable ARM docker builds (#438) f2abc23 Update README.md 3c2b055 Noyez fix dir hostname utf8 (#430) 3f5350d [feature] Add scroll wheel support to interactive history search (#435) dcdde22 Fix text outline for dark mode 9ac0c60 Implement cursor (#412) 119ab9e Adds password prompt for register and login (#424) e5df809 Noyez zsh histdb import (#393) b08e254 Improve default fish keybindings (#420) 4096c6e Update README.md cd2a3ab Add fish shell to key binding docs (#418) b278211 Bump clap_complete from 3.1.3 to 3.1.4 (#397) ee66c0a Bump axum from 0.5.5 to 0.5.6 (#415) 4297e26 Bump tokio from 1.18.1 to 1.18.2 (#396) dbd9ca5 Bump clap from 3.1.16 to 3.1.18 (#401) a7c9d19 Bump tower-http from 0.3.2 to 0.3.3 (#399) 3b79f68 Bump axum from 0.5.4 to 0.5.5 (#402) f340771 Cleanup dependencies – disable unnecessary or unused features (#407) ab294cd Don't pollute shell environment - remove 'id' variable (#408) 14b3060 Allow to build atuin server without client (#404) 5e4e8d1 Don't create config dir for server in default location if not needed (#406) b7946cc Update Chinese version README.md (#403) e0291f6 Update README.md 301190e Build ARM docker image in GitHub Actions using QEMU (#400) 1d030b9 Importer V3 (#395) d3a4ff9 Bump clap from 3.1.15 to 3.1.16 (#392) e9d2ec4 Add ctrl-k and ctrl-j for up and down (#394) 25afb5b Bump serde_json from 1.0.80 to 1.0.81 (#387) 4a839da Adds stats summary (#384) 7a394b0 Bump serde from 1.0.136 to 1.0.137 (#375) edd3f81 Bump clap_complete from 3.1.2 to 3.1.3 (#377) d85d03d Bump log from 0.4.16 to 0.4.17 (#382) dc3b7ef Bump tokio from 1.18.0 to 1.18.1 (#383) 12440c1 Bump serde_json from 1.0.79 to 1.0.80 (#376) 731042f Bump tower-http from 0.3.1 to 0.3.2 (#378) 82505e6 Bump clap from 3.1.12 to 3.1.15 (#381) e05c19d Add Chinese documentation translation & Fix spelling mistakes (#373) 6e280e2 Add Russian documentation translation (#365) 40efdd1 Bump http from 0.2.6 to 0.2.7 (#368) 8bc5bec Bump tower-http from 0.3.0 to 0.3.1 (#367) 172ac8d Create FUNDING.yml 7cdd00b Bump tokio from 1.17.0 to 1.18.0 (#357) 9d2e9ea Search: Allow specifiying the limited of returned entries (#364) 93ab4e7 ignore JetBrains IDEs, tidy-up imports (#348) 2cb4cb3 Bump axum from 0.5.3 to 0.5.4 (#355) 796644e Add created_at column to users (#354) f8233bc SQLx cannot run this migration OK (#353) d8ef5dd fix db range query (#351) 5926ea6 fix import auto for bash (#352) 43d299f bump tui (#346) 8ac6571 Remove all select * from the server queries (#347) 4030de4 Add btree index on history table (#345) b692e0c Bump tower-http from 0.2.5 to 0.3.0 (#343) 3680f4a Bump clap from 3.1.11 to 3.1.12 (#342) 7f5310a history list (#340)
06ac958 Show current version on server index (#436) 706b1af Disable ARM docker builds (#438) f2abc23 Update README.md 3c2b055 Noyez fix dir hostname utf8 (#430) 3f5350d [feature] Add scroll wheel support to interactive history search (#435) dcdde22 Fix text outline for dark mode 9ac0c60 Implement cursor (#412) 119ab9e Adds password prompt for register and login (#424) e5df809 Noyez zsh histdb import (#393) b08e254 Improve default fish keybindings (#420) 4096c6e Update README.md cd2a3ab Add fish shell to key binding docs (#418) b278211 Bump clap_complete from 3.1.3 to 3.1.4 (#397) ee66c0a Bump axum from 0.5.5 to 0.5.6 (#415) 4297e26 Bump tokio from 1.18.1 to 1.18.2 (#396) dbd9ca5 Bump clap from 3.1.16 to 3.1.18 (#401) a7c9d19 Bump tower-http from 0.3.2 to 0.3.3 (#399) 3b79f68 Bump axum from 0.5.4 to 0.5.5 (#402) f340771 Cleanup dependencies – disable unnecessary or unused features (#407) ab294cd Don't pollute shell environment - remove 'id' variable (#408) 14b3060 Allow to build atuin server without client (#404) 5e4e8d1 Don't create config dir for server in default location if not needed (#406) b7946cc Update Chinese version README.md (#403) e0291f6 Update README.md 301190e Build ARM docker image in GitHub Actions using QEMU (#400) 1d030b9 Importer V3 (#395) d3a4ff9 Bump clap from 3.1.15 to 3.1.16 (#392) e9d2ec4 Add ctrl-k and ctrl-j for up and down (#394) 25afb5b Bump serde_json from 1.0.80 to 1.0.81 (#387) 4a839da Adds stats summary (#384) 7a394b0 Bump serde from 1.0.136 to 1.0.137 (#375) edd3f81 Bump clap_complete from 3.1.2 to 3.1.3 (#377) d85d03d Bump log from 0.4.16 to 0.4.17 (#382) dc3b7ef Bump tokio from 1.18.0 to 1.18.1 (#383) 12440c1 Bump serde_json from 1.0.79 to 1.0.80 (#376) 731042f Bump tower-http from 0.3.1 to 0.3.2 (#378) 82505e6 Bump clap from 3.1.12 to 3.1.15 (#381) e05c19d Add Chinese documentation translation & Fix spelling mistakes (#373) 6e280e2 Add Russian documentation translation (#365) 40efdd1 Bump http from 0.2.6 to 0.2.7 (#368) 8bc5bec Bump tower-http from 0.3.0 to 0.3.1 (#367) 172ac8d Create FUNDING.yml 7cdd00b Bump tokio from 1.17.0 to 1.18.0 (#357) 9d2e9ea Search: Allow specifiying the limited of returned entries (#364) 93ab4e7 ignore JetBrains IDEs, tidy-up imports (#348) 2cb4cb3 Bump axum from 0.5.3 to 0.5.4 (#355) 796644e Add created_at column to users (#354) f8233bc SQLx cannot run this migration OK (#353) d8ef5dd fix db range query (#351) 5926ea6 fix import auto for bash (#352) 43d299f bump tui (#346) 8ac6571 Remove all select * from the server queries (#347) 4030de4 Add btree index on history table (#345) b692e0c Bump tower-http from 0.2.5 to 0.3.0 (#343) 3680f4a Bump clap from 3.1.11 to 3.1.12 (#342) 7f5310a history list (#340)
Addresses #344.
I've added a dependency of rpassword since I figured the library would be able to handle the password input better across different platforms compared to something that I would write. I'm also not sure if the Cmd struct for login and register should remain since there are no longer any "options" to use with them.
Do let me know if these changes make sense. Thanks!