Skip to content
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

Better progress bar handling for OTP #2019

Merged
merged 2 commits into from Nov 3, 2021

Conversation

AnomalRoil
Copy link
Member

RELEASE_NOTES=[BUGFIX] Do not print OTP progress bar if not in terminal
RELEASE_NOTES=[UX] Use new progress bar for OTP expiry time

This is making the expiry time for OTP visually go from 0 to expiry time in terminal, but prevents printing the progress bar when not in a terminal.

image

I also replaced the special handling of the remaining time until expiry to use Truncate instead.

Sadly, I'm yet again adding some "special handling" to detect whether we are in a terminal or not. We might want to heavily refactor all such things.

This fixes #2017.

RELEASE_NOTES=[BUGFIX] Do not print OTP progress bar if not in terminal

RELEASE_NOTES=[UX] Use new progress bar for OTP expiry time

Signed-off-by: Yolan Romailler <anomalroil@users.noreply.github.com>
dominikschulz
dominikschulz previously approved these changes Oct 30, 2021
Copy link
Member

@dominikschulz dominikschulz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks. But I'd suggest some small changes.

internal/action/otp.go Outdated Show resolved Hide resolved
internal/action/otp.go Outdated Show resolved Hide resolved
internal/action/otp.go Outdated Show resolved Hide resolved
RELEASE_NOTES=n/a

Signed-off-by: Yolan Romailler <anomalroil@users.noreply.github.com>
Copy link
Member

@dominikschulz dominikschulz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dominikschulz dominikschulz merged commit 757bcad into gopasspw:master Nov 3, 2021
@AnomalRoil AnomalRoil deleted the fix-2017 branch November 3, 2021 16:12
@posquit0
Copy link

posquit0 commented Nov 8, 2021

Is there any release schedule? I wanna use this feature! 😆

@dominikschulz
Copy link
Member

Cutting new releases has become very easy. We can do one soon. Probably this week.

@mirkonasato
Copy link

This may be a silly question, but how do I stop the OTP progress bar introduced in 1.13.0 after I used the OTP code? I don't want to wait N seconds before I can run the next gopass command in the interactive shell, and Ctrl+C quits the whole shell.

@dominikschulz
Copy link
Member

@AnomalRoil ?

Personally I don't put OTP into my stores so I don't use this.

@AnomalRoil
Copy link
Member Author

Mhhh I personally never use the interactive shell, so I didn't realize we needed a way to do that.
I could had a key binding to q maybe to quit it?

@mirkonasato
Copy link

q to quit would be great!

@dominikschulz
Copy link
Member

@mirkonasato I'm sorry for that inconvenience. I have just reproduced it and we definitely need a way to abort the progress bar w/o killing gopass.

@AnomalRoil
Copy link
Member Author

I've added a fix in #2041. 👍🏻

kpitt pushed a commit to kpitt/gopass that referenced this pull request Jul 21, 2022
* Better progress bar handling for OTP

RELEASE_NOTES=[BUGFIX] Do not print OTP progress bar if not in terminal

RELEASE_NOTES=[UX] Use new progress bar for OTP expiry time

Signed-off-by: Yolan Romailler <anomalroil@users.noreply.github.com>

* Applying code review suggestions

RELEASE_NOTES=n/a

Signed-off-by: Yolan Romailler <anomalroil@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add config option to output otp-code only
4 participants