(Low-priority) various minor code improvements#1751
Open
Cheong-Lau wants to merge 5 commits into
Open
Conversation
Contributor
Author
|
looks like I've duplicated some of the changes in #1742, specifically 971374f. I've done things using I personally think the |
The diff is mostly indentation changes from moving around code. Most of the changes in this commit should not affect behaviour meaningfully.
This is done in two places: `tab::Item::display_name()` and `thumbnail_cacher::thumbnail_uri()`, where both functions replace two substrings. `aho-corasick` does this faster without allocating redundantly the second time. This is already a transient dependency so this doesn't add any new deps.
This changes the `Mounter` trait methods and other functions that return `Box<dyn Error>` or `String` to now return an `anyhow::Error`. The `large_image` instead has its error handling simplified with a new simple custom error type.
This removes two `Mutex`s and one `RwLock` and replaces them with `AtomicCell`, which should improve performance since they do not require locks. This commit also removes an unnecessary `Arc<Mutex>` that was wrapping `mpsc::Sender<Message>`, which already has `Arc<Mutex>` internally.
This commit replaces buffers like `Vec`, `String` and `PathBuf` with `Box` or `Arc`, depending on whether they were likely to be cloned, with other minor improvements as well
jacobgkau
approved these changes
Apr 20, 2026
jacobgkau
left a comment
jacobgkau
left a comment
Member
There was a problem hiding this comment.
Regression testing passed:
Checklist
Basic navigation
- Middle-click opens directory in a new tab (not focused).
- Open two scrollable tabs. Scroll one tab, then switch to the other tab; it should not have scrolled.
- Hover over the top item in the folder, then scroll down until it's out of view (while still hovered). On scrolling back up (with the mouse in a different position), the item should not have the hover highlight.
- Right-click an item in the sidebar. No visual change should occur with the rest of the items.
- Remove an item from the sidebar, then re-pin it.
File operations
- Right-click -> Create a new folder, then enter it.
- Right-click in the empty folder -> Create a new file.
- Navigate to the parent folder, create another new file, then drag it into the created folder.
- Files can be renamed.
- Files can be opened with non-default apps & browsing store for new apps works.
- Normal right-click shows
Move to trashoption. - Shift right-click, and right-click followed by Shift, both show
Permanently deleteoption.
Advanced navigation & view settings
- Image and video thumbnails generate & display in local folders.
- Gallery preview shows with Spacebar.
- Details pane shows with Ctrl+Spacebar.
- Zoom in/out and reset to default zoom work.
- Ctrl+1 and Ctrl+2 switch between list and icon view.
- Ctrl+H shows/hides hidden files.
- Directories can be sorted at top or inline.
- Settings -> Theme works.
- Settings -> Type to Search affects behavior as designed.
- Single-click to open setting takes effect.
- Recents in sidebar setting works.
- Sorting options work.
- Cutting, copying, and pasting files works.
- F5 reloads current directory.
- Left sidebar can be collapsed and expanded.
External filesystems
- Add a network drive (e.g. SFTP) and navigate into it.
- Plug in a USB drive; able to mount, browse, and eject.
Integrations
- Desktop icons display as expected
- Drag-and-drop into Firefox works
I'm not seeing any differences in behavior with this version. The code changes are up to the engineering team's discretion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Another large PR that adds nothing but makes things technically better :)
Adds three dependencies that were already being built, so they're not new (see lockfile diff)
Some highlights:
I apologise for the massive diffs, especially in the first commit. But these changes have built up over the past 6 months and there's a lot of code.
Thoroughly tested on my own machine, but I may not have caught everything.
No AI/LLMs used, only a lot of reading through the codebase / clippy warnings and making manual changes.