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

feat: add -C/--copy-link to copy video link #1402

Closed
wants to merge 1 commit into from

Conversation

Vosjedev
Copy link

@Vosjedev Vosjedev commented Aug 22, 2024

-C or --copy-link to copy video link

Type of change

  • Bug fix
  • Feature
  • Documentation update

Description

This feature adds the option -C or --copy-link, and it takes the link that would otherwise be passed to mpv, vlc, or a downloader, and copies it to the system clipboard. I have added support for:

  • linux (detects wayland or xorg by the presence of $WAYLAND_DISPLAY and $DISPLAY, fails if both are unset)
  • MacOS (using pbcopy)
  • windows (using C:\Windows\System32\clip.exe)
  • termux (using termux-clipboard-set from the termux-api package)

it throws an error on iOS and prints the link instead, and for everything else it defaults to linux behaviour.
I have tested this on windows, linux (arch, running hyprland), and iOS. I'm planning on testing MacOS as well, but I have to wait a bit for that.
For any future issues with this feature or issues caused by this feature you're allowed to ping/assign me to solve it (seems logical to me, if I add a feature and it breaks I have to solve it ofc :D).

Note I have not checked all things in the checklist below, and for what I didn't check I appended (I have not tested) to the item. These items were difficult enough for me to skip because I don't think the features would break due to this addition. -r seems to be broken on master already (got from AUR), so I don't know what went wrong there. I'll make a bug report if needed.

Checklist

  • any anime playing
  • bumped version

  • next, prev and replay work (I have not tested)
  • -c history and continue work
  • -d downloads work
  • -s syncplay works (I have not tested)
  • -q quality works (probably)
  • -v vlc works
  • -e select episode works
  • -S select index works
  • -r range selection works (master seemed to be broken already?)
  • --skip ani-skip works (I have not tested)
  • --skip-title ani-skip title argument works (I have not tested)
  • --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) (I have not tested)

  • -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)

@port19x
Copy link
Collaborator

port19x commented Aug 22, 2024

This can already be achieved today by changing the environment variable ANI_CLI_PLAYER to xclip -selection c or an equivalent wrapper.

I'm impressed with the thorough implementation and testing you have done, but caution you to raise feature requests first, or at least wait with extensive support and testing until you have the green light from the project maintainer in an open source project.

I'm declining this feature, because I believe it brings little value to the table, despite being as well documented and implemented as it is.
Don't let this discourage you from contributing to OSS, you clearly have the motivation and skill to make a positive impact

@port19x port19x closed this Aug 22, 2024
@port19x
Copy link
Collaborator

port19x commented Aug 22, 2024

I should also add that ANI_CLI_PLAYER=debug will print out the links in a pipeable way

@Vosjedev
Copy link
Author

I'm impressed with the thorough implementation and testing you have done

Thank you!
(I was testing it on a mac while writing and found out it was broken on there, but I guess I don't have to fix a fix for it now, as I don't use a mac normally. I also found other issues with the formatting due to editor configuration issues on my side (to be precise incorrect tab usage))

but caution you to raise feature requests first, or at least wait with extensive support and testing until you have the green light from the project maintainer in an open source project.

That might me smart, although I did plan on keeping the fork if this pr would get closed, because I have had several occasions where I would have liked to be able to do this using a quick -C instead of messing with environment variables.

I'm declining this feature, because I believe it brings little value to the table, despite being as well documented and implemented as it is.

I half expected this, so no problem.

Don't let this discourage you from contributing to OSS, you clearly have the motivation and skill to make a positive impact

Of course it won't!

Also thanks for the quick review, I had expected it to take at least a day before I would get a response

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.

2 participants