Skip to content

feat(file_browser): restore selection_mode='all' and accept list form#9630

Merged
kirangadhave merged 1 commit into
mainfrom
kg/file-browser-files-and-dirs
May 21, 2026
Merged

feat(file_browser): restore selection_mode='all' and accept list form#9630
kirangadhave merged 1 commit into
mainfrom
kg/file-browser-files-and-dirs

Conversation

@kirangadhave
Copy link
Copy Markdown
Member

@kirangadhave kirangadhave commented May 20, 2026

Summary

  • Restore the selection_mode="all" string on mo.ui.file_browser (regressed in fix: handle cloudpath client #4130).
  • Also accept a list/tuple form, e.g. selection_mode=["file", "directory"].
  • Internally normalized to a frozenset of selectable kinds; the wire format stays a single string ("file" | "directory" | "all"), so the frontend (which already handles "all") is unchanged.
  • Default remains "file"; no behavior change for existing callers.

Closes #9402

Re-introduces the 'all' string and adds a list form (e.g. ['file','directory'])
to mo.ui.file_browser, normalized internally to a frozenset of selectable
kinds. The wire format stays a single string ('file' | 'directory' | 'all'),
so the frontend is unchanged. Default remains 'file'.

Closes #9402
@vercel
Copy link
Copy Markdown

vercel Bot commented May 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment May 20, 2026 7:17pm

Request Review

@kirangadhave kirangadhave marked this pull request as ready for review May 20, 2026 19:17
Copilot AI review requested due to automatic review settings May 20, 2026 19:17
@kirangadhave kirangadhave added the bug Something isn't working label May 20, 2026
@kirangadhave kirangadhave requested a review from mscolnick May 20, 2026 19:17
@kirangadhave
Copy link
Copy Markdown
Member Author

@cubic-dev-ai

@cubic-dev-ai
Copy link
Copy Markdown
Contributor

cubic-dev-ai Bot commented May 20, 2026

@cubic-dev-ai

@kirangadhave I have started the AI code review. It will take a few minutes to complete.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 3 files

Architecture diagram
sequenceDiagram
    participant User as User (Python cell)
    participant FileBrowser as mo.ui.file_browser __init__
    participant Normalizer as _normalize_selection_mode()
    participant WireFormat as Wire format builder
    participant Frontend as Frontend component
    participant DirLister as _list_directory()
    participant FileSystem as Filesystem

    Note over User,FileSystem: Initialization flow

    User->>FileBrowser: file_browser(selection_mode="all")
    FileBrowser->>Normalizer: NEW: normalize "all"
    Normalizer-->>FileBrowser: frozenset({"file","directory"})

    User->>FileBrowser: file_browser(selection_mode=["file","directory"])
    FileBrowser->>Normalizer: NEW: normalize list
    Normalizer-->>FileBrowser: frozenset({"file","directory"})

    User->>FileBrowser: file_browser(selection_mode="file") (unchanged)
    FileBrowser->>Normalizer: normalize "file"
    Normalizer-->>FileBrowser: frozenset({"file"})

    Note over FileBrowser,WireFormat: Derive wire format for frontend

    alt selection_mode == {"file","directory"} (i.e. "all")
        FileBrowser->>WireFormat: wire = "all"
    else single kind selected
        FileBrowser->>WireFormat: wire = single element from frozenset
    end
    WireFormat-->>FileBrowser: "all" | "file" | "directory"
    FileBrowser-->>User: UIElement created (unchanged frontend contract)

    Note over Frontend,FileSystem: Directory listing flow (CHANGED: filtering logic)

    Frontend->>DirLister: list_directory(path)
    DirLister->>FileSystem: read directory entries
    FileSystem-->>DirLister: list of entries (files + directories)

    loop each entry
        alt entry is directory
            DirLister->>DirLister: Always included (for navigation)
        else entry is file
            alt "file" in self._selection_mode
                DirLister->>DirLister: Include (subject to filetypes)
            else "file" not in self._selection_mode
                DirLister->>DirLister: NEW: Skip file (was only directory mode previously)
            end
        end
    end

    DirLister-->>Frontend: filtered file list
Loading

Re-trigger cubic

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Restores mo.ui.file_browser(selection_mode="all") support (regressed previously) and extends selection_mode to also accept a list/tuple form, while keeping the frontend wire format limited to "file" | "directory" | "all".

Changes:

  • Added internal normalization of selection_mode to a frozenset of selectable kinds, supporting "all" and list/tuple inputs.
  • Updated directory listing behavior to hide files when file selection is disabled, while always showing directories for navigation.
  • Added/updated tests and an example demonstrating selection_mode="all".

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
marimo/_plugins/ui/_impl/file_browser.py Adds selection mode normalization + wire-format mapping and updates listing behavior accordingly.
tests/_plugins/ui/_impl/test_file_browser.py Updates existing assertions and adds coverage for normalization, "all" mode, and list/tuple inputs.
examples/ui/file_browser.py Demonstrates a file browser instance using selection_mode="all".

Comment thread marimo/_plugins/ui/_impl/file_browser.py
Comment thread marimo/_plugins/ui/_impl/file_browser.py
@kirangadhave kirangadhave requested a review from Light2Dark May 20, 2026 23:54
Comment on lines +170 to +173
selection_mode (str | Sequence[str], optional): Which kinds of entries
the user can select. Accepts one of "file" (default), "directory",
"all", or a list/tuple containing "file" and/or "directory".
"all" is equivalent to ["file", "directory"]. Defaults to "file".
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't this mean ["file", "directory"] is the same as "all". What's the reason to have both list & str support?

Copy link
Copy Markdown
Member Author

@kirangadhave kirangadhave May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah it's same as all. If we add something like symlinks or some other type in future, we can pick and choose with this pattern. all will always be all supported types. just a bit of over-engineering for future proofing.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it, im usually not keen on over-eng, but leaving it to you.

@kirangadhave kirangadhave requested a review from Light2Dark May 21, 2026 00:49
@kirangadhave kirangadhave merged commit 9a149d6 into main May 21, 2026
49 of 51 checks passed
@kirangadhave kirangadhave deleted the kg/file-browser-files-and-dirs branch May 21, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

file_browser() support for selecting either/both files and directories from one instance

3 participants