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

Refactor LogFilteredData #67

Closed

Conversation

gin-ahirsch
Copy link

These are the changes of my eager-filteredItemsCache-branch which do not actually change the behavior of the cache. Thus the changes should be uncontroversial.
I'm unsure if FilteredLineType should be kept as enum class as the operator overloading, as well as no implicit cast to bool, add some noise. Maybe it should be changed back to a weaklier-typed enum.

@variar
Copy link
Owner

variar commented Mar 18, 2019

Sorry, I've been to busy planing vacation. Will try to look through the code this week.

@variar variar added this to the 2019.03 milestone Mar 18, 2019
That method was used solely to delete the last matched line so that it
is not matched a second time if we update the search.
This change instead modifies the UpdateSearchOperation::start() so that
the line never gets matched twice in the first place.
Since we add a chunk of results, which is sorted in itself, we don't
need to perform a full merge/sort algorithm. We just need to find the
starting-place for the chunk and can insert it there without additional
sorting.
@gin-ahirsch
Copy link
Author

I added two changes from my upcoming PR implementing #37. These stand on their own though, so I added them to this branch.

@variar
Copy link
Owner

variar commented Jun 26, 2019

I've merged core of PR in 58ed0e4 using QFlags to avoid manually write flag operations.

I'd prefer include structure as it is now, this is like we do it in internal projects.

@variar variar closed this Jun 26, 2019
@gin-ahirsch gin-ahirsch deleted the refactor-logfiltereddata branch October 15, 2019 13:23
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