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

entry: move entries within a group #347

Merged
merged 4 commits into from
Nov 14, 2023

Conversation

pwall2222
Copy link
Contributor

Code changes:

  • Add index property to Entry
  • Add move method to Entry
  • Add _first_entry property to Group

Explanation:
To move Entries around we need to remove and then insert at the desired index, the KDBX format makes it so that Entries and group info is all mixed, so we have to get the first element index to position correctly the Entries.

If you know a better method to get the index of the first entry I'll put it in.

What it does:

It moves entries around a group, pretty self explanatory, like the Keepass and KeepassXC functionality.

@pwall2222
Copy link
Contributor Author

Is just for feature parity with the KeePassXC client.

@Evidlo
Copy link
Member

Evidlo commented Nov 10, 2023

Perhaps it would be better to call this .reindex() or something similar. I'd like to avoid confusion with the move_entry() function, which is for hierarchical operations. Also needs a test in tests.py

Is just for feature parity with the KeePassXC client.

How do you do this in KeePassXC? As far as I can tell, the entries are just automatically sorted by Title/Username/URL/etc.

@pwall2222
Copy link
Contributor Author

pwall2222 commented Nov 10, 2023

As you sugggested I added a test and I rename the function to reindex.
The keepassxc custom reorder, but anyway it was a feature in the original keepass.
keepassxreboot/keepassxc#2295
keepassxreboot/keepassxc#4357
To replicate you will first need to click the current sorting column till it shows no icon, then in the context menu you will see move up and move down.

@Evidlo
Copy link
Member

Evidlo commented Nov 10, 2023

Thanks! This looks good so far.

Would you mind adding docstrings on reindex and index and also in README.md beneath move_entry? Sorry, I should've mentioned that before. After that it looks good to merge.

@pwall2222
Copy link
Contributor Author

Shouldn't it be near Entry.delete_attachment since its in the Entry class it self?

@Evidlo
Copy link
Member

Evidlo commented Nov 14, 2023

Shouldn't it be near Entry.delete_attachment since its in the Entry class it self?

Actually there doesn't appear to be sections for Group/Entry member functions in the README, so you can ignore that for now.

Thanks for the contribution!

@Evidlo Evidlo merged commit 1085c59 into libkeepass:master Nov 14, 2023
5 of 6 checks passed
@pwall2222 pwall2222 deleted the move-inside-group branch November 15, 2023 13:36
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.

2 participants