Skip to content

Add Platform Utils / Show Labels by OS in File Explorer#416

Merged
pedramamini merged 4 commits intoRunMaestro:mainfrom
chr1syy:fix-reveal-label
Feb 19, 2026
Merged

Add Platform Utils / Show Labels by OS in File Explorer#416
pedramamini merged 4 commits intoRunMaestro:mainfrom
chr1syy:fix-reveal-label

Conversation

@chr1syy
Copy link
Copy Markdown
Contributor

@chr1syy chr1syy commented Feb 19, 2026

This pull request improves platform awareness and user interface labeling for file manager actions in the application. The most significant changes include introducing a utility function to generate platform-specific labels (e.g., "Reveal in Finder", "Reveal in Explorer", "Reveal in File Manager"), updating UI components and tests to use this function, and exposing the platform string synchronously to the renderer process.

Platform-aware labeling and UI updates:

  • Added a new utility function getRevealLabel in platformUtils.ts to return the appropriate label for the "reveal in file manager" action based on the user's operating system.
  • Updated FileExplorerPanel and TabBar components to use getRevealLabel(window.maestro.platform) instead of hardcoding "Reveal in Finder", ensuring the label matches the user's platform. [1] [2] [3] [4]

Platform detection and preload changes:

  • Exposed the platform string synchronously to the renderer process via window.maestro.platform in the preload script, replacing the need for an asynchronous IPC call.
  • Updated tests and mocks to include the platform property on window.maestro, ensuring consistent platform detection in test environments. [1] [2] [3] [4] [5] [6]

Test updates for platform-aware labels:

  • Modified tests in FileExplorerPanel.test.tsx and TabBar.test.tsx to check for the correct platform-specific label using regular expressions, rather than hardcoding "Reveal in Finder". [1] [2] [3] [4] [5]

IPC handler additions:

  • Added IPC handlers in system.ts to retrieve the platform and architecture, though the platform is now exposed synchronously via preload.

Summary by CodeRabbit

  • Bug Fixes

    • The "reveal in file manager" button now shows a platform-appropriate label (Finder on macOS, Explorer on Windows, File Manager on Linux), improving clarity and consistency.
  • Tests

    • Tests were made platform-agnostic so the reveal action label is validated reliably across operating systems.

chr1syy and others added 2 commits February 19, 2026 07:12
Add OS platform detection via IPC so UI components can show the correct
label for the reveal action: "Reveal in Explorer" on Windows, "Reveal in
Finder" on macOS/Linux.

- Add os:getPlatform and os:getArch IPC handlers in system.ts
- Expose OsApi via preload contextBridge
- Add useOsPlatform hook with module-level caching to avoid repeated IPC
- Update FileExplorerPanel and TabBar to use platform-aware revealLabel
- Update tests to accept both label variants via regex

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove OsApi interface, createOsApi function, OsApiType from preload/system.ts
- Replace `os: createOsApi()` with `platform: process.platform` in preload/index.ts
- Update MaestroAPI type in global.d.ts: replace `os` namespace with `platform: string`
- Add shared getRevealLabel(platform) utility in src/renderer/utils/platformUtils.ts
  with correct Linux support ("Reveal in File Manager")
- Delete useOsPlatform hook (src/renderer/hooks/useOsPlatform.ts)
- Update FileExplorerPanel.tsx: use getRevealLabel(window.maestro.platform) directly,
  removing async hook call and useMemo derive
- Update TabBar.tsx: same refactor as FileExplorerPanel.tsx
- Update test setup.ts: add platform: 'darwin' to global window.maestro mock
- Update FileExplorerPanel.test.tsx: add platform to maestro mock in beforeEach and
  individual tests that override window.maestro; update regex to include File Manager
- Update TabBar.test.tsx: update reveal label regex assertions to include File Manager

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Feb 19, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Expose a synchronous platform string on window.maestro, add getRevealLabel(platform) utility, update UI to use the platform-aware reveal label, and adjust tests to mock maestro.platform and accept Finder/Explorer/File Manager variants.

Changes

