send absolute path to frontend for file_browser#8668
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
||
| # Frontend can't handle relative paths, so normalize it and make it absolute | ||
| # Use normalize_path to avoid symlink resolution | ||
| self._initial_path = self._create_path(normalize_path(initial_path)) |
There was a problem hiding this comment.
normalized here
There was a problem hiding this comment.
Pull request overview
Updates the file_browser UI element backend to always send a normalized absolute initial-path to the frontend, preventing frontend path delimiter mis-detection (Fixes #8665).
Changes:
- Send
self._initial_path(normalized/absolute) instead of the rawinitial_pathin the component args. - Add a regression test ensuring relative
initial_pathinputs are transmitted to the frontend as absolute paths and that navigating “up” works. - Minor formatting change to the restricted-navigation RuntimeError message.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
marimo/_plugins/ui/_impl/file_browser.py |
Uses the normalized absolute _initial_path when emitting initial-path to the frontend. |
tests/_plugins/ui/_impl/test_file_browser.py |
Adds a test covering absolute-path transmission and parent navigation from the initial directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| assert "/" in initial_path_arg, ( | ||
| f"initial-path sent to frontend must contain '/' " | ||
| f"(be absolute), got: {initial_path_arg!r}" | ||
| ) | ||
| assert Path(initial_path_arg).is_absolute() | ||
|
|
||
| # Navigating up from the initial path should work | ||
| parent = str(Path(initial_path_arg).parent) |
There was a problem hiding this comment.
This test hard-codes '/' as the path separator ("assert '/' in initial_path_arg"). That will fail on Windows where absolute paths typically contain '\' (and may not contain '/'), even though the backend behavior is correct. Consider asserting that the value is absolute and/or equals the expected normalized path (e.g., Path(initial_path_arg) == sub_dir), or use os.sep for a separator check instead of '/'.
| assert "/" in initial_path_arg, ( | |
| f"initial-path sent to frontend must contain '/' " | |
| f"(be absolute), got: {initial_path_arg!r}" | |
| ) | |
| assert Path(initial_path_arg).is_absolute() | |
| # Navigating up from the initial path should work | |
| parent = str(Path(initial_path_arg).parent) | |
| initial_path = Path(initial_path_arg) | |
| # The initial path sent to the frontend should be an absolute, | |
| # normalized path pointing to the expected subdirectory. | |
| assert initial_path.is_absolute() | |
| assert initial_path == sub_dir | |
| # Navigating up from the initial path should work | |
| parent = str(initial_path.parent) |
|
im fixing this CI issue btw, elsehwere |
tq! |
📝 Summary
Fixes #8665
In the backend, we passed the relative path to the frontend. We should instead send the normalized absolute path.
The un-normalized path would cause errors with the frontend's guessDeliminator, so passing "." would parse the delimiter as
\\even on Mac, when it should be/before:
after:
🔍 Description of Changes
📋 Checklist