Mediatypes support

[sudobash1]

This is a partial solution to #121. It uses a slightly different configuration syntax than the one proposed. It looks like this:

[mime-handlers]
# Allows setting the commands to run for various mime types.
# E.g. to open png files with the imagemagic display command:
#   [mime-handlers."image/png"]
#   command = ["display"]
#
# The subtype may be a * to open all files of a set type E.g:
#   [mime-handlers."image/*"]
#   command = ["display"]
#
# Both the type and subtype may be */* to specify the default handler.
#
# If command is "" or [] the file will be opened using the mozz.us http portal
# and a-general.http
#
# Mime type handlers may also have an action field containing one of the following:
# "prompt"   - Ask user every time which action to take.
# "download" - Always download the file without prompting
# "open"     - Always open the file using the below command without prompting
[mime-handlers."*/*"]
command = []
action = "prompt"

[mime-handlers."image/*"]
command = "display"
action = "open"

It leaves the mozz.us portal as the default, but it is trivial for a Linux user to add a */* handler for xdg-open. On mac I think plain open works. I don't know about Windows though.

Limitations

There are a handful of limitations with this implementation that I want to point out:

• Currently there is no piping/streaming. Files must be downloaded in full before opening
• Files are downloaded to the system temp dir (but this is configurable). They are never removed by amfora. I am relying on the system to clean the tmp folder on reboot.
• MIME parameters are ignored.
• Opening files in a TUI application (like vim/emacs/viu) is a bit tricky. A user would probably need to write a wrapper script

[makew0rld]

Thanks for making this PR. This was actually something I wanted to work on personally, which I indicated by assigning myself in #121. Next time feel free to ask and I'll let you know. But I still appreciate the work here, and thanks for your interest in Amfora!

I'll outline some issues I see with the way you've added this below, you're welcome to either make the changes if you agree, or close the PR and I will work on it at some point later.

Config

• Your new format for configuring mediatypes doesn't seem to allow for specifying multiple mediatypes for one command. A user would have to create multiple sections instead, which seems verbose and repetitive
• I like the idea of being able to disable the prompt modal and just always open the file, but I don't see the value in always initiating a download, especially since there's currently no way to stop one (see Way to cancel downloading #129)
  • Maybe this could be replaced with an optional no_prompt variable that defaults to false
• If command is "" or [] the file will be opened using the mozz.us http portal and a-general.http
  • I don't really see the point of this. Why not just not have the section at all? And the portal usage should be removed entirely, as I mentioned in Mediatype handlers #121

It leaves the mozz.us portal as the default, but it is trivial for a Linux user to add a */* handler for xdg-open. On mac I think plain open works. I don't know about Windows though.

It's important to me that those commands are the default, built-in to the application. So by default when accessing a file the user gets three options (Open, Download, Cancel), and clicking "Open" will use one of these default handlers. On Windows you can use exec.Command("rundll32", "url.dll,FileProtocolHandler", file), this is used in the webbrowser/ code as a reference.

Note that opening in the portal was not one of the options, I'd like to remove that dependency.

Limitations

Currently there is no piping/streaming. Files must be downloaded in full before opening

This is something I need to have before releasing and before closing #121, but it's fine if it's not in this PR, happy to add it myself.

Files are downloaded to the system temp dir (but this is configurable). They are never removed by amfora. I am relying on the system to clean the tmp folder on reboot.

That's fine with me, I believe it's what Firefox does as well.

MIME parameters are ignored.

Also fine.

Opening files in a TUI application (like vim/emacs/viu) is a bit tricky. A user would probably need to write a wrapper script

I was thinking about this as well. Unfortunately there doesn't seem to be a good way around this, I'm not interested in nesting a terminal inside Amfora as all the implementations of that I've see don't work very well (slow, no truecolor support, etc).

Code

There are some things in the code I could go over and do a full review, but I'd rather hear your response to this and what you think, as well as see the above changes made, before doing that.

Thanks again for your work!

[sudobash1]

Thanks for the reply. I completely understand wanting to work on this yourself. I initially was just going to kludge something together for myself since I was really wanting to get around using the portal, but I figured it couldn't hurt to clean it a bit and post what I had. Bad form not introducing myself.

Your reply on the config makes sense to me. I'll make changes to this PR. But I'll not be offended in the least if you want to do your own implementation, or rework things.

With regards to the terminal apps, a solution I am using myself is:

#!/bin/sh
if [ -n "$TMUX" ]; then
  tmux new-window sh -c 'viu "$@"; read -p "[Press Enter]"' sh "$@"
else
  display "$@"
fi

Setting this script as the handler will allow you to stay in the terminal if you are using tmux otherwise it falls back to opening a new window.

