Enhancement/change file filtering logic #5146
Conversation
|
Thank you for opening your first PR! 🎉 We are very happy and would like to thank you very much for your contribution. If everything checks out, we'll make sure to review the PR as soon as possible and give feedback. In the meantime, to make the reviewing process as fast as possible, you can help us by checking the following things:
Furthermore, make sure that the linter does not complain, which will check your code on every new commit. If the linter task fails, make sure to run |
|
Thank you. We'll have to solve the merge conflicts with the Changelog, but in general I appreciate that you took the time to write a few lines indicating this change for users. @kyaso Do you have time to review this PR before your vacation? It doesn't have urgent priority, but I'm booked until the end of June, and so I want to save my resources for potential urgent fixes. |
CHANGELOG.md
Outdated
|
|
||
| Previously when searching for files and workspaces in the filter field in | ||
| Zettlr, OR logic would be used, meaning that if a item matched any search query, | ||
| that item would be desplayed as a result of the search. This is not a very |
There was a problem hiding this comment.
| that item would be desplayed as a result of the search. This is not a very | |
| that item would be displayed as a result of the search. This is not a very |
There was a problem hiding this comment.
Thanks @kyaso for the code review, I just have a few "formalities" regarding the changelog :)
| return function (item: AnyDescriptor): boolean { | ||
| // Initialize a variable to keep track of whether all queries are matched |
There was a problem hiding this comment.
| // Initialize a variable to keep track of whether all queries are matched |
| for (const q of queries) { | ||
| let queryMatched = false // Track if the current query is matched |
There was a problem hiding this comment.
| let queryMatched = false // Track if the current query is matched | |
| let queryMatched = false |
| // The rest can only match files | ||
| if (item.type === 'file') { | ||
| // Type assertion to check if 'firstHeading' exists on file descriptors | ||
| const fileDescriptor = item as { type: 'file', tags: string[], frontmatter?: Record<string, any>, firstHeading?: string | null } |
There was a problem hiding this comment.
Can you directly coerce to MDFileDescriptor?
CHANGELOG.md
Outdated
|
|
||
| ## GUI and Functionality | ||
|
|
||
| (Nothing here) |
There was a problem hiding this comment.
Please indicate this using Change: short description
(See previous changelog for how)
|
Thanks for the help @kyaso @nathanlesage. Have implemented these changes :) |
|
Looks good, thank you! Unfortunately I'm on mobile right now and the app indicates that there are merge conflicts. I have little time to manually solve them over the weekend, but should you have time, I can merge in a calm minute so that we can mark this PR as done. |
All done, should be good to merge |
|
Perfect! Thank you for reacting so quickly! |
|
There we go, your PR got merged! Welcome to the party! 🔥 |
Description
#5089 Changes the file filtering logic for match-query to AND instead of OR. This means if multiple queries are searched for within the filter field, only entries that match all queries will be displayed.
Changes
Additional information
yarn test passes and yarn lint reports no errors
Tested on: MacOs 14.3.1