Skip to content

Conversation

@aybe
Copy link
Contributor

@aybe aybe commented Nov 7, 2025

Before:

Screenshot 2025-11-07 011334

After:

Screenshot 2025-11-07 005337

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 7, 2025

Walkthrough

The drive path string representation in ImFileDialog.cpp has been updated to include a trailing backslash when enumerating logical drives under "This PC". Drive paths now format as ":\" instead of ":", modifying only the path string construction for drive nodes.

Changes

Cohort / File(s) Summary
Drive path string formatting
third_party/ImFileDialog/ImFileDialog.cpp
Drive nodes now created with trailing backslash in path string (e.g., "C:\" vs "C:")

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Verify the trailing backslash change is applied consistently across all drive path constructions
  • Confirm the change does not impact path validation or downstream path processing logic

Poem

🐰 A backslash here, a backslash there,
The paths now dance with proper care,
From C: to C:\, the drives align,
A tiny tweak, but oh so fine! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main fix: correcting the behavior of the C:\ drive display in the file dialog, matching the observable change of adding a trailing backslash to drive paths.
Description check ✅ Passed The description provides visual before/after screenshots demonstrating the fix, which effectively communicates the functional change even though it lacks textual explanation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

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

Copy link
Contributor

@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.

Actionable comments posted: 0

🧹 Nitpick comments (1)
third_party/ImFileDialog/ImFileDialog.cpp (1)

412-412: LGTM! Drive paths now consistently include trailing backslash.

The fix correctly formats drive paths as "C:\" from creation, rather than relying on the workaround in m_setDirectory (lines 932-936). This is consistent with Windows path conventions and other path constructions in the file (e.g., line 386).

The workaround at lines 932-936 is now technically redundant, though keeping it as defensive code is reasonable. Consider adding a comment explaining it's a safety net, or removing it in a future cleanup if you're confident all code paths construct drive paths correctly.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 47d83a2 and c37d059.

📒 Files selected for processing (1)
  • third_party/ImFileDialog/ImFileDialog.cpp (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: macos-intel-build-and-test
  • GitHub Check: macos-arm-build-and-test
  • GitHub Check: pcsx-redux (aarch64-linux)
  • GitHub Check: pcsx-redux (x86_64-linux)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: build
  • GitHub Check: cross-arm64
  • GitHub Check: asan
  • GitHub Check: macos-build-and-test-toolchain
  • GitHub Check: coverage
  • GitHub Check: toolchain

@nicolasnoble nicolasnoble merged commit 1a678f4 into grumpycoders:main Nov 13, 2025
21 of 22 checks passed
@aybe aybe deleted the fix-wrong-c-drive branch November 13, 2025 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants