%edit editorhook fails if path has spaces
- Dominant language
- Python
- Stars
- 16.8k
- Forks
- 4.5k
- Avg merge
- 1d 2h
- Merged PRs (30d)
- 6
Description
Notepad++ editorhook doesn't work for %edit if you try to edit an object from a file whose path has spaces.
----
## Diagnosis
The `%edit` magic single-quotes the path if it has spaces, before passing to the editor hook:
https://github.com/ipython/ipython/blob/cbac235320fe91056a0fb6dba74cd8bef7fc21d9/IPython/core/magics/code.py#L723-L725
Then the default editor hook expects a safe filename, and joins it into a string to use in a Popen call:
https://github.com/ipython/ipython/blob/cbac235320fe91056a0fb6dba74cd8bef7fc21d9/IPython/core/hooks.py#L76-L77
(Notice that it DOES quote the editor path, conditionally, so that is an inconsistency in how it treats the two paths: https://github.com/ipython/ipython/blob/cbac235320fe91056a0fb6dba74cd8bef7fc21d9/IPython/core/hooks.py#L71-L73)
However, the editor hooker makes an editor hook which expects the filename to be an unescaped string, and then uses `shlex.quote` to substitute it into a template and do the call:
https://github.com/ipython/ipython/blob/cbac235320fe91056a0fb6dba74cd8bef7fc21d9/IPython/lib/editorhooks.py#L49
----
## Solution
In my opinion, the responsibility of escaping should be moved down to the callee, because taking a quoted filepath is weird.
This will break people's custom hooks which correct for this bug.
- But there is already a test which calls a custom editor hook with unquoted spaces:
https://github.com/ipython/ipython/blob/cbac235320fe91056a0fb6dba74cd8bef7fc21d9/tests/test_editorhooks.py#L25
- And `_edit_macro` also calls the editor hook without quoting:
https://github.com/ipython/ipython/blob/cbac235320fe91056a0fb6dba74cd8bef7fc21d9/IPython/core/magics/code.py#L539-L540
I suspect that breaks if the temp folder is on a path with spaces.
The escaping is also not the best because it doesn't check for existing special characters. It should just use a safer call, if available.
Also, there should be more consistency between how the default and the custom process calls are done.
----
Also, %edit takes the absolute path, which might lengthen error messages unnecessarily.
https://github.com/ipython/ipython/blob/cbac235320fe91056a0fb6dba74cd8bef7fc21d9/IPython/core/magics/code.py#L722
Contributor guide
Assessment
This issue has not been assessed yet.