pystardust / ani-cli

A cli tool to browse and play anime
GNU General Public License v3.0
7.92k stars 544 forks source link

Non interactive selection broken #491

Closed 71zenith closed 2 years ago

71zenith commented 2 years ago

Metadata (please complete the following information) Version: 1.6.5 OS: Arch Shell: dash, zsh Anime: not applicable

Describe the bug When selecting an episode and anime non-interactively, it runs into an error

Steps To Reproduce

  1. Run ani-cli -a 10 death note

Expected behavior It should start playing the episode

Screenshots (if applicable) 15:09:54-04-02

margual56 commented 2 years ago

This bug stems from two issues:

1 - Anime selection

As said in the help:

When selecting non-interactively, the first result will be selected, if anime is passed

Therefore, the variable select_first should be assigned the value 1 if either -d or -p are passed:

while getopts 'viq:dp:chDUVa:' OPT; do
     case $OPT in
         h)
             help_text
             exit 0
             ;;
         d)
             is_download=1
+            select_first=1
             ;;
         a)
             ep_choice_to_start=$OPTARG
             ;;
         D)
             : > "$logfile"
             exit 0
             ;;
         p)
             is_download=1
             download_dir=$OPTARG
+            select_first=1
             ;;
         i)
             player_fn="iina"
             ;;
         q)
             .
             .
             .

And fixed!

2 - Episode selection

The second issue appears to be in the if from line 573, where it is said:

If the variable $ep_choice_to_start is not empty, then set select_first to one

Problem is, select_first is not used anywhere, the only variable that "matters" is REPLY. Therefore, a new if statement could be introduced to check if $select_first is set; or alternatively, you could do:

- [ -n "$ep_choice_to_start" ] && select_first=1
+ if [ -n "$ep_choice_to_start" ] ; then
+     REPLY=1
+     select_first=1
+ fi

And it would function normally.

I checked and with those solutions implemented, it works just fine. I took the liberty of creating a pull request solving this issue: #494