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

commands/thread/open-attachment: add command #1505

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pacien
Copy link
Contributor

@pacien pacien commented May 9, 2020

This allows the user to open an attachment file with a program of their choice.

GitHub: closes #1494

Completing the already existing on_success for callbacks that should be
executed even after a non-zero exit code.
@pacien pacien force-pushed the command-open-attachment branch 7 times, most recently from 1979cd9 to 9a75424 Compare May 9, 2020 16:07
alot/commands/thread.py Outdated Show resolved Hide resolved
@pazz
Copy link
Owner

pazz commented May 9, 2020

I'd rather not call this command open because it may be confused with unfolding a message. How's view ? open-attachment ?

@pacien pacien force-pushed the command-open-attachment branch from 9a75424 to 7af6934 Compare May 9, 2020 16:19
@pacien
Copy link
Contributor Author

pacien commented May 9, 2020

Amended to rename the command to "open-attachment".

@pacien pacien changed the title commands/thread/open: add "open" command to open attachments commands/thread/open: add "open-attachment" command May 9, 2020
@pacien pacien requested a review from pazz May 9, 2020 20:04
@pacien pacien changed the title commands/thread/open: add "open-attachment" command commands/thread/open-attachment: add command May 10, 2020
@pacien pacien force-pushed the command-open-attachment branch from 7af6934 to 13186af Compare May 10, 2020 17:16
alot/commands/thread.py Outdated Show resolved Hide resolved
raise RuntimeError('not focused on an attachment')

@staticmethod
def _make_temp_file(attachment, prefix='', suffix=''):

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?

Copy link
Contributor Author

@pacien pacien May 10, 2020

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.

alot/commands/thread.py Outdated Show resolved Hide resolved
alot/commands/thread.py Outdated Show resolved Hide resolved
alot/commands/thread.py Outdated Show resolved Hide resolved
@pazz
Copy link
Owner

pazz commented May 10, 2020

I feel like I have implemented a very similat logic in alot.commands.globals.EditCommand.
Perhaps we should rather subclass that to recycle code?

This allows the user to open an attachment file with a program of their choice.

GitHub: closes pazz#1494
@pacien pacien force-pushed the command-open-attachment branch from 13186af to ebbb23c Compare May 10, 2020 21:11
@pacien
Copy link
Contributor Author

pacien commented May 10, 2020

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.

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.

Open attachment with a custom command like xdg-open
3 participants