Complex issue filter gui #847
Conversation
csordasmarton
force-pushed
the
issue_filters
branch
2 times, most recently
from
dbe6197
to
9a834cd
Compare
| @@ -50,7 +59,7 @@ def conv(text): | |||
| return text.replace('*', '%') | |||
|
|
|||
|
|
|||
| def process_report_filter_v2(report_filter): | |||
| def process_report_filter_v2(report_filter, count_filter=None): | |||
There was a problem hiding this comment.
I would add a comment here how the count_filter modifies the filter expression.
| @@ -41,6 +41,15 @@ | |||
| LOG = LoggerFactory.get_new_logger('ACCESS HANDLER') | |||
|
|
|||
|
|
|||
| class CountFilter: | |||
| FILE = 1 | |||
| @@ -299,7 +299,7 @@ def test_get_diff_checker_counts_core_unresolved(self): | |||
| cmp_data) | |||
| # Unesolved core checkers. | |||
| test_res = {'core.NullDereference': 4, 'core.DivideZero': 3} | |||
| self.assertDictEqual(diff_res, test_res) | |||
| self.assertDictContainsSubset(test_res, diff_res) | |||
There was a problem hiding this comment.
I would add some comments here, how the filtering behavior changed and why a subset check is enough or we should compare to the whole result returned in diff_res as before and change test_res.
This comment applies to the other tests too which were changed.
|
|
||
| // TODO |
There was a problem hiding this comment.
Awesome! 💛 👍
I have minor comments for now.
Is this (the input box's border being empty on the corners) intended?
- I think the placeholder text is too large compared to the input box itself. But this is just a personal preference.
- The enum-driven (review status, detection status) filters have an orange border for me. The "free text" filters only have this orange border when I click into the tooltip. This feels weird, and the border creates a clunky bounding box around the nice "message bubble" styled border.
- When a "free text" filter dropdown is filtered in the text box to a value that does not exist, a
No itemis shown, but it still has the "click" cursor. I think it would convey this meaning if we'd use thenot-allowedstandard cursor.
Are we sure we want the user to filter runs when the filter sidebar is opened in a run? Consider this: I clicked Xerces to open the bugs in Xerces, and now I'm seeing all bugs.):
Filter counts work strange when I select multiple different filters and such, without a Ctrl-F5, the other filters don't update their counts.
- ❓ In case of the enum-based filters, shouldn't we encode the string value (instead of the enum int) into the URL? It would make for more meaningful URLs to save.
Another minor thing I think which is easy to implement and makes this feature even nicer if one could entirely hide or make the filter sidebar very small. Currently I feel it occupies a lot of space on the page (this could be because I'm yet to get used to it) especially if I want to just scroll through my bugs without wanting to execute any filters whatsoever.
| @@ -299,7 +299,7 @@ def test_get_diff_checker_counts_core_unresolved(self): | |||
| cmp_data) | |||
| # Unesolved core checkers. | |||
| test_res = {'core.NullDereference': 4, 'core.DivideZero': 3} | |||
| self.assertDictEqual(diff_res, test_res) | |||
| self.assertDictContainsSubset(test_res, diff_res) | |||
There was a problem hiding this comment.
What is the actual change here, why is this test change needed? Now the diff will send back more data?
| /** | ||
| * Removes duplications from the given array. | ||
| */ | ||
| function arrayUnique (arr) { |
There was a problem hiding this comment.
Is this approach faster than using a Set? If so, could you please move this to codechecker/util? I'll have use for this function later on!
csordasmarton
force-pushed
the
issue_filters
branch
9 times, most recently
from
2d9a7d3
to
2c152ff
Compare
csordasmarton
force-pushed
the
issue_filters
branch
3 times, most recently
from
9e05e90
to
ab388a6
Compare
csordasmarton
force-pushed
the
issue_filters
branch
from
ab388a6
to
b0a6aba
Compare
No description provided.