Cheers 🍻

[makew0rld]

I initially was just going to kludge something together for myself since I was really wanting to get around using the portal, but I figured it couldn't hurt to clean it a bit and post what I had.

This is how open source is built. Thank you!

Your reply on the config makes sense to me. I'll make changes to this PR. But I'll not be offended in the least if you want to do your own implementation, or rework things.

Happy to hear it, thanks for understanding. Once you make the changes let me know here, and I'll decide what I want to do. Since you're open to my ideas I suspect I'll probably just end up merging your work.

With regards to the terminal apps, a solution I am using myself is [snip]

What is that display command? Are you on macOS and it's some Apple thing? On my Linux machine it's a command for displaying images. Using tmux seems like a cool way to do it though. Thanks for sharing!

---

EDIT: Something I forgot mention in my original reply -- please use "mediatype" everywhere instead of "mimetype", it's the newer and more accurate term.

[sudobash1]

I've pushed an update which uses the configuration you specified. I looked into viper and found its unmarshalling ability made things cleaner.

What is that display command? Are you on macOS and it's some Apple thing? On my Linux machine it's a command for displaying images. Using tmux seems like a cool way to do it though. Thanks for sharing!

The display command is what you have on Linux (I'm on Linux too). That script is for opening an image. viu is a program to render an image using utf-8 characters.

EDIT: Something I forgot mention in my original reply -- please use "mediatype" everywhere instead of "mimetype", it's the newer and more accurate term.

Noted and done. Thanks!
EDIT: Github will not let me change the PR branch, so that name is stuck ☹️

There are a few code smell issues:

Location of MediaHandler struct

I wasn't sure where the MediaHandler struct should go. Currently it is config/config.go but perhaps it should go in structs/structs.go?

(Ab)using the webbrowser package

Instead of copying from the webbrowser package, I simply used it directly. I think that functionally this is OK, but it seems like an abuse of the webbrowser package in that a) I am not using it for a web browser per se and b) I am discarding the msg it returns, and substituting it for one which doesn't mention "browser". The *nix one is using $BROWSER as a fallback, but I don't see that as a particular problem. At least with Firefox or Chrome, if you give it a random file on the command line, it will try to display it, or offer to open/download it.

If it were me, I would be tempted to make the webbrowser package into a general "system open" utility, but I don't want to run off and do my own thing drastically changing stuff (again 😉). And I may have an overdeveloped aversion to code duplication...

Old commits left in place.

The commits should be squashed. I'm leaving things as-is for the PR review.

[makew0rld]

Thanks for the changes! Please see my comments below, in this message and the specific code comments.

Location of MediaHandler struct

I'm fine with it being in config/config.go, it seems directly related.

(Ab)using the webbrowser package

I'm not really okay with this. I think this (mis)usage can result in strange errors, as well as incorrect logic. I'm not in favour of the accidental $BROWSER fallback you mention, or that files won't open on *nix systems if there's no Xorg or Wayland display server.

I would be tempted to make the webbrowser package into a general "system open" utility

I would prefer something like this. But leave the existing webbrowser package in place, as it already has some specific code that I'd rather not shift elsewhere. It should be simple enough to copy webbrowser/ into a new folder and just remove the stuff that doesn't apply, and change the package name.

The commits should be squashed. I'm leaving things as-is for the PR review.

Thank you, it's helpful to have to the full history. I will squash the commits for merging.

[sudobash1]

(Ab)using the webbrowser package

I'm not really okay with this. I think this (mis)usage can result in strange errors, as well as incorrect logic. I'm not in favour of the > accidental $BROWSER fallback you mention

Makes sense.

or that files won't open on *nix systems if there's no Xorg or Wayland display server.

I think that if a user does not have a display server running, then there is not going to be a way to determine the system default viewer. The xdg-open man page says:

xdg-open is for use inside a desktop session only.

I think the best course of action for when there is no display server is to display an error dialog instructing the user to add their own default [[mediatype-handlers]].

[makew0rld]

I think that if a user does not have a display server running, then there is not going to be a way to determine the system default viewer.

Aha you're totally right, lol. I still think this should be a separate package because of the $BROWSER fallback, but otherwise it's basically a copy of webbrowser/. I should probably combine them...

[makew0rld]

Just some small changes, I think this is almost ready to go! Thanks for this.

I don't want this in the upcoming v1.7.0 release though, so I might leave this PR open, or merge it into a different branch. It should be in v1.8.0.

[sudobash1]

OK. The CI checks have passed, and I think I got all of your requests in. 🍻

[makew0rld]

Alright this looks done, thank you! I'll merge it into the new mediatype-handlers branch, and then it should be in the v1.8.0 release. Thanks for your work.