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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bitwarden enhancements #189

Merged
merged 11 commits into from
Nov 18, 2023
Merged

Bitwarden enhancements #189

merged 11 commits into from
Nov 18, 2023

Conversation

DavidDeadly
Copy link
Contributor

Primarily, I just wanted to add the 'copy_username' action because it's something I've been needing regularly these days. In the proccess of understanding I found a few things that could be improved, and I just did it. 馃 The commits have explanations of my decisions.

Initially, I considered keeping this changes local, but I then realized was the perfect time to help and do my first ever open source contribution. Hope to be helpful!

I'm totally open to debate and feedback, thanks!

This helps readability and performance.
Before password and code subprocesses were running for each filtered_password.
Now they only run when the user requests it.
Avoid abbreviations & acronyms, in favor of readability
'Unlock' item is unncesary because rbw binary by default asks
for the master password on any action, like listing (__get_items).
The only reason maybe to use it before might probably be to sync
with the cloud items, and that can be done via 'rbw sync'.
Copy link
Contributor

@tomsquest tomsquest left a comment

Choose a reason for hiding this comment

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

馃憤 for the contribution!

Minor styling issue: the "double underscore" could be replace with single underscore, which is traditionally used for private members.

[pep8] _single_leading_underscore: weak "internal use" indicator. E.g. from M import * does not import objects whose name starts with an underscore.

@DavidDeadly
Copy link
Contributor Author

Sure!! I was kind of confused about that, but now I know. Thanks!!

@ManuelSchneid3r
Copy link
Member

@ovitor @tylio what do you think?

@@ -20,86 +20,35 @@ class Plugin(PluginInstance, TriggerQueryHandler):

def __init__(self):
TriggerQueryHandler.__init__(self,
id=md_id,
id=md_iid,
Copy link
Member

Choose a reason for hiding this comment

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

dont use md_iid here. md_id is the id of the plugin, md_iid is the interface id, currently v2.1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I thought it was a misspelling because I didn't find where 'md_id' was declared, the linter marked as not defined

@ManuelSchneid3r
Copy link
Member

please also increase the version.

@ManuelSchneid3r
Copy link
Member

Actually I meant the plugin version not the interface id. To me it looks like the plugin has some new features. If the UX changes make it 2.0 if it's just some non user recogniceable changes make it 1.4.

@DavidDeadly
Copy link
Contributor Author

DavidDeadly commented Nov 8, 2023

Actually I meant the plugin version not the interface id. To me it looks like the plugin has some new features. If the UX changes make it 2.0 if it's just some non user recogniceable changes make it 1.4.

Right, my bad...

Well, I added an action (copy username), that's the only recognisable change in what respects to the user, not sure if that's enough to make a major, please confirm it.

And should I leave the interface id in v2.1?? It's the currently active, isn't it?

@ManuelSchneid3r
Copy link
Member

ManuelSchneid3r commented Nov 8, 2023

Go with md_version 2.0. I dont remember the additions to md_iid 2.1 exacty and forgot to document them. Yeah better leave 2.1 in there

@ManuelSchneid3r ManuelSchneid3r merged commit 4001c43 into albertlauncher:master Nov 18, 2023
@DavidDeadly DavidDeadly deleted the bitwarden_enhancements branch November 19, 2023 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants