Tags containing blank spaces were not handled properly in the search widget.
Now we enclose them in quotes and strip the quotes before setting them to the widget.
Details
Details
- Reviewers
elvisangelaccio ngraham meven - Group Reviewers
Dolphin - Commits
- R318:5778099ece7a: (search) Fix searching tags with spaces
No artifacts when searching tags containing spaces
Added test cases to bin/dolphinquerytest
Diff Detail
Diff Detail
- Repository
- R318 Dolphin
- Branch
- search_tags_space
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 20542 Build 20560: arc lint + arc unit
This comment was removed by iasensio.
Comment Actions
Is it possible to add a test case in dolphinquerytest.cpp?
| src/search/dolphinfacetswidget.cpp | ||
|---|---|---|
| 184–185 | IMHO the new helper function should only strip the quotes. I'd keep the QString::mid(10) logic here, because it's clear that we are removing the modified>= prefix. | |
| src/search/dolphinfacetswidget.cpp | ||
|---|---|---|
| 184–185 | Done. It's more clear indeed. | |
| src/search/dolphinfacetswidget.cpp | ||
|---|---|---|
| 38 | Maybe check that the first and last characters are quotes first before removing " | |
Comment Actions
Sorry, I totally oversaw your comment. That test unit only checks on DolphinQuery, not on the UI.
But in fact this function is copied verbatim from there, so it makes sense to use one and only function, and to be able to test it.
With your suggestion and @meven's one, I think I'll strip the quotes out directly in DoplhinQuery, so after parsing and splitting the terms, the tag:"some weird quoted tag" just becomes tag:some weird quoted tag and no further quote stripping is needed here. I'll add a test case too.
Comment Actions
Better approach, by stripping out DolphinQuery
- Widget: Add quotes to selected tags with spaces
- Query: Strip quotes around value for every search term
- Do not strip out unpaired quotes
- Add test cases
Comment Actions
Since I'm also using master I had to check to be sure. Tag filter was added later and it's not on 19.12.