-
Notifications
You must be signed in to change notification settings - Fork 164
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
commands/thread/open-attachment: add command #1505
base: master
Are you sure you want to change the base?
Conversation
Completing the already existing on_success for callbacks that should be executed even after a non-zero exit code.
1979cd9
to
9a75424
Compare
I'd rather not call this command |
9a75424
to
7af6934
Compare
Amended to rename the command to "open-attachment". |
7af6934
to
13186af
Compare
raise RuntimeError('not focused on an attachment') | ||
|
||
@staticmethod | ||
def _make_temp_file(attachment, prefix='', suffix=''): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems overcomplicated to me. Why not just extend tempfile.NamedTemporaryFile
and return a contextmanager
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ExternalCommand in charge of executing the command using the temporary file does not accept context managers. It's not either possible to use a "with" block on top of it because it can spawn an independent thread.
I feel like I have implemented a very similat logic in |
This allows the user to open an attachment file with a program of their choice. GitHub: closes pazz#1494
13186af
to
ebbb23c
Compare
The EditCommand and OpenAttachmentCommand implementations do not seem to share much in common: the way the file path is substituted in the command differs with mailcap templates. |
This allows the user to open an attachment file with a program of their choice.
GitHub: closes #1494