Introduce Path object to siplify use of std::filesystem::path with QS…#817
Open
TheOneRing wants to merge 1 commit intomainfrom
Open
Introduce Path object to siplify use of std::filesystem::path with QS…#817TheOneRing wants to merge 1 commit intomainfrom
TheOneRing wants to merge 1 commit intomainfrom
Conversation
TheOneRing
force-pushed
the
work/path2
branch
from
cd038f2 to
6767b77
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new OCC::FileSystem::Path wrapper to ease interop between QString and std::filesystem::path, and refactors VFS setup code to derive the VFS root path from the SyncEngine rather than passing it around as a mutable QString.
Changes:
- Add
OCC::FileSystem::Path(src/libsync/path.{h,cpp}) and wire it into libsync build. - Refactor
VfsSetupParamsto remove thefilesystemPathdata member and addfilesystemPath()/root()accessors; update CfApi call sites accordingly. - Adjust filesystem path conversion helpers (
toFilesystemPath/fromFilesystemPath) and remove manual VFS path assignments in GUI/tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/testutils/syncenginetestutils.cpp | Stops manually setting VFS root path in tests; relies on SyncEngine path via VfsSetupParams. |
| src/gui/folder.cpp | Stops manually setting VFS root path in GUI; relies on SyncEngine path via VfsSetupParams. |
| src/libsync/vfs/vfs.h | Removes filesystemPath member; introduces accessors and FileSystem::Path root storage. |
| src/libsync/vfs/vfs.cpp | Initializes/stores VFS root as FileSystem::Path; uses it in dehydrated file cleanup. |
| src/plugins/vfs/cfapi/vfs_cfapi.cpp | Updates usages to call filesystemPath() accessor. |
| src/plugins/vfs/cfapi/cfapiwrapper.cpp | Updates usages to call filesystemPath() accessor (notably in async WinRT callback). |
| src/libsync/path.h / src/libsync/path.cpp | Adds new FileSystem::Path wrapper type and basic join/conversion helpers. |
| src/libsync/common/filesystembase.h / src/libsync/common/filesystembase.cpp | Changes toFilesystemPath signature and implementation; updates fromFilesystemPath conversion. |
| src/libsync/CMakeLists.txt | Adds path.cpp to libsync build sources. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+8
to
+31
| OCC::FileSystem::Path::Path(QAnyStringView path) | ||
| : _path(toFilesystemPath(path.toString())) | ||
| { | ||
| } | ||
|
|
||
| OCC::FileSystem::Path::Path(const std::filesystem::path &path) | ||
| : _path(path) | ||
| { | ||
| } | ||
|
|
||
| OCC::FileSystem::Path::Path(std::filesystem::path &&path) | ||
| : _path(std::move(path)) | ||
| { | ||
| } | ||
|
|
||
| OCC::FileSystem::Path OCC::FileSystem::Path::relative(QAnyStringView path) | ||
| { | ||
| return QtPrivate::toFilesystemPath(path.toString()); | ||
| } | ||
|
|
||
| QString OCC::FileSystem::Path::toString() const | ||
| { | ||
| return fromFilesystemPath(_path); | ||
| } |
There was a problem hiding this comment.
FileSystem::Path is a new cross-platform path abstraction with Windows-specific behavior (long-path/\\?\ handling) and several operator overloads (relative, /, toString). There don’t appear to be tests covering its conversion/join behavior yet; adding a small unit test (including a Windows-only section) would help prevent regressions in path normalization and string round-tripping.
TheOneRing
force-pushed
the
work/path2
branch
from
8a89c48 to
0f2109c
Compare
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
1 participant
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.
…tring