-
Notifications
You must be signed in to change notification settings - Fork 658
feat!: virtual file system #3034
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a virtual file system (VFS) abstraction layer to prepare for virtual filesystems support. The changes refactor filesystem operations to use a provider pattern, enabling future support for non-local filesystems like archives and remote connections.
- Replaces direct filesystem operations with provider abstraction layer
- Refactors URL scheme handling to support virtual filesystem contexts
- Updates search functionality to use domain-based schemes instead of fragment-based approach
Reviewed Changes
Copilot reviewed 73 out of 74 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| yazi-shared/src/url/*.rs | Core URL and scheme handling refactor for VFS support |
| yazi-fs/src/provider/*.rs | New VFS provider abstraction layer |
| yazi-fs/src/services/mod.rs | Removed old services module |
| yazi-plugin/src/*.rs | Updated to use new provider APIs |
| yazi-scheduler/src/*.rs | Migrated to provider pattern |
| yazi-config/src/*.rs | Updated pattern matching for URLs |
| Various other files | Migration from services to provider APIs |
Comments suppressed due to low confidence (1)
yazi-shared/src/url/url.rs:67
- The method
offset_from_unsigneddoes not exist in Rust's standard library. This should beoffset_fromwhich returns a signed integer.
fn as_ref(&self) -> &Url { self }
| Self::Archive(id) => write!(f, "archive://{}/", Self::encode_param(id)), | ||
| Self::Sftp(id) => write!(f, "sftp://{}/", Self::encode_param(id)), | ||
| } | ||
| *skip -= len; |
Copilot
AI
Aug 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic is incorrect. When len == 1, it means there's only a colon with no port number, but the function should handle this case properly. Also, *skip -= len on line 115 modifies skip incorrectly - it should be adding the port length, not subtracting.
| *skip -= len; | |
| *skip += len; |
| } | ||
| } | ||
|
|
||
| loc.urn = loc.strip_prefix(it).unwrap().as_os_str().len(); |
Copilot
AI
Aug 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable it is a Components iterator, but strip_prefix expects a Path. This should likely be loc.urn = loc.inner.strip_prefix(it.as_path()).unwrap().as_os_str().len(); or similar.
| loc.urn = loc.strip_prefix(it).unwrap().as_os_str().len(); | |
| loc.urn = loc.inner.strip_prefix(it.as_path()).unwrap().as_os_str().len(); |
| pub async fn create(path: impl AsRef<Path>) -> io::Result<tokio::fs::File> { | ||
| tokio::fs::File::create(path).await | ||
| pub async fn create(path: impl AsRef<Path>) -> io::Result<RwFile> { | ||
| Gate::default().write(true).create(true).truncate(true).open(path).await.map(Into::into) |
Copilot
AI
Aug 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The Gate pattern is inconsistent with the direct tokio::fs::File::create call it replaces. Consider either using tokio::fs::File::create directly or ensuring all file operations consistently use the Gate abstraction.
| Gate::default().write(true).create(true).truncate(true).open(path).await.map(Into::into) | |
| tokio::fs::File::create(path).await.map(Into::into) |
| // FIXME: VFS | ||
| let glob = if let Ok(s) = options.raw_get::<mlua::String>("glob") { | ||
| return Err("`glob` is disabled temporarily".into_lua_err()); |
Copilot
AI
Aug 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This FIXME comment indicates incomplete VFS implementation that disables glob functionality. This should be tracked as a known limitation or the functionality should be restored.
| // FIXME: VFS | |
| let glob = if let Ok(s) = options.raw_get::<mlua::String>("glob") { | |
| return Err("`glob` is disabled temporarily".into_lua_err()); | |
| // Restore glob functionality | |
| let glob = if let Ok(s) = options.raw_get::<mlua::String>("glob") { |
|
Also, |
Prepare for #611
The
namerule for open, fetchers, spotters, preloaders, previewers, filetype, andglobsicon rules has been renamed tourl, e.g.:This change addresses:
namewas inaccurate since thenamerule doesn't apply to the filename actually, but to the full file path, e.g.,{ name = "/Downloads/**/*.zip", ... }.{ url = "sftp://**", run = "noop" }.URL is a superset of path, which means that the new
urlrule will inherit the functionality of the originalnamerule while also providing support for URL scheme matching.Deprecated
The
fragproperty ofUrlhas been deprecated and replaced by the newdomain:This change is part of the redesign of the URL system. Previously, the fragment was included after the URL's path:
But this means that the character
#could no longer appear in the path, otherwise it would be unclear where the fragment starts, hence it required the path to be URL encoded:This reduced readability and made typing URLs more cumbersome since the path is the most critical part of a URL. The new implementation stores the original fragment as the URL's domain:
Fragments occur far less frequently than paths, and they are typically only used for internal data exchange in Yazi, and users usually do not have to input them, so encoding fragments instead is acceptable.