Fix %edit failing when path contains spaces (editor hook double-quoting)#15315
Open
shreyabyte wants to merge 1 commit into
Open
Fix %edit failing when path contains spaces (editor hook double-quoting)#15315shreyabyte wants to merge 1 commit into
%edit failing when path contains spaces (editor hook double-quoting)#15315shreyabyte wants to merge 1 commit into
Conversation
Carreau
force-pushed
the
fix-editorhook-path-spaces
branch
from
July 22, 2026 16:46
4e167ff to
e6b4fba
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #14961
This PR standardises filename handling by moving quoting responsibility into the editor hook.
Problem
%editsingle-quoted the target filename before passing it toself.shell.hooks.editor()if the path contained spaces. The default editor hook (IPython/core/hooks.py) then built a shell command by string-formattingeditor,linemark, andfilenametogether, without quotingfilenameat all — relying on%edithaving already done it. Custom hooks installed viaIPython/lib/editorhooks.pyalso callshlex.quote(filename)on their own, assuming a raw filename — so any path with spaces reaching those hooks got double-quoted and broken (e.g. the Notepad++ hook).In short, callers and hooks disagreed about who owns quoting, and only one side actually did it.
Fix
Established a clear contract:
hooks.editor()always receives a raw, unquotedfilename. Quoting/escaping for shell execution is the responsibility of the hook implementation.IPython/core/magics/code.py: removed the pre-quoting logic in%edit(quoted = "'%s'" % quoted); now passes the raw filename toself.shell.hooks.editor(), matching what_edit_macroalready did. Added comments at both call sites documenting the contract.IPython/core/hooks.py: added a small_quote_if_needed()helper, applied consistently to botheditorandfilenamebefore building thePopencommand string, so the default hook now correctly quotes filenames with spaces (previously only the editor binary path was quoted).IPython/lib/editorhooks.py: unchanged — itsshlex.quote(filename)calls already assumed a raw filename; this was previously being fed an already-quoted string, causing the double-quoting bug. No code change needed there, it now works as originally intended.Tests
IPython/core/tests/test_hooks.py: added tests for the defaulteditor()hook verifying filenames with spaces are quoted in the resulting command, filenames without spaces are left untouched, the editor binary path quoting behavior is preserved, and the full command (with a line number) is assembled correctly.IPython/core/tests/test_magics_code.py: fixedtest_edit_filename_with_space_is_quoted(renamed totest_edit_filename_with_space_passed_raw), which previously asserted the quoted form was received and stripped quotes in theEditorStubfixture — masking the real bug. It now asserts%editpasses the hook a fully raw filename with no surrounding quote characters. Removed the now-unneededfilename.strip("'")inEditorStuband the@skip_win32marker, since the test no longer depends on shell-quoting semantics.Compatibility note
This changes the editor hook contract so that
hooks.editor()now receives a raw filename instead of a pre-quoted one.Manual verification
Verified
%editon a path containing spaces opens correctly via the default hook and via a custom hook (Notepad++), with no stray quote characters and no "file not found" errors.