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

Make use of new bemenu password indicator option #21

Merged
merged 2 commits into from
Feb 25, 2024

Conversation

nmeum
Copy link
Contributor

@nmeum nmeum commented Feb 25, 2024

This option was added in bemenu 0.6.17, when enable it displays asterisks instead of hiding the input text entirely. This is also what pinentry-gtk does and I think it's much more user friendly as it clearly indicates that input has been received by bemenu-pinentry.

This option was added in bemenu 0.6.17, when enable it displays
asterisks instead of hiding the input text entirely. This is also
what pinentry-gtk does and I think it's much more user friendly.
@t-8ch
Copy link
Owner

t-8ch commented Feb 25, 2024

Thanks!

I'd like to have a cli option for this that mimics the one from bemenu, defaulting to the current behavior.

@nmeum
Copy link
Contributor Author

nmeum commented Feb 25, 2024

Hi, thanks for the quick reply. Updated accordingly, does warrant some further testing.

I also created Cloudef/bemenu#388 in the hopes that we can reduce code duplication with upstream regarding option handling.

else if (!strcmp(password, "none"))
bm_menu_set_password(menu, BM_PASSWORD_NONE);
else if (!strcmp(password, "indicator"))
bm_menu_set_password(menu, BM_PASSWORD_INDICATOR);
Copy link
Owner

Choose a reason for hiding this comment

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

Please also a fallback to BM_PASSWORD_HIDE,
Other than that it looks great.

Copy link
Contributor Author

@nmeum nmeum Feb 25, 2024

Choose a reason for hiding this comment

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

Isn't -x hide handled in Line 170?

if (!password || !strcmp(password, "hide"))

or what do you mean by "fallback"?

Copy link
Owner

Choose a reason for hiding this comment

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

-x foo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, right. Good catch! Should be fixed now.

@t-8ch t-8ch merged commit e85e5f4 into t-8ch:main Feb 25, 2024
@ammgws
Copy link

ammgws commented Apr 27, 2024

@t-8ch Would it be possible to cut a new release that includes this feature?

@t-8ch
Copy link
Owner

t-8ch commented Apr 27, 2024

Would it be possible to cut a new release that includes this feature?

Thanks for the reminder. Done in v0.13.2

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.

3 participants