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

Annotation probably fails on Windows due to no $EDITOR #98

Closed
robintw opened this Issue Jan 1, 2016 · 4 comments

Comments

Projects
None yet
2 participants
@robintw
Contributor

robintw commented Jan 1, 2016

This code will most probably fail on Windows as there is no $EDITOR (not your PR, the annotate function as it is).
To make it more robust, on Windows you could use os.startfile to open the file in question with the default application.

(From comment on #97 by @bilderbuchi)

@robintw robintw added the bug label Jan 1, 2016

@cash

This comment has been minimized.

Show comment
Hide comment
@cash

cash Jan 6, 2016

Contributor

Just tried this on Windows and recipy annotate does not work because Windows has a different format for environments variables (uses %EDITOR% format).

Changing to:

os.system('%s %s' % (os.environ.get('EDITOR'), f.name))

did work for me as long as I set the environment variable first as it is not a standard Windows variable:

set EDITOR=notepad.exe
Contributor

cash commented Jan 6, 2016

Just tried this on Windows and recipy annotate does not work because Windows has a different format for environments variables (uses %EDITOR% format).

Changing to:

os.system('%s %s' % (os.environ.get('EDITOR'), f.name))

did work for me as long as I set the environment variable first as it is not a standard Windows variable:

set EDITOR=notepad.exe
@robintw

This comment has been minimized.

Show comment
Hide comment
@robintw

robintw Jan 6, 2016

Contributor

Thanks. I've done a bit of playing too, and I think something like this will work:

editor = os.getenv('EDITOR')
if editor:
    os.system('%s %s' % (editor, f.name))
else:
    touch(f.name)
    os.system(f.name)

So, it will try and get the EDITOR variable (which should be set on most Linux/OS X systems) - and if it can't, it will 'touch' the file (like the Unix touch command, but in pure Python) and then execute the file which, on Windows, will open the file in Notepad.

For reference, the touch function is defined at http://stackoverflow.com/questions/1158076/implement-touch-using-python

Contributor

robintw commented Jan 6, 2016

Thanks. I've done a bit of playing too, and I think something like this will work:

editor = os.getenv('EDITOR')
if editor:
    os.system('%s %s' % (editor, f.name))
else:
    touch(f.name)
    os.system(f.name)

So, it will try and get the EDITOR variable (which should be set on most Linux/OS X systems) - and if it can't, it will 'touch' the file (like the Unix touch command, but in pure Python) and then execute the file which, on Windows, will open the file in Notepad.

For reference, the touch function is defined at http://stackoverflow.com/questions/1158076/implement-touch-using-python

@cash

This comment has been minimized.

Show comment
Hide comment
@cash

cash Jan 6, 2016

Contributor

I assume you're also adding a .txt extension to the temp file to get that last system call to work.

Contributor

cash commented Jan 6, 2016

I assume you're also adding a .txt extension to the temp file to get that last system call to work.

trbedwards pushed a commit to trbedwards/recipy that referenced this issue Sep 19, 2016

Thomas Edwards
#98: Defined a launch_text_editor function which tries different comm…
…ands for different operating systems. work in progress

trbedwards added a commit to trbedwards/recipy that referenced this issue Sep 19, 2016

#98: Refactored find_editor into get_editor from recipyCommon.config.…
… Added unit test for find_editor. Merged with Jan's commit.

trbedwards added a commit to trbedwards/recipy that referenced this issue Sep 19, 2016

@robintw

This comment has been minimized.

Show comment
Hide comment
@robintw

robintw Sep 19, 2016

Contributor

Fixed in #155.

Contributor

robintw commented Sep 19, 2016

Fixed in #155.

@robintw robintw closed this Sep 19, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment