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.

Sign up for GitHub

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

Jump to bottom

Several improvements to tags editing #7708

Merged
merged 1 commit into from
Mar 29, 2022
Merged

Several improvements to tags editing #7708

merged 1 commit into from
Mar 29, 2022

Conversation

droidmonkey
Copy link
Member

Testing strategy

Tested on windows.

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)

@droidmonkey droidmonkey added bug feature: Tags pr: backport pending Pull request yet to be backported to a previous release labels Mar 29, 2022
@droidmonkey droidmonkey added this to the v2.7.1 milestone Mar 29, 2022
@droidmonkey droidmonkey requested review from hifi and phoerious March 29, 2022 02:49
@droidmonkey
Several improvements to tags editing
9af0ab3 
* Fix #7602 - Allow spaces in tag names
* Fix #7528 - Allow pasting text into the tags field. Text is split by comma creating tags for each section of text. If there are no commas then the pasted text becomes a tag.
* Fix tags editing not causing the entry to be marked as modified.
@droidmonkey
Copy link
Member Author

Fixed the gui test

michaelk83
michaelk83 reviewed Mar 29, 2022
View reviewed changes
src/gui/tag/TagsEdit.cpp
} else if (event == QKeySequence::Paste) {
auto clipboard = QApplication::clipboard();
if (clipboard) {
for (auto tagtext : clipboard->text().split(",")) {
Copy link

@michaelk83 michaelk83 Mar 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Shouldn't this use the same regex as when editing manually, the one on Entry.cpp line 195?
Also, what happens when you paste a tag that was already added (or enter a duplicate tag manually, for that matter)?

Copy link
Member Author

@droidmonkey droidmonkey Mar 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Tags don't duplicate so non issue. I am just trying to get this out the door right now. This needs far more work to synch the various pieces. Not doing that right now. (aka this is good enough for now)

@codecov-commenter
Copy link

codecov-commenter commented Mar 29, 2022 •
edited by codecov bot
Loading

Codecov Report

Attention: Patch coverage is 50.00000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 64.24%. Comparing base (ad61d71) to head (9af0ab3).
Report is 466 commits behind head on develop.

Files with missing lines Patch % Lines
src/gui/tag/TagsEdit.cpp 41.67% 7 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #7708      +/-   ##
===========================================
- Coverage    64.27%   64.24%   -0.03%     
===========================================
  Files          340      340              
  Lines        43470    43482      +12     
===========================================
- Hits         27936    27932       -4     
- Misses       15534    15550      +16

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

@droidmonkey droidmonkey merged commit 02602da into develop Mar 29, 2022
@droidmonkey droidmonkey deleted the hotfix/tag-improve branch March 29, 2022 22:41
@droidmonkey droidmonkey added pr: backported Pull request backported to previous release and removed pr: backport pending Pull request yet to be backported to a previous release labels Apr 6, 2022
@phoerious phoerious added pr: bugfix Pull request that fixes a bug and removed bug labels Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelk83 michaelk83 michaelk83 left review comments

@phoerious phoerious phoerious approved these changes

@hifi hifi Awaiting requested review from hifi

Assignees
No one assigned
Labels
feature: Tags pr: backported Pull request backported to previous release pr: bugfix Pull request that fixes a bug
Projects
None yet
Milestone
v2.7.1
Development

Successfully merging this pull request may close these issues.

Words with spaces used for tags get split into multiple tags Pasting a new tag into the tag field don't work
4 participants
@droidmonkey @codecov-commenter @phoerious @michaelk83