ipython / ipython/ipython

%edit editorhook fails if path has spaces

Ouverte
#14,961 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
Langage dominant
Python
Étoiles
16.8k
Forks
4.5k
Merge moyen
1 j 2 h
PR mergées (30 j)
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

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.