Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesFootnotes
|
| if conflictFileCount > 0 && prevConflictFileCount == 0 { | ||
| if fileTreeViewModel.GetFilter() == filetree.DisplayAll { | ||
| if fileTreeViewModel.GetStatusFilter() == filetree.DisplayAll { |
There was a problem hiding this comment.
Renamed to GetStatusFilter to avoid the naming collision with GetFilter which comes from the IFilterableContext interface.
There was a problem hiding this comment.
It would be nice to extract this to a separate preparation commit, with a commit message explaining the reason, so that you don't have to explain it here. 😄
There was a problem hiding this comment.
Pull request overview
This PR updates Lazygit’s file-based panels (working tree files and commit files) to use the existing filter mechanism (i.e., actually reducing the list contents) instead of the search mechanism (highlighting matches without changing the list).
Changes:
- Add text-filter support to working-tree and commit-file trees, including fuzzy/non-fuzzy matching.
- Convert Working Tree Files and Commit Files contexts from “searchable” to “filterable”.
- Enable and adjust integration tests that validate file/commit-file filtering behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/integration/tests/filter_and_search/nested_filter_transient.go | Updates expectations to reflect filtering behavior in transient panels. |
| pkg/integration/tests/filter_and_search/nested_filter.go | Updates expectations/comments for filtering behavior across nested panels. |
| pkg/integration/tests/filter_and_search/filter_files.go | Unskips and runs file filtering integration test now that filtering is implemented. |
| pkg/integration/tests/filter_and_search/filter_commit_files.go | Unskips and runs commit-file filtering integration test now that filtering is implemented. |
| pkg/gui/filetree/file_tree_view_model.go | Adds IFilterableContext methods + search history to working tree file view model. |
| pkg/gui/filetree/file_tree.go | Implements text filtering for working-tree files and renames status filter getter. |
| pkg/gui/filetree/file_filter.go | Adds shared text-filter helpers for files and commit files (substring or fuzzy). |
| pkg/gui/filetree/commit_file_tree_view_model.go | Adds IFilterableContext methods + search history to commit-file view model. |
| pkg/gui/filetree/commit_file_tree.go | Implements text filtering in commit-file tree building. |
| pkg/gui/controllers/switch_to_diff_files_controller.go | Clears commit-files filtering when switching into diff-files view. |
| pkg/gui/controllers/helpers/search_helper.go | Resets filter history index on open; adjusts when selection resets on re-apply. |
| pkg/gui/controllers/helpers/refresh_helper.go | Updates call sites for renamed status filter getter. |
| pkg/gui/controllers/files_controller.go | Updates status filter references to new getter name. |
| pkg/gui/context/working_tree_context.go | Converts working tree context from searchable to filterable. |
| pkg/gui/context/commit_files_context.go | Converts commit files context from searchable to filterable. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
12e06ec to
49977e6
Compare
|
Nice! From what I remember, the main reason why we excluded the Files view from filtering back then was that we couldn't decide whether it should filter just the file name, or the full path. I can't find any discussion about that any more though. In this PR it seems you have decided to filter by full path, and it may well be the more desirable behavior (I'm still not really sure though). I would have expected some mention of this in the PR description or commit message. One inconsistency that I found: if you filter in the Commit Files view so that only some files in a directory are visible, and then select the directory and press space, only the visible files are added to the custom patch. However, if you do the same in the Files panel, all files are staged, even the invisible ones. I'm not really sure which behavior is the more expected one, but I feel it would be good to be consistent. And finally, after using it for a day I found a very confusing aspect of this, related to the previous parapraph: if you filter a directory so that only one file is visible, and then select the directory, it shows the diff of all files in it. This is of course the expected behavior if you think about it, but if you don't, it can confuse the hell out of you. (It did for me, anyway, and I suspect it could confuse other users.) I don't have any idea what we could do about it (probably nothing), just wanted to bring it up as something that confused me. |
This avoids a naming collision with GetFilter from the IFilterableContext interface, which will be implemented by FileTreeViewModel in the next commit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Change working tree files and commit files panels to use filtering (reducing the list) instead of search (highlighting matches). This matches the behavior of other filterable views. The text filter matches against the full file path, not just the filename, which is more useful for navigating large directory trees. When toggling a directory for a custom patch while a text filter is active, all files in the directory are included (not just the visible filtered ones), consistent with how staging a directory in the files panel works. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
49977e6 to
5586a87
Compare
I had forgotten there was discussion about that: this time around doing the full path feels like a no-brainer: in rust you can often have a bunch of files with the same name
Good catch. I think it's less surprising if we only stage things which are visible. It also supports more use cases like only wanting to stage all
I've resolved this in the last commit. |
PR Description
Way back when I implemented filter functionality I could not muster the motivation to apply it to the file views (files and commit files), but it really should be used on them.
The text filter matches against the full file path (not just the filename), which is more useful for navigating large directory trees.
When a text filter is active, all operations respect it consistently:
Full disclosure, opus 4.6 wrote this entire PR but reviewing its changes they seem reasonable, and I have done some local testing and everything works as expected.
Please check if the PR fulfills these requirements
go generate ./...)