Cohort / File(s) Summary
Preload & Type Declarations
src/main/preload/index.ts, src/renderer/global.d.ts
Add platform: process.platform to the Maestro API and declare MaestroAPI.platform: string.
Platform Utility
src/renderer/utils/platformUtils.ts
Add getRevealLabel(platform: string) returning platform-specific "Reveal in ..." labels (win32 → Explorer, linux → File Manager, default Finder).
UI Components
src/renderer/components/FileExplorerPanel.tsx, src/renderer/components/TabBar.tsx
Replace static "Reveal in Finder" text with getRevealLabel(window.maestro.platform) and update related comments.
Tests / Test Setup
src/__tests__/setup.ts, src/__tests__/renderer/components/FileExplorerPanel.test.tsx, src/__tests__/renderer/components/TabBar.test.tsx
Mock maestro.platform = 'darwin' synchronously in test setup and relax assertions to accept Finder/Explorer/File Manager via regex.
Minor Formatting
src/main/ipc/handlers/system.ts
Whitespace-only formatting change (extra blank line).

Sequence Diagram

sequenceDiagram
    participant Main as Main Process
    participant Preload as Preload Script
    participant Renderer as Renderer Process
    participant Component as UI Component

    Main->>Preload: read process.platform
    Preload->>Renderer: expose window.maestro.platform
    Component->>Renderer: read window.maestro.platform
    Component->>Component: call getRevealLabel(platform)
    Component->>Component: render platform-specific "Reveal in ..." label
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: adding platform utilities and displaying OS-specific labels in the file explorer.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Feb 19, 2026

Greptile Summary

This PR improves cross-platform user experience by replacing hardcoded "Reveal in Finder" labels with platform-aware labels that display "Reveal in Explorer" on Windows, "Reveal in File Manager" on Linux, and "Reveal in Finder" on macOS.

Key changes:

  • Added platformUtils.ts with a getRevealLabel() utility function for platform detection
  • Exposed process.platform synchronously via window.maestro.platform in the preload script (appropriate since platform never changes at runtime)
  • Updated FileExplorerPanel and TabBar components to use the new utility function
  • Updated all tests to mock the platform property and use regex matching for platform-agnostic assertions
  • Added IPC handlers for os:getPlatform and os:getArch, though these are currently unused since the platform is now available synchronously

Minor suggestion:

  • The newly added IPC handlers in system.ts (lines 591-598) are unused since platform detection is now synchronous. Consider removing them to avoid maintaining dead code unless they're planned for future use.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The implementation is clean, well-tested, and follows best practices. All changes are straightforward refactoring with comprehensive test coverage. The approach of exposing platform synchronously is appropriate since process.platform never changes at runtime. Only minor cleanup suggestion regarding unused IPC handlers.
  • No files require special attention - all changes are straightforward and well-implemented

Important Files Changed

Filename Overview
src/renderer/utils/platformUtils.ts New utility function added to return platform-specific file manager labels - clean implementation with proper comments
src/main/preload/index.ts Exposed process.platform synchronously via contextBridge - appropriate since platform never changes at runtime
src/main/ipc/handlers/system.ts Added IPC handlers for platform and arch detection - handlers are unused since platform is now synchronous in preload
src/renderer/components/FileExplorerPanel.tsx Updated to use getRevealLabel(window.maestro.platform) for platform-aware labeling in context menu
src/renderer/components/TabBar.tsx Updated to use getRevealLabel(window.maestro.platform) for platform-aware labeling in file tab menu

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Main Process: preload/index.ts] -->|Expose process.platform| B[contextBridge]
    B -->|window.maestro.platform| C[Renderer Process]
    C --> D[platformUtils.ts]
    D -->|getRevealLabel| E{Platform Check}
    E -->|win32| F[Return 'Reveal in Explorer']
    E -->|linux| G[Return 'Reveal in File Manager']
    E -->|darwin/other| H[Return 'Reveal in Finder']
    F --> I[FileExplorerPanel.tsx]
    G --> I
    H --> I
    F --> J[TabBar.tsx]
    G --> J
    H --> J
    I -->|Display label in context menu| K[User sees platform-specific text]
    J -->|Display label in tab menu| K
