[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#49238] [PATCH v1 2/2] gnu: Add ytfzf.
From: |
Maxime Devos |
Subject: |
[bug#49238] [PATCH v1 2/2] gnu: Add ytfzf. |
Date: |
Wed, 30 Jun 2021 21:38:15 +0200 |
User-agent: |
Evolution 3.34.2 |
Raghav Gururajan via Guix-patches via schreef op zo 27-06-2021 om 01:47 [-0400]:
> + (patches
> + (search-patches
> + ;; Prerequisite for 'patch phase.
> + "ytfzf-programs.patch"
That's a neat trick to avoid fragile (substitute* ...).
However, as I understand it, the origin is meant to be
usable as source code (think install dependencies && guix build && tar xzf
stuff.tgz
&& ./configure && make && try it out), such that
"guix build --source=transitive stuff" would give all the
source code for building stuff (in the sense of ‘Corresponding Source’
of the GPL), and "guix-mpv", 'guix-jq" ... don't exist anywhere.
I don't know if that has been spelled out somewhere though.
I'd suggest adding "patch" to 'native-inputs', adding the patch
to 'inputs' or 'native-inputs' (doesn't really matter which) and
doing (invoke "patch" OPTIONS "blabla.patch") before the substitute*.
Actually, myself I'm not convinced because you could consider
the package definition itself to be part of the ‘corresponding source’.
> + (substitute* "ytfzf"
> + (("guix-catimg")
> + (format #f "~a/bin/catimg"
> + (assoc-ref inputs "catimg")))
I'm wondering if (string-append (assoc-ref inputs "catimg") "/bin/catimg")
would be better, as 'string-append' is less complex than 'format'
and "format" doesn't seem to provide any additional value here.
Now about the patch:
> + ############################
> + # Help Texts #
> +@@ -326,8 +326,8 @@ print_info () {
> + }
> +
> + print_error () {
> +- [ $ext_menu_notifs -eq 1 ] && notify-send "error" "$*" || printf
> "\033[31m$*\033[0m" >&2
> +- [ $ext_menu_notifs -eq 1 ] && notify-send "Check for new versions and
> report at: https://github.com/pystardust/ytfzf\n" || printf "Check for new
> versions and report at: https://github.com/pystardust/ytfzf\n" >&2
> ++ [ $ext_menu_notifs -eq 1 ] && guix-notify-send "error" "$*" || printf
> "\033[31m$*\033[0m" >&2
> ++ [ $ext_menu_notifs -eq 1 ] && guix-notify-send "Check for new versions
> and report at: https://github.com/pystardust/ytfzf\n" || printf "Check for
> new versions and report at: https://github.com/pystardust/ytfzf\n" >&2
Maybe tell people to report issues at bug-guix@gnu.org or #guix? Dunno
> + }
> +
> + ############################
> +@@ -398,8 +398,8 @@ format_fzf () {
> + format_menu () {
> + if [ "$is_ext_menu" -eq 0 ]; then
> + #dep_ck fzf here because it is only necessary to use here
> +- dep_ck "fzf"
> +- menu_command='column -t -s "$tab_space" | fzf -m --bind
> change:top --tabstop=1 --layout=reverse --delimiter="$tab_space" --nth=1,2
> --expect="$shortcuts" $FZF_DEFAULT_OPTS'
> ++ dep_ck "guix-fzf"
> ++ menu_command='column -t -s "$tab_space" | guix-fzf -m --bind
> change:top --tabstop=1 --layout=reverse --delimiter="$tab_space" --nth=1,2
> --expect="$shortcuts" $FZF_DEFAULT_OPTS'
> + format_fzf
Don't forget to patch 'column'.
> + else
> + # Dmenu doesn't render tabs so removing it
> +@@ -462,7 +462,7 @@ WIDTH=$FZF_PREVIEW_COLUMNS
> + HEIGHT=$FZF_PREVIEW_LINES
> + start_ueberzug () {
> + [ -e $FIFO ] || { mkfifo "$FIFO" || exit 1 ; }
> +- ueberzug layer --parser json --silent < "$FIFO" &
> ++ guix-ueberzug layer --parser json --silent < "$FIFO" &
> + exec 3>"$FIFO"
> + }
> + stop_ueberzug () {
Don't forget to patch "mkfifo".
> +@@ -585,17 +585,17 @@ download_thumbnails () {
> + if [ "$thumbnail_quality" -eq 1 ]; then
> + image_download () {
> + # higher quality images
> +- curl -s "$Url" -G --data-urlencode "sqp=" >
> "$thumb_dir/$Name.png"
> ++ guix-curl -s "$Url" -G --data-urlencode "sqp=" >
> "$thumb_dir/$Name.png"
> + }
> + else
> + image_download () {
> +- curl -s "$Url" > "$thumb_dir/$Name.png"
> ++ guix-curl -s "$Url" > "$thumb_dir/$Name.png"
> + }
> + fi
> +
> + print_info "Downloading Thumbnails...\n"
> + thumb_urls=$(printf "%s" "$*" |\
> +- jq -r '.[]|[.thumbs,.videoID]|@tsv' )
> ++ guix-jq -r '.[]|[.thumbs,.videoID]|@tsv' )
> +
> + while IFS=$tab_space read -r Url Name; do
> + sleep 0.001
>
Don't forget patching 'sleep'. It is not shell a built-in
(try "type sleep" and "type [" in a terminal").
> +@@ -988,7 +988,7 @@ format_user_selection () {
> + 11)
> selected_urls=$selected_urls$new_line'https://www.youtube.com/watch?v='$surl
> ;;
> + 34)
> selected_urls=$selected_urls$new_line'https://www.youtube.com/playlist?list='$surl
> ;;
> + 36)
> +- selected_urls=$selected_urls$new_line"$(printf "%s"
> "$videos_json" | jq '.[].url' | grep -F "$surl" | tr -d '"')" ;;
> ++ selected_urls=$selected_urls$new_line"$(printf "%s"
> "$videos_json" | guix-jq '.[].url' | grep -F "$surl" | tr -d '"')
Don't forget patching 'grep' and 'tr'.
> " ;;
> + *) continue ;;
> + esac
> + refined_selected_data=$refined_selected_data$new_line$(printf
> '%s' "$videos_data" | grep "|$surl" )
> +@@ -1014,7 +1014,7 @@ print_data () {
> + get_video_format () {
> + # select format if flag given
> + [ $show_format -eq 0 ] && return
> +- formats=$(youtube-dl -F "$(printf "$selected_urls")")
> ++ formats=$(guix-youtube-dl -F "$(printf "$selected_urls")")
> + line_number=$(printf "$formats" | grep -n '.*extension
> resolution.*' | cut -d: -f1)
Don't forget 'grep -> guix-grep' and 'cut -> guix-cut'
> + quality=$(printf "$formats \n1 2 xAudio" | awk -v
> lineno=$line_number 'FNR > lineno {print $3}' | sort -n | awk -F"x" '{print
> $2 "p"}' | uniq | sed -e "s/Audiop/Audio/" -e "/^p$/d" | eval "$menu_command"
> | sed "s/p//g")
Don't forget 'awk -> guix-awk' and 'sort -> guix-sort' and 'uniq -> 'guix-uniq'
> + [ -z "$quality" ] && exit;
> +@@ -1026,9 +1026,9 @@ get_video_format () {
> + get_sub_lang () {
> + if [ $auto_caption -eq 1 ]; then
> + #Gets the auto generated subs and stores them in a file
> +- sub_list=$(youtube-dl --list-subs --write-auto-sub
> "$selected_urls" | sed '/Available subtitles/,$d' | awk '{print $1}' | sed
> '1d;2d;3d')
> ++ sub_list=$(guix-youtube-dl --list-subs --write-auto-sub
> "$selected_urls" | sed '/Available subtitles/,$d' | awk '{print $1}' | sed
> '1d;2d;3d')
Don't forget 'sed -> guix-sed' and 'awk -> guix-awk'
> + if [ -n "$sub_list" ]; then
> +- [ -n "$selected_sub" ] || selected_sub=$(printf "$sub_list" |
> eval "$menu_command") && youtube-dl --sub-lang $selected_sub
> --write-auto-sub --skip-download "$selected_urls" -o /tmp/ytfzf &&
> YTFZF_SUBT_NAME="--sub-file=/tmp/ytfzf.$selected_sub.vtt" || printf "Auto
> generated subs not available."
> ++ [ -n "$selected_sub" ] || selected_sub=$(printf "$sub_list" |
> eval "$menu_command") && guix-youtube-dl --sub-lang $selected_sub
> --write-auto-sub --skip-download "$selected_urls" -o /tmp/ytfzf &&
> YTFZF_SUBT_NAME="--sub-file=/tmp/ytfzf.$selected_sub.vtt" || printf "Auto
> generated subs not available."
FWIW, writing to "/tmp/ytfzf" seems to be a potential security problem
and bad behaviour on multiple-user systems.
What would happen if /tmp/ytfzf is a symlink to /etc/passwd an ytfzf is run
as root (yes, running as root is not recommended)? Would that brick the system?
What if /tmp/ytfzf is a symlink to ~/.profile? Would that brick the login?
What if multiple users run ytfzf concurrently? Would they overwrite eaech
other subtitles? Would a different user be able to see what the other
is downloading?
A relatively easy fix would be to write to, say, $HOME/.cache/ytzf-subs
instead (not sure what the proper directory would be), which is completely
under the user's control.
> +@@ -1262,7 +1262,7 @@ EOF
> + update_ytfzf () {
> + branch="$1"
> + updatefile="/tmp/ytfzf-update"
> +- curl -L
> "https://raw.githubusercontent.com/pystardust/ytfzf/$branch/ytfzf" -o
> "$updatefile"
> ++ guix-curl -L
> "https://raw.githubusercontent.com/pystardust/ytfzf/$branch/ytfzf" -o
> "$updatefile"
> +
> + if sed -n '1p' < "$updatefile" | grep -q '#!/bin/sh'; then
FWIW, as update-ytfzf will be more-or-less deleted
in the next patch, don't forget to patch 'sed' and 'grep'
> + chmod 755 "$updatefile"
> +@@ -1346,10 +1346,10 @@ create_subs () {
> + : > "$config_dir/subscriptions"
> +
> + # check how many subscriptions there are in the file
> +- sublength=$( jq '. | length' < "$yt_sub_import_file" )
> ++ sublength=$( guix-jq '. | length' < "$yt_sub_import_file" )
> +
> + for i in $(seq $((sublength - 1))); do
> +- channelInfo=$(jq --argjson index ${i} '[
> "https://www.youtube.com/channel/" + .[$index].snippet.resourceId.channelId +
> "/videos", "#" + .[$index].snippet.title ]' < "$yt_sub_import_file")
> ++ channelInfo=$(guix-jq --argjson index ${i} '[
> "https://www.youtube.com/channel/" + .[$index].snippet.resourceId.channelId +
> "/videos", "#" + .[$index].snippet.title ]' < "$yt_sub_import_file")
> + printf "%s\n" "$(printf "%s" "$channelInfo" | tr -d '[]"\n,')" >>
> "$subscriptions_file"
And don't forget 'tr'.
Greetings,
Maxime.
signature.asc
Description: This is a digitally signed message part