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

feat: Implement builtin mode for keepassxc #4149

Merged
merged 1 commit into from
Dec 29, 2024
Merged

Conversation

bakito
Copy link
Contributor

@bakito bakito commented Dec 20, 2024

Implements: #4148

Copy link
Owner

@twpayne twpayne left a comment

Choose a reason for hiding this comment

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

Thanks for this!

Instead of a new set of template functions, I think this different "mode" for opening KeepassXC databases. Currently chemzoi supports "cache-password" and "open" modes. I think this should be a new "builtin" mode.

What do you think?

.gitignore Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@bakito
Copy link
Contributor Author

bakito commented Dec 20, 2024

Thanks for this!

Instead of a new set of template functions, I think this different "mode" for opening KeepassXC databases. Currently chemzoi supports "cache-password" and "open" modes. I think this should be a new "builtin" mode.

What do you think?

Sounds reasonable.
I modified the implementation and added the functions for the new mode.

@bakito bakito force-pushed the keepass branch 3 times, most recently from 5396dc8 to f42961b Compare December 20, 2024 22:20
Copy link
Owner

@twpayne twpayne left a comment

Choose a reason for hiding this comment

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

Thank you, this is looking excellent, and I will do a full review after the weekend.

Please also check the contributing guide for low-hanging fruit.

But, thank you, this PR is excellent.

@bakito bakito changed the title [WIP] implement keepasslib as alternative to keepassxc feat: Implement builtin mode for keepassxc Dec 21, 2024
@bakito bakito marked this pull request as ready for review December 21, 2024 06:27
@bakito bakito force-pushed the keepass branch 3 times, most recently from 5e0b26c to a5f1457 Compare December 23, 2024 20:45
@twpayne
Copy link
Owner

twpayne commented Dec 24, 2024

Thank you, this is looking excellent. Once you've made the small changes to the documentation, this should be ready to merge.

Copy link
Collaborator

@halostatue halostatue 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 overall. Just a couple of comments and only one "required" change (for the docs).

internal/cmd/keepassxctemplatefuncs.go Outdated Show resolved Hide resolved
internal/cmd/keepassxctemplatefuncs.go Outdated Show resolved Hide resolved
@twpayne
Copy link
Owner

twpayne commented Dec 28, 2024

@bakito Thanks for making the changes. Please can you squash everything into a single commit, and then this should be good to go.

@twpayne twpayne merged commit 8f6fea6 into twpayne:master Dec 29, 2024
@twpayne
Copy link
Owner

twpayne commented Dec 29, 2024

Thank you @bakito for your excellent work and patience here!

@bakito
Copy link
Contributor Author

bakito commented Dec 29, 2024

@twpayne I introduced a lint issue in the last change. Sorry for that.
#4166 provides a fix

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