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

Run script after rip #394

Closed
srussel opened this issue Apr 12, 2019 · 3 comments
Closed

Run script after rip #394

srussel opened this issue Apr 12, 2019 · 3 comments
Labels
Feature New feature Rejected Proposal/issue which won't be addressed

Comments

@srussel
Copy link

srussel commented Apr 12, 2019

In #393 I was looking for a way to get the output directory so I can run a script afterwards to fetch the artwork. Even if this issue is fixed it still has the problem that I can't select from multiple releases if I am redirecting the output.

I may be better to provide an config file option to run a script after a successful rip. Whipper could provide information such as output dir, release id, log file etc to the script via command line arguments or environment variables.

This would be useful for things like adding the release to beets, fetching artwork, and converting the tracks to other formats.

@Freso
Copy link
Member

Freso commented Apr 13, 2019

I feel like there’s already a ticket for this, but if there is, I can’t find it right now. In general, I’m not a huge fan of this. You can either include whipper in a script (and thus do things both before and after whipper runs; see e.g., rip-disc) or you can use general shell syntax (e.g., whipper cd rip && fetch-coverart.sh). Both of these approaches will obviously be more viable if whipper provides more information (like #393), but I feel it’s the wrong solution to have whipper fork into a different process upon completion rather than using native ways of running consecutive programs.

@srussel
Copy link
Author

srussel commented Apr 14, 2019

I got my script working based on the example in rip-disc. It would be nice if this was better documented. Maybe provide an examples directory with a few simple scripts for automating common tasks.

I still think it would be useful if whipper supported running a script after the rip. For a command line based tool like whipper I expect a lot of users will be scripting their ripping activities to do things like converting to other formats. It would be great if all these users did not have to spend time fiddling with scripts to rip to a temporary directory and grep for MB ids from log files (which could break if the log file format changed).

@Freso
Copy link
Member

Freso commented Apr 14, 2019

It would be nice if this was better documented.

If… what was better documented? There’s nothing whipper specific going on here. It’s standard Unix. https://duckduckgo.com/?q=linux+scripting should have plenty of documentation to get you started.

It would be great if all these users did not have to spend time fiddling with scripts

I mean, even if we end up adding this functionality to whipper (which I’m more and more against the more I think about it), they’d still have to "fiddle with scripts", it’d just be the script being evoked by whipper instead of the script evoking whipper.

rip to a temporary directory

There’s no need to rip to a temporary directory—it’s just how I decided to do it for my script (because of how I set up /tmp and utilise tmpfs’s). There are other and valid ways to go about this.

grep for MB ids from log files (which could break if the log file format changed).

Use a YAML parser instead of grepping if you want something more stable (though I don’t see grep stopping to give good enough results in any close future, based on our open ticket). Also, I consider the log file format as part of the public interface of whipper. Thus any (breaking) change to the .log should result in a MAJOR version (see SemVer), which should give downstream users a hint that they need to check that their setups still work as expected.

Note that this would also be true for your example. If the variables being passed to an external shell were to be changed/repurposed, scripts utilising this would certainly break as well.

@JoeLametta JoeLametta added Rejected Proposal/issue which won't be addressed Feature New feature labels May 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature Rejected Proposal/issue which won't be addressed
Projects
None yet
Development

No branches or pull requests

3 participants