feat(config): add file_panel.sort_file custom comparator#3
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for custom file sorting in the file panel by introducing a new file_panel.sort_file configuration option. Users can now provide a custom comparator function to control how files are sorted in the file tree, while directories continue to always appear before files.
Changes:
- Added
sort_fileconfiguration option tofile_panelconfig (defaults to nil) - Modified
Node.comparatorto use the custom comparator for file-to-file comparisons when configured - Updated documentation comments to reflect the new behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| lua/diffview/config.lua | Adds the sort_file configuration option to file_panel defaults |
| lua/diffview/ui/models/file_tree/node.lua | Implements custom comparator support in Node.comparator function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not a_dir then | ||
| local sort_file = config.get_config().file_panel.sort_file | ||
| if sort_file and type(sort_file) == "function" then | ||
| return sort_file(a.name, b.name, a.data, b.data) |
There was a problem hiding this comment.
The custom comparator function is called without error handling. If the user-provided function throws an error or returns an incorrect type, it could crash the sorting operation. Consider wrapping the custom comparator call in pcall to handle potential errors gracefully and fall back to the default comparison.
Example pattern:
local ok, result = pcall(sort_file, a.name, b.name, a.data, b.data)
if ok and type(result) == "boolean" then
return result
end
-- Fall back to default comparison on error| return sort_file(a.name, b.name, a.data, b.data) | |
| local ok, result = pcall(sort_file, a.name, b.name, a.data, b.data) | |
| if ok and type(result) == "boolean" then | |
| return result | |
| end |
| }, | ||
| file_panel = { | ||
| listing_style = "tree", | ||
| sort_file = nil, -- Custom file comparator: function(a_name, b_name, a_data, b_data) -> boolean |
There was a problem hiding this comment.
The new sort_file configuration option is not documented in the documentation files (doc/diffview.txt and doc/diffview_defaults.txt). Users will not know how to use this feature without proper documentation.
The documentation should explain:
- The function signature:
function(a_name, b_name, a_data, b_data) -> boolean - What
a_nameandb_namerepresent (file names) - What
a_dataandb_datarepresent (FileEntry objects) - That this only affects file-to-file comparisons (not directories)
- An example of how to use it (e.g., sorting by file extension or status)
| function Node.comparator(a, b) | ||
| if a:has_children() == b:has_children() then | ||
| return string.lower(a.name) < string.lower(b.name) | ||
| else | ||
| return a:has_children() | ||
| local a_dir = a:has_children() | ||
| local b_dir = b:has_children() | ||
|
|
||
| if a_dir ~= b_dir then | ||
| return a_dir | ||
| end | ||
|
|
||
| -- For file nodes, use custom comparator if configured. | ||
| if not a_dir then | ||
| local sort_file = config.get_config().file_panel.sort_file | ||
| if sort_file and type(sort_file) == "function" then | ||
| return sort_file(a.name, b.name, a.data, b.data) | ||
| end | ||
| end | ||
|
|
||
| return string.lower(a.name) < string.lower(b.name) | ||
| end |
There was a problem hiding this comment.
The new custom file comparator feature lacks test coverage. The codebase has existing tests in lua/diffview/tests/functional/, but there are no tests for Node.comparator or the sorting functionality.
Consider adding tests that verify:
- Default sorting behavior (alphabetical, case-insensitive)
- Custom comparator is called for file nodes only
- Directories always sort before files regardless of custom comparator
- Handling of invalid custom comparator return values
- Behavior when custom comparator is nil or not a function
| }, | ||
| file_panel = { | ||
| listing_style = "tree", | ||
| sort_file = nil, -- Custom file comparator: function(a_name, b_name, a_data, b_data) -> boolean |
There was a problem hiding this comment.
The inline comment for sort_file could be more descriptive. It should clarify that:
- The comparator is only used for file-to-file comparisons (not directories)
- It should return true if a_name should come before b_name
- The a_data and b_data parameters are FileEntry objects containing file metadata like status, path, etc.
Consider updating to:
sort_file = nil, -- Custom file comparator (files only, dirs always first): function(a_name, b_name, a_data, b_data) -> boolean. Return true if a comes before b. a_data/b_data are FileEntry objects.| sort_file = nil, -- Custom file comparator: function(a_name, b_name, a_data, b_data) -> boolean | |
| sort_file = nil, -- Custom file comparator (files only, dirs always first): function(a_name, b_name, a_data, b_data) -> boolean. Return true if a comes before b. a_data/b_data are FileEntry objects. |
No description provided.