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: no-detach #1264

Merged
merged 9 commits into from
Feb 3, 2024
Merged

feat: no-detach #1264

merged 9 commits into from
Feb 3, 2024

Conversation

port19x
Copy link
Collaborator

@port19x port19x commented Jan 24, 2024

Pull Request Template

Type of change

  • Bug fix
  • Feature
  • Documentation update

Description

fixes #1261

Checklist

  • any anime playing
  • bumped version

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

@port19x port19x linked an issue Jan 24, 2024 that may be closed by this pull request
@port19x port19x changed the title feat: no_detach feat: no-detach Jan 24, 2024
@port19x port19x marked this pull request as ready for review January 24, 2024 15:02
@port19x port19x requested a review from Derisis13 as a code owner January 24, 2024 15:02
@germaniuss
Copy link

Awesome you're actually doing this. Locally I also removed the >/dev/null 2>&1 & at the end since I believe we do not want to redirect the mpv output (nor open it in a background process). Though to be fair I am not knowledgeble enough about bash nor terminal emulators to actually know is that make a difference.

@port19x
Copy link
Collaborator Author

port19x commented Jan 30, 2024

Can you test if it works like this as well?

@port19x port19x merged commit fde5470 into master Feb 3, 2024
9 checks passed
@port19x port19x deleted the no-detach branch February 3, 2024 17:55
@germaniuss
Copy link

Awesome you're actually doing this. Locally I also removed the >/dev/null 2>&1 & at the end since I believe we do not want to redirect the mpv output (nor open it in a background process). Though to be fair I am not knowledgeble enough about bash nor terminal emulators to actually know is that make a difference.

@port19x Sorry for getting back to you so late, had a pretty hectic week. Just tested it out, it does not seem to properly function without this (the anime is played in the background and I have to pkill mpv. I modified the sript to do $([ "no_detach" == 0 ] && echo ">/dev/null 2>&1 &") ;; at the end and it seems to work. Is there any technical reason not to do it?

@port19x
Copy link
Collaborator Author

port19x commented Feb 4, 2024

Awesome you're actually doing this. Locally I also removed the >/dev/null 2>&1 & at the end since I believe we do not want to redirect the mpv output (nor open it in a background process). Though to be fair I am not knowledgeble enough about bash nor terminal emulators to actually know is that make a difference.

@port19x Sorry for getting back to you so late, had a pretty hectic week. Just tested it out, it does not seem to properly function without this (the anime is played in the background and I have to pkill mpv. I modified the sript to do $([ "no_detach" == 0 ] && echo ">/dev/null 2>&1 &") ;; at the end and it seems to work. Is there any technical reason not to do it?

Nah, I'll patch it

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.

Add a --no-detach option
3 participants