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

fix: where_mpv checks for mpv than ANI_CLI_PLAYER #1424

Merged
merged 9 commits into from
Sep 18, 2024
Merged

Conversation

71zenith
Copy link
Collaborator

Pull Request Template

Type of change

  • Bug fix
  • Feature
  • Documentation update

Description

ramble here

Checklist

  • any anime playing
  • bumped version

  • next, prev and replay work
  • -c history and continue work
  • -d downloads work
  • -s syncplay works
  • -q quality works
  • -v vlc works
  • -e select episode works
  • -S select index works
  • -r range selection works
  • --skip ani-skip works
  • --skip-title ani-skip title argument works
  • --no-detach no detach works
  • --dub and regular (sub) mode both work
  • all providers return links (not necessarily on a single anime, use debug mode to confirm)

  • -h help info is up to date
  • Readme is up to date
  • Man page is up to date

Additional Testcases

  • The safe bet: One Piece
  • Episode 0: Saenai Heroine no Sodatekata ♭
  • Unicode: Saenai Heroine no Sodatekata ♭
  • Non-whole episodes: Tensei shitara slime datta ken (ep. 24.5, ep. 24.9)

@71zenith 71zenith requested a review from Derisis13 as a code owner September 18, 2024 14:46
Copy link
Collaborator

@Derisis13 Derisis13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on linux against no mpv, flatpak mpv and system mpv. It works all the time

@71zenith
Copy link
Collaborator Author

Would be safer to have one test against mac to be sure. @port19x ?

@71zenith
Copy link
Collaborator Author

known issue for parity with discord
vlc wont work with --vlc/-v only ANI_CLI_PLAYER=vlc works due to early exit if mpv isnt found

Copy link
Collaborator

@port19x port19x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure this will never fail, regardless of installed player or not

ani-cli Outdated Show resolved Hide resolved
ani-cli Show resolved Hide resolved
@71zenith
Copy link
Collaborator Author

does it work as expected on mac?

@port19x
Copy link
Collaborator

port19x commented Sep 18, 2024

To further comment on the control flow here:

  1. Plattform Detection
  2. Either where_iina or where_mpv run. I'm going with where_iina here.
  3. where_iina checks of the iina-cli path (the out-of-band installation, not with homebrew)
    4a. if it finds it, it returns sth matching IINA
    4b. if it does not, it returns the string "iina"
  4. We get to the case $player_function in
    6a. if 4a, dependency check has already been performed and is thus skipped
    6b. if 4b, dep_ch iina is executed, failing if not found

@port19x
Copy link
Collaborator

port19x commented Sep 18, 2024

Finally catching the shfmt thingy again 😅

@port19x
Copy link
Collaborator

port19x commented Sep 18, 2024

I'll check on mac now

@port19x
Copy link
Collaborator

port19x commented Sep 18, 2024

Can confirm working on mac

@port19x port19x merged commit 81e6ecd into master Sep 18, 2024
9 checks passed
@port19x port19x deleted the fix_where_mpv branch September 18, 2024 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants