fix(fs): file transfer, reconnect, restore dir#41
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughFileController.onReady builds a de-duplicated, ordered candidate list of directories (current path, saved peer dir, home) and sequentially attempts to open each until one succeeds, retrying after reconnect delay if none do. Navigation methods now return Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flutter/lib/models/file_model.dart`:
- Around line 410-415: The logic now uses directory.value.path as currentPath
causing a stale path to be preferred and preventing the later home fallback if
openDirectory fails; change selectFileControllerReadyPath/openDirectory usage so
you first determine a safe path (fall back to savedDir or options.value.home
when directory.value.path looks stale) and/or handle openDirectory failures by
awaiting its result and, on error, calling openDirectory with options.value.home
(or re-run the savedDir fallback) so the file panel isn't left stuck; touch the
selectFileControllerReadyPath invocation and the openDirectory call to add the
fallback/error handling around directory.value.path, savedDir, and
options.value.home.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 22348bf3-ddf8-493d-8a43-8c7fb4910273
📒 Files selected for processing (1)
flutter/lib/models/file_model.dart
fufesou
force-pushed
the
fix/file-transfer-reconnect-dir
branch
from
3826a6b to
aeea19f
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
Improves the file manager’s initial directory restore/selection flow by trying multiple candidate directories (current, saved, home) and using an openDirectory success/failure result to decide which to keep.
Changes:
- Add helper to build an ordered list of candidate directories to open on controller ready.
- Update
onReady()to attempt opening current/saved/home paths until one succeeds. - Change
openDirectory,refresh, andgoToParentDirectoryto returnFuture<bool>indicating success.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fufesou
force-pushed
the
fix/file-transfer-reconnect-dir
branch
from
aeea19f to
b38454c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fufesou
force-pushed
the
fix/file-transfer-reconnect-dir
branch
from
b38454c to
0e08942
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fufesou
force-pushed
the
fix/file-transfer-reconnect-dir
branch
from
0e08942 to
f076bb5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fufesou
force-pushed
the
fix/file-transfer-reconnect-dir
branch
from
f076bb5 to
957158c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fufesou
force-pushed
the
fix/file-transfer-reconnect-dir
branch
from
957158c to
38e22c6
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flutter/lib/models/file_model.dart`:
- Around line 50-70: The helper _selectFileControllerReadyPaths currently always
adds homePath even when empty, causing openDirectory("") and duplicate retry
attempts; change _selectFileControllerReadyPaths to only add homePath when it is
not empty (same guard as for currentPath/savedPath), and also update the
retry/enqueue logic that calls openDirectory (referenced around the retry block
that checks lines ~433-435) to skip enqueuing or retrying when homePath is empty
so openDirectory("") is never attempted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2677141c-2116-4b21-b1e2-caff13fcc1f7
📒 Files selected for processing (1)
flutter/lib/models/file_model.dart
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
flutter/lib/models/file_model.dart:479
pushHistory()is called before verifying the directory open succeeds. IffetchDirectory()throws,directory.value.pathstays unchanged but history still records it, and subsequentgoBack()will pop the current path (and potentially clear additional history entries due to thedirectory.value.path == pathrecursion). Consider only pushing history after a successful open (or reverting the push when returningfalse).
if (!isBack) {
pushHistory();
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fufesou
force-pushed
the
fix/file-transfer-reconnect-dir
branch
from
38e22c6 to
8330b59
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flutter/lib/models/file_model.dart`:
- Around line 505-517: The public method goToParentDirectory currently discards
the result by calling unawaited(_goToParentDirectory()); change
goToParentDirectory to return Future<bool> and return/await the underlying
_goToParentDirectory({bool isBack = false}) so callers can observe
success/failure; update any callers to await the Future<bool> and handle a false
return (or propagate) instead of assuming success; reference functions:
goToParentDirectory, _goToParentDirectory, and openDirectory.
- Around line 394-412: The reconnect logic currently attempts opening
directory.value.path, savedDir, and options.value.home once before a 1s delay
and if none opened only retries options.value.home; change it so after the delay
you retry the saved/current directories too: preserve savedDir and
directory.value.path (from bind.sessionGetPeerOption and directory.value.path),
and if opened is still false after the delay call openDirectory for
directory.value.path and savedDir (in that order) before falling back to calling
openDirectory(options.value.home, isBack: true); ensure you keep using the
existing openDirectory function and the opened flag so the first successful open
short-circuits further attempts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 403227a1-54c3-427f-bb8e-59096d74cc31
📒 Files selected for processing (1)
flutter/lib/models/file_model.dart
fufesou
force-pushed
the
fix/file-transfer-reconnect-dir
branch
from
8330b59 to
a9dd267
Compare
Signed-off-by: fufesou <linlong1266@gmail.com>
fufesou
force-pushed
the
fix/file-transfer-reconnect-dir
branch
from
a9dd267 to
0812f80
Compare
Signed-off-by: fufesou <linlong1266@gmail.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
flutter/lib/models/file_model.dart (1)
464-485:⚠️ Potential issue | 🟠 MajorRecord history only after a directory open succeeds.
pushHistory()runs beforefetchDirectory(). If the open fails, the stale entry stays on the stack and latergoBack()calls can skip or drop entries, leaving navigation in a bad state after a typo or reconnect failure.Proposed fix
Future<bool> _openDirectoryPath(String path, {bool isBack = false}) async { - if (!isBack) { - pushHistory(); - } final showHidden = options.value.showHidden; final isWindows = options.value.isWindows; @@ try { final fd = await fileFetcher.fetchDirectory(path, isLocal, showHidden); fd.format(isWindows, sort: sortBy.value); + if (!isBack) { + pushHistory(); + } directory.value = fd; return true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter/lib/models/file_model.dart` around lines 464 - 485, The current _openDirectoryPath calls pushHistory() before attempting fetchDirectory(), which records a history entry even when opening fails; move the pushHistory() call so it runs only after fetchDirectory succeeds (i.e., after obtaining and formatting fd but before updating directory.value), and preserve the conditional on isBack so history is only pushed when !isBack; keep the existing try/catch and error logging around fetchDirectory and do not change the failure path so no history is recorded on error.
♻️ Duplicate comments (1)
flutter/lib/models/file_model.dart (1)
396-417:⚠️ Potential issue | 🟡 MinorGuard the empty home path in the retry candidate list.
options.value.homeis still inserted unconditionally, so an empty string can still reach_openDirectoryPath("")on the first pass and the delayed retry. This is the same empty-home-path issue reported earlier.Proposed fix
final dirs = <String>{ if (directory.value.path.isNotEmpty) directory.value.path, if (savedDir.isNotEmpty) savedDir, - options.value.home, + if (options.value.home.isNotEmpty) options.value.home, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter/lib/models/file_model.dart` around lines 396 - 417, The set of candidate paths in tryOpenReadyDirs currently always includes options.value.home which can be empty; update tryOpenReadyDirs to only insert options.value.home when it is non-empty (e.g., guard with options.value.home.isNotEmpty) so _openDirectoryPath is never called with an empty string; keep other candidates (directory.value.path and savedDir) as-is so the delayed retry that calls tryOpenReadyDirs() uses the same guarded candidate list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@flutter/lib/models/file_model.dart`:
- Around line 464-485: The current _openDirectoryPath calls pushHistory() before
attempting fetchDirectory(), which records a history entry even when opening
fails; move the pushHistory() call so it runs only after fetchDirectory succeeds
(i.e., after obtaining and formatting fd but before updating directory.value),
and preserve the conditional on isBack so history is only pushed when !isBack;
keep the existing try/catch and error logging around fetchDirectory and do not
change the failure path so no history is recorded on error.
---
Duplicate comments:
In `@flutter/lib/models/file_model.dart`:
- Around line 396-417: The set of candidate paths in tryOpenReadyDirs currently
always includes options.value.home which can be empty; update tryOpenReadyDirs
to only insert options.value.home when it is non-empty (e.g., guard with
options.value.home.isNotEmpty) so _openDirectoryPath is never called with an
empty string; keep other candidates (directory.value.path and savedDir) as-is so
the delayed retry that calls tryOpenReadyDirs() uses the same guarded candidate
list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 04ea97bc-78ca-46c0-8aa1-884c6bf2618e
📒 Files selected for processing (1)
flutter/lib/models/file_model.dart
Signed-off-by: fufesou <linlong1266@gmail.com>
Summary by CodeRabbit