Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not fail if $EDITOR contains spaces #1

Closed
wants to merge 1 commit into from
Closed

Conversation

yajo
Copy link
Contributor

@yajo yajo commented Sep 18, 2020

A common usage is code --wait to use VSCode as editor.

Before this patch, the library would fail with:

>>> import editor
>>> editor.edit('hello')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/var/home/yajo/prodevel/copier/.venv/lib64/python3.8/site-packages/editor.py", line 101, in edit
    proc = subprocess.Popen(args, close_fds=True, stdout=stdout)
  File "/usr/lib64/python3.8/subprocess.py", line 854, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib64/python3.8/subprocess.py", line 1702, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'code --wait'
>>> editor.__version__
'1.0.4'

A common usage is `code --wait` to use VSCode as editor.

Before this patch, the library would fail with:

```python
>>> import editor
>>> editor.edit('hello')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/var/home/yajo/prodevel/copier/.venv/lib64/python3.8/site-packages/editor.py", line 101, in edit
    proc = subprocess.Popen(args, close_fds=True, stdout=stdout)
  File "/usr/lib64/python3.8/subprocess.py", line 854, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib64/python3.8/subprocess.py", line 1702, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'code --wait'
>>> editor.__version__
'1.0.4'
```
@yajo yajo mentioned this pull request Sep 18, 2020
6 tasks
@rec
Copy link
Owner

rec commented Sep 18, 2020

Hello! Thanks so much for doing this, and DOH, what a dumb bug of mine.

I'd pull this but it seems to break the tests. I'm about to go and eat (I'm in Europe) and might not be back tonight on this unless you were in a desperate hurry.

If you can get the tests working, I'll pull it tonight or tomorrow AM or otherwise, I'll patch in this branch and tweak it first thing.

Thanks again for keeping me honest! :-D

@rec
Copy link
Owner

rec commented Sep 19, 2020

¿Hola señor, como esta? Comenzando con esta tarea.

I can reproduce the unit test breakage here, so I'll see how to tweak it to get that to go away.

@rec
Copy link
Owner

rec commented Sep 19, 2020

All right! I tweaked this and pushed a fixup patch here: https://github.com/rec/editor/commits/patch-1

I'm using shlex to split the strings because it gives better results on actual command lines!

If you like this, I'll rebase the fixup into your commit (so we get joint authorship) and push it.

Or you can patch it into your repo, etc

On to the next!

@yajo
Copy link
Contributor Author

yajo commented Sep 19, 2020

All you say seems good to me 😉

¡Gracias amigo!

@rec
Copy link
Owner

rec commented Sep 20, 2020

(Closing this in favor of #5)

@rec rec closed this Sep 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants