Closed forthrin closed 6 years ago
This is way way out of scope. Just write a custom wrapper script as your BROWSER.
Agreed.
Before I consider discussing "out of scope": How do you enable the user to tell the wrapper script to open the link with youtube-dl
or your regular browser? (Or another handler for that matter.)
How do you enable the user to tell the wrapper script to open the link with youtube-dl or your regular browser?
You show a prompt when you detect a YouTube URL.
A wrapper script could work, but I don't find the request very out of scope. It would provide quite related convenience for the user. Pseudo-code:
if regex("y (\d+)", user_input, matches)
shell_exec("youtube-dl <user options> " + url_list[matches[1]])
The problem here is twofold. First, providing support for youtube-dl is too specific. I can argue the same for mpv support where you play the video with mpv --ytdl
. Secondly, the devil is in the details — do we run the external command blocking or non-blocking? In the latter case, how do we deal with logging? Etc. Handling those details either require decisions that won't please everyone, or additional options and code paths outside googler's core competency, which is a burden.
You could argue for a generic launcher command (there's one already, mind you, which is o
, with a customizable BROWSER
, but let's just say we add another one) with an option/env var for a thing to execvp. And let's just say we managed to nail the details already. Even then, there are probably more than two actions you may want to take with a URL: open, download, play, bookmark, DDoS, etc. To deal with more than two actions, you still need to have a prompt in the generic launcher; that's not much better than just one command o
with a BROWSER
.
Now, a real solution would be a plugins/extensions architecture where you can register your own commands. However, first, that's kind of awkward when googler's stated policy on configuration files is none; secondly, the project is basically in feature freeze without outside contributions, so someone needs to take the initiative.
With all that said, help yourself with this patch if you just want the feature:
diff --git a/googler b/googler
index 8ddbf59..9daa743 100755
--- a/googler
+++ b/googler
@@ -1993,6 +1993,12 @@ class GooglerCmd(object):
else:
printerr('Invalid index %s.' % nav)
+ @require_keywords
+ def do_ytdl(self, *args):
+ for nav in args:
+ if nav in self._urltable:
+ subprocess.run(['youtube-dl', self._urltable[nav]])
+
@require_keywords
@no_argument
def do_previous(self):
@@ -2050,6 +2056,8 @@ class GooglerCmd(object):
open_url.override_text_browser = True
self.do_open(*cmd[2:].split())
open_url.override_text_browser = False
+ elif cmd.startswith('y '):
+ self.do_ytdl(*cmd[2:].split())
elif cmd == 'p':
self.do_previous('')
elif cmd == 'q':
That's beautiful! I understand your concerns. My suggestions would be:
youtube-dl
options.Whether and how to implement a good generic system would be the result of a discussion between developers and users. Personally, I just wanted the video support. For now :) Which I now have :)
I'd like
googler
to download video links withyoutube-dl <options>
. Proposed solution: