-
-
Notifications
You must be signed in to change notification settings - Fork 394
feat: remove and create tags from tag database panel #569
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
Merged
Merged
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
47ba3c0
[feat] can now add a new tag from the tag library panel
DandyDev01 8763216
[feat] can remove tags from the tag library panel
DandyDev01 476cc15
[test] added test for library->remove_tag
DandyDev01 7add23e
[fix] type error
DandyDev01 746a234
removed redundant lambda
DandyDev01 b8fbf9d
fix: tags with a reserved id could be edited or removed, now they can…
DandyDev01 9f9b9a7
fix: when a tag is removed or edited the preivew panel will update to…
DandyDev01 5400227
Merge remote-tracking branch 'upstream/main' into remove_add_tag
DandyDev01 3bbcb11
fix: mypy check
DandyDev01 a4652a3
merge upstream/main
DandyDev01 90a9a6d
fix: aliases and subtags not being removed from DB when tag they refe…
DandyDev01 af06e0e
feat: added a confirmation message box when removing tags.
DandyDev01 405ecda
fix: mypy
DandyDev01 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,3 +13,4 @@ | |
|
||
TAG_FAVORITE = 1 | ||
TAG_ARCHIVED = 0 | ||
RESERVED_TAG_IDS = range(0, 999) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use
functools.partial
instead oflambda
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe, the codebase uses lambda throughout when using slots. Is there a real world benefit to using a partial here (or throughout)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this case, partial feels comfortable for me because that makes the intent more explicit and ties directly to the idea of binding
tag
toself.remove_tag
. however i would say it's a matter of style so i'm okay if @DandyDev01 decides to go with lambda here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, I will just stick with the lambda, because it seems that using a partial will not guarantee a memory leak won't occur and @seakrueger didn't run into any leaks while testing #131 (comment). Also, I haven't seen partials used too much elsewhere in the code base, so for consistency and readability (most people already know lambda and might have to lookup partials) lambda make more sense for now.