search

makew0rld / amfora

A fancy terminal browser for the Gemini protocol.
GNU General Public License v3.0
1.14k stars 65 forks source link

Adds support for custom commands #265

Open mntn-xyz opened 2 years ago

mntn-xyz commented 2 years ago

For #262

Supports up to 10 custom commands, defined in the configuration file. The current or selected URL can be passed to the command by placing ${url} in the command string.

Default hotkeys are Alt-1 through Alt-0 for triggering the command with the selected link URL as a parameter, or Alt-Shift-1 through Alt-Shift-0 for triggering the command with the current page URL as a parameter. This follows the convention set by the copy page URL (C) and copy selected URL (c) defaults.

Custom commands must be available in the $PATH or use absolute paths. Relative paths are not supported, neither are pipes or shell redirections.

makew0rld commented 2 years ago

Thanks for your PR. I'll try and take a look at this when I have time, but it's not a priority for v1.9.0.

Also note there's now a conflict, let me know if you need any help resolving that.

mntn-xyz commented 2 years ago

Thanks, I had not seen the conflict. It is resolved now. Let me know if there's any improvements I need to make when you get a chance to review it.

mntn-xyz commented 2 years ago

One of the changes has broken this, need to figure out what happened.

makew0rld commented 2 years ago

Ok, let me know if you need any help or have questions.

This is a cool feature for sure, but not top of my list, so I'm not sure if it'll make it into the v1.10.0 release. It's gonna be a while before I get there though, so who knows :) Thanks for making contributions to Amfora!

mntn-xyz commented 2 years ago

The issue I was encountering seems to be a recently introduced bug, see #281. Once that is fixed, this should be ready.

mntn-xyz commented 2 years ago

Todo: use goroutines to prevent deadlock per #281

makew0rld commented 2 years ago

Please see changes made in #284 (once merged) for a guide on how to fix issues like #281.

makew0rld commented 2 years ago

@mntn-xyz #284 has been merged and #281 has been closed, so feel free to continue working on this whenever you can. You'll have to merge master back into this PR to get those updates.

mntn-xyz commented 2 years ago

Merged up to master, and I made the calls to Error and Info use goroutines as in #284. I think the risk of a race condition here with other parts of the UI is minimal, as it looks like it shouldn't be possible to invoke one of these commands during other operations that also pops up a dialog. The commands are forked when run so long-running commands also won't be an issue. I did some light testing and couldn't intentionally break it.

makew0rld commented 2 years ago

Sorry I haven't had a lot of time to review this PR. I'm going to add it to the milestone so that I won't forget to come back to it when I make time for Amfora in the future.