fix(downloads): drop /select on Windows — opens wrong folder
Reported: clicking "Open Downloads folder" was opening the Documents folder instead of Downloads. Root cause is the classic Windows gotcha: when the path contains a space (e.g. ``C:\Users\Michael Dombaugh\Downloads``), Python's ``subprocess.Popen`` packs the ``/select,...`` argument into a single quoted token, and Explorer's ``/select`` argument parser does NOT accept that form — it silently falls back to whatever the user's default Explorer view is (typically Documents). Resolution paths considered: - ``shell=True`` with a hand-built command string — works but opens the door to shell-injection if a file_name ever contained a quote or special char. - ``cmd /c start "" explorer /select,...`` — same parsing issue. - ctypes ShellExecuteW — pulls in a Windows-only dependency. - **Skip /select. Open the folder directly.** ✓ Going with the last. ``explorer <folder>`` reliably opens the folder regardless of spaces in the path; the user finds the freshly-saved file by its name. The previous "highlight the file" nicety wasn't worth the path-parsing fragility — every user folder on Windows is ``C:\Users\<name>`` and every Windows username can contain a space. macOS keeps the ``open -R <file>`` reveal-in-Finder path because macOS argument parsing is sane and that's a strict UX win. 2220 tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -258,14 +258,18 @@ def _open_in_file_manager(folder: "Path", *, select: "Path | None" = None) -> bo
|
||||
"""Open the OS file manager at *folder*, optionally highlighting *select*.
|
||||
|
||||
Windows
|
||||
``explorer /select,<file>`` is preferred when *select* is given —
|
||||
Explorer pops to the foreground with the freshly-saved file
|
||||
already highlighted. Falls back to ``explorer <folder>``, then
|
||||
``os.startfile`` if both fail; ``os.startfile`` is known to
|
||||
silently no-op or open behind the active window in some Windows
|
||||
configurations, which is why we don't lead with it.
|
||||
``explorer <folder>`` only. We deliberately do NOT use
|
||||
``explorer /select,<file>``: when the path contains a space
|
||||
(e.g. ``C:\\Users\\Michael Dombaugh\\Downloads``), Python's
|
||||
``subprocess.Popen`` quotes the ``/select,...`` argument as one
|
||||
unit, and Explorer's ``/select`` parser does not handle that
|
||||
form — it silently falls back to opening the user's default
|
||||
view (typically Documents). Opening the bare folder works
|
||||
reliably regardless of spaces. ``os.startfile`` is kept as a
|
||||
last-resort fallback only.
|
||||
macOS
|
||||
``open -R <file>`` reveals the file in Finder.
|
||||
``open -R <file>`` reveals the file in Finder when ``select``
|
||||
is given; otherwise just opens the folder.
|
||||
Linux / *BSD
|
||||
``xdg-open`` on the folder. No reliable cross-distro way to
|
||||
highlight a specific file.
|
||||
@@ -278,19 +282,11 @@ def _open_in_file_manager(folder: "Path", *, select: "Path | None" = None) -> bo
|
||||
import subprocess
|
||||
|
||||
if sys.platform == "win32":
|
||||
# explorer.exe with /select wants a literal "/select," followed
|
||||
# by the path — no space between the comma and the filename, or
|
||||
# Explorer misinterprets and opens "My Computer" instead.
|
||||
attempts = []
|
||||
if select is not None:
|
||||
attempts.append(["explorer", f"/select,{select}"])
|
||||
attempts.append(["explorer", str(folder)])
|
||||
for cmd in attempts:
|
||||
try:
|
||||
subprocess.Popen(cmd)
|
||||
subprocess.Popen(["explorer", str(folder)])
|
||||
return True
|
||||
except Exception:
|
||||
continue
|
||||
pass
|
||||
try:
|
||||
os.startfile(str(folder)) # type: ignore[attr-defined]
|
||||
return True
|
||||
|
||||
Reference in New Issue
Block a user