Skip to content

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

Open
pacien wants to merge 2 commits intopazz:masterfrom
pacien:command-open-attachment
Open

commands/thread/open-attachment: add command#1505
pacien wants to merge 2 commits intopazz:masterfrom
pacien:command-open-attachment

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
@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
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.

@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