Loading

Last reviewed commit: 00c3115

Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

9 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment thread src/main/ipc/handlers/system.ts Outdated
Comment on lines +591 to +598
ipcMain.handle('os:getPlatform', async () => {
return process.platform;
});

// Get architecture (x64, arm64, etc.)
ipcMain.handle('os:getArch', async () => {
return process.arch;
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These IPC handlers (os:getPlatform and os:getArch) are unused since window.maestro.platform is now exposed synchronously via the preload script. Consider removing to avoid maintaining dead code.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/renderer/utils/platformUtils.ts (1)

1-6: Minor: JSDoc comment mentions "linux-unknown" which is misleading.

The comment states darwin / linux-unknown → "Reveal in Finder" (macOS) but "linux-unknown" is not macOS. Consider clarifying that the default case covers darwin (macOS) and any other/unknown platforms.

📝 Suggested documentation fix
 /**
  * Returns the platform-appropriate label for the "reveal in file manager" action.
- *   darwin / linux-unknown → "Reveal in Finder" (macOS)
+ *   darwin (and other/unknown) → "Reveal in Finder" (macOS default)
  *   win32               → "Reveal in Explorer" (Windows)
  *   linux               → "Reveal in File Manager" (Linux)
  */
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/utils/platformUtils.ts` around lines 1 - 6, Update the top JSDoc
in platformUtils.ts to remove the misleading "linux-unknown" entry and clearly
describe mappings: specify that darwin → "Reveal in Finder" (macOS), win32 →
"Reveal in Explorer" (Windows), linux → "Reveal in File Manager" (Linux), and
that any unknown/other platforms fall back to the default behavior (e.g., treat
as darwin or use a generic label). Edit the comment block so it references
"unknown/other" or "default" instead of "linux-unknown" and ensure the mapping
lines and examples reflect the actual behavior implemented by the module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/renderer/utils/platformUtils.ts`:
- Around line 1-6: Update the top JSDoc in platformUtils.ts to remove the
misleading "linux-unknown" entry and clearly describe mappings: specify that
darwin → "Reveal in Finder" (macOS), win32 → "Reveal in Explorer" (Windows),
linux → "Reveal in File Manager" (Linux), and that any unknown/other platforms
fall back to the default behavior (e.g., treat as darwin or use a generic
label). Edit the comment block so it references "unknown/other" or "default"
instead of "linux-unknown" and ensure the mapping lines and examples reflect the
actual behavior implemented by the module.

…ment

- Remove unused os:getPlatform and os:getArch IPC handlers (platform is
  now exposed synchronously via window.maestro.platform in preload)
- Fix misleading JSDoc comment in platformUtils.ts (darwin / linux-unknown
  → darwin and other/unknown)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@pedramamini pedramamini left a comment

Choose a reason for hiding this comment

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

Nice, clean PR. Synchronous process.platform via preload is the right approach — no IPC round-trip for a constant. getRevealLabel() utility is clean.

One issue to fix:

In src/__tests__/renderer/components/FileExplorerPanel.test.tsx line 271, the beforeEach override:

(window as any).maestro = { platform: 'darwin' };

This replaces the full mockMaestro from setup.ts (which already has platform: 'darwin') with a bare object. All other window.maestro.* properties are destroyed for every test in the describe block. It doesn't break existing tests because the ones needing fs/shell re-mock explicitly, but it's a landmine — any future test relying on the global mock will fail and the cause won't be obvious.

Fix: Just delete these two lines (the comment + the assignment). setup.ts already provides platform: 'darwin' in the global mock.

Everything else looks good.

The bare `(window as any).maestro = { platform: 'darwin' }` in beforeEach
replaced the full mockMaestro from setup.ts, destroying all other API
properties for every test in the block. setup.ts already provides
platform: 'darwin' in the global mock, so these two lines were redundant
and a future-test landmine.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@pedramamini pedramamini merged commit f892ded into RunMaestro:main Feb 19, 2026
1 check was pending
@chr1syy chr1syy deleted the fix-reveal-label branch April 14, 2026 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants