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

[BUG]: The search-again option breaks ytfzf #613

Closed
pitsi opened this issue Dec 16, 2022 · 18 comments
Closed

[BUG]: The search-again option breaks ytfzf #613

pitsi opened this issue Dec 16, 2022 · 18 comments
Labels
bug Something isn't working

Comments

@pitsi
Copy link

pitsi commented Dec 16, 2022

Describe the bug

The search-again option (either as a command line parameter or as a parameter in the configuration file) breaks ytfzf in a weird way. Once the playback, instead of returning to the results, it just returns to fzf "menu" which waits for input in a menu with zero entries! And the only way to exit it is to completely close the terminal.
Pressing ctrl+c shows what is behind fzf for a split second, and it mentions ~/.cache/ytfzf as far as I can read.

To Reproduce

  1. Start ytfzf with the --search-again parameter.
  2. Search for anything and select something to play when fzf comes up with the results.
  3. Wait for it to finish (or just press q to make mpv quit before it ends).
  4. Notice the emptyness on the screenshot below.

Expected behavior

Ytfzf should return to the list where fzf waits for user input. Fzf works like it should here, it just has nothing to display as a list.

Screenshots

2022-12-16-181444_800x600_scrot

Information

  • OS: Debian testing/unstable x64
  • Terminal: Urxvt and alacritty
  • Ytfzf version: 2.5.3
  • Output of ls -l "$(which sh)" (if you're using fish: ls -l (which sh)):
    lrwxrwxrwx 1 root root 4 Sep 1 17:11 /usr/bin/sh -> dash

Additional context

Pictured above is alacritty, fzf is on v0.35, below is my conf.sh and ytfzf was working fine until 2.5.2.
Last but not least, I want to clean anything that is under the ~/.cache folder, but I am keeping it for now, if it helps troubleshoot the issue.

search_source=hist
search_again=1
video_pref=140
#video_pref=22
#invidious_instance=
notify_playing=1
scrape=yt
@pitsi pitsi added the bug Something isn't working label Dec 16, 2022
Euro20179 added a commit that referenced this issue Dec 16, 2022
@Euro20179
Copy link
Collaborator

This should now be fixed in the development branch.

@pitsi
Copy link
Author

pitsi commented Dec 16, 2022

Thank you! Since debian decided to package it, I no longer use it as another standalone script from my ~/bin folder.
If I patch it manually ( = one straight edit on /usr/bin/ytfzf), do I have to add all the other patches before this one to make it work like it used to?

@Euro20179
Copy link
Collaborator

This patch should work on it's own.

@pitsi
Copy link
Author

pitsi commented Dec 16, 2022

Thank you. I will patch it in ~15 minutes, because I have some work to do right now, and report back.

@pitsi
Copy link
Author

pitsi commented Dec 16, 2022

It did not work :(
My file here does not have the 2898 lines that the one in the commit has, it stops at 2746, so I had to improvise.
I changed these lines, which are 2709 to 2441 from

main() {
	while :; do
		# calls the interface
		run_interface
		handle_keypress "$keypress_file" || case "$?" in 2) break ;; 3) continue ;; esac
		handle_actions "$ytfzf_selected_urls" || { [ "$?" -eq 2 ] || continue && break; }

		[ $enable_hist -eq 1 ] && add_to_hist "$ytfzf_video_json_file" < "$ytfzf_selected_urls"

		# nothing below needs to happen if  this is empty (causes bugs when this is not here)
		[ ! -s "$ytfzf_selected_urls" ] && break

		# shellcheck disable=SC2015
		if [ "$info_to_print" ]; then
		    data="$(get_requested_info "$ytfzf_selected_urls")"
		    display_text_wrapper "$data"
		    handle_info_wait_action
		    case "$?" in 3) continue ;; 2) break ;; esac
		fi
		open_url_handler "$ytfzf_selected_urls"
		close_url_handler "$url_handler"

		[ "$is_loop" -eq 0 ] && break
	done

    # doing this after the loop allows for -l and -s to coexist
    while [ "$search_again" -eq 1 ] ; do
        clean_up
        initial_search=""
        main
    done
}
main

to

main() {
	while :; do
		# calls the interface
		run_interface
		handle_keypress "$keypress_file" || case "$?" in 2) break ;; 3) continue ;; esac
		handle_actions "$ytfzf_selected_urls" || { [ "$?" -eq 2 ] || continue && break; }

		[ $enable_hist -eq 1 ] && add_to_hist "$ytfzf_video_json_file" < "$ytfzf_selected_urls"

		# nothing below needs to happen if  this is empty (causes bugs when this is not here)
		[ ! -s "$ytfzf_selected_urls" ] && break

		# shellcheck disable=SC2015
		if [ "$info_to_print" ]; then
		    data="$(get_requested_info "$ytfzf_selected_urls")"
		    display_text_wrapper "$data"
		    handle_info_wait_action
		    case "$?" in 3) continue ;; 2) break ;; esac
		fi
		open_url_handler "$ytfzf_selected_urls"
		close_url_handler "$url_handler"

		[ "$is_loop" -eq 0 ] && break
	done
}

main

    # doing this after the loop allows for -l and -s to coexist
    while [ "$search_again" -eq 1 ] ; do
        clean_up
        initial_search=""
        main
    done

but the problem is still there. Can I clean the cache folder now?

---edit
Ok, deleting the cache was the dumbest idea I had in 2022! It deleted the history file as well, which had ~1700 entries!
For a future update, can you please move the history file to ~/.config/ytfzf/ folder and offer a parameter to delete it? I do not know what else counts as cache for ytfzf, but this one shouldn't.

@Euro20179
Copy link
Collaborator

make sure to change the initial_search="" line to the new line in the patch.

@pitsi
Copy link
Author

pitsi commented Dec 16, 2022

make sure to change the initial_search="" line to the new line in the patch.

Can you please be more specific about that? I am not that experienced with bash :(

@Euro20179
Copy link
Collaborator

You put this

    # doing this after the loop allows for -l and -s to coexist
    while [ "$search_again" -eq 1 ] ; do
        clean_up
        initial_search=""
        main
    done

You need to put this

# doing this after the loop allows for -l and -s to coexist
while [ "$search_again" -eq 1 ] ; do
    clean_up
    init_and_make_search "" "prompt"
    main
done

The difference is that instead of just initial_search="" (resetting the variable), it's running the init_and_make_search function.

@pitsi
Copy link
Author

pitsi commented Dec 17, 2022

This is what I now have at the lines above

# doing this after the loop allows for -l and -s to coexist
while [ "$search_again" -eq 1 ] ; do
	clean_up
	initial_search="" "prompt"
	main
done

but it still behaves like before. I even removed the indents from the left side.
I will remove the package and reuse the script from the development branch until a new version is released.

@Euro20179
Copy link
Collaborator

do not put initial_search="" "prompt", put init_and_make_search "" "prompt"

@pitsi
Copy link
Author

pitsi commented Dec 17, 2022

Ok this fixed it. I am posting my change again, just in case

main

# doing this after the loop allows for -l and -s to coexist
while [ "$search_again" -eq 1 ] ; do
	clean_up
	init_and_make_search "" "prompt"
	main
done

There is a missing "=" after the word search on line 6. I do not know if it is important, I am only mentioning it.
It does work, but it does not get me back to history once playback is done. Please note that I use search_source=hist as seen in my config above.

@Euro20179
Copy link
Collaborator

Euro20179 commented Dec 17, 2022

Please note that I use search_source=hist as seen in my config above.

You can replace "prompt" on line 6, with "$search_source" if you want. I may add this to the main script as it makes more sense.

There is a missing "=" after the word search on line 6. I do not know if it is important

init_and_make_search is a function not a variable, there is no need for an = sign.

@pitsi
Copy link
Author

pitsi commented Jan 21, 2023

Closing as fixed in 2.5.4. I did not notice it was released here 1 week ago, I was waiting for the the package update on my distro :(

@pitsi pitsi closed this as completed Jan 21, 2023
@pitsi
Copy link
Author

pitsi commented Jan 28, 2023

One thing I just noticed. The search again parameter seems to work only once.

I launch ytfzf -q, so as to grab entries from its search history, it finds stuff, it plays the one I choose and then it asks for a new search, instead of returning to the search history. Is this normal?

@Euro20179
Copy link
Collaborator

oddly, yes it was hardcoded to only show the prompt with -s, this has been fixed in the development branch. (please note that -q will use a prompt as a backup if nothing was selected from history, I will likely be changing this.)

@pitsi
Copy link
Author

pitsi commented Mar 6, 2023

The 1+ month late reply :(
I just noticed that v2.5.5 was released 5 days ago. Does it solve the issue I mentioned above?
Debian is under freeze now, so I doubt it will reach the repo before summer.

@Euro20179
Copy link
Collaborator

The 1+ month late reply :(

All good, and yes this is fixed in 2.5.5

@pitsi
Copy link
Author

pitsi commented Mar 6, 2023

Thank you :)
If it does not reach the repo, I will probably end up using it as a standalone script like like I used to before debian decided to package it. Meanwhile, I have more serious issues to solve with mpv and the new yt-dlp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants