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

✨Add option to open unknown URLs #74

Merged
merged 1 commit into from Aug 27, 2020
Merged

✨Add option to open unknown URLs #74

merged 1 commit into from Aug 27, 2020

Conversation

sotpapathe
Copy link
Contributor

A new config option that sets a command to open unsupported URL schemes (e.g. gopher://). It is unset by default to keep the previous behavior.

@makew0rld
Copy link
Owner

Thanks for this PR, it looks well done. My issue is that an unknown URL could be anything, and so having just one command for it isn't really portable. It could be any number of schemes: gopher://, ipfs://, magnet:, etc. Perhaps using xdg-open is the solution? I'm not sure, as that's often harder to configure.

@sotpapathe
Copy link
Contributor Author

It allows the user to at least specify another program that will handle unknown schemes, like xdg-open or pso.

I guess the better approach would be to have a dictionary of scheme-program in the configuration, although I don't know how to implement that in go yet.

@sotpapathe
Copy link
Contributor Author

OK, I've got a better suggestion. The config file will look like this:

[url_handlers]
http = "xdg-open"
gopher = "off"
ftp = "off"
# and any other URL schemes the user desires.
other = "xdg-open"

Then in handleURL(), if the scheme is not gemini, we'll do a viper.GetString("url_handlers." + scheme). If a value is found, it's used as the program to open the URL with. Otherwise the value in "url_handlers.other" is used as the program to open the URL with.

If this sounds good I think I can implement it in the next days.

@makew0rld
Copy link
Owner

That sounds great! Good idea. I think we can leave the HTTP setting in a-general for compatibility though.

@makew0rld
Copy link
Owner

Make sure to update default-config.toml as well as default.go. I tend to edit the TOML first, then run go generate ./....

@sotpapathe
Copy link
Contributor Author

Thanks for the tip, will do!

@sotpapathe
Copy link
Contributor Author

Updated as discussed and tested with Gopher URLs. I didn't know of any pages with other kinds of URLs but it should work for all schemes.

Copy link
Owner

@makew0rld makew0rld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great feature, thanks! See my comments below, I will merge soon.

default-config.toml Show resolved Hide resolved
display/private.go Outdated Show resolved Hide resolved
@makew0rld
Copy link
Owner

Looks good now, thanks. I will merge soon, after my current linting fixes, like with the other PR. Also I'd appreciate if you didn't force-push for any future PRs, as it makes it impossible to see the diffs as you update things.

Thanks for your interest in Amfora!

@sotpapathe
Copy link
Contributor Author

Sorry for the force pushes, won't do it again 😃

And thanks for making Amfora!

@makew0rld makew0rld merged commit 6c3583b into makew0rld:master Aug 27, 2020
@sotpapathe sotpapathe deleted the handle-unknown-urls branch August 31, 2020 18:52
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.

None yet

2 participants