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

Allow Non-Windows installations to populate Windows folder artwork #978

Merged

Conversation

muchtall
Copy link
Contributor

Proposed fix to #977

@rmcrackan
Copy link
Owner

Thanks for the PR! This removes the check but it doesn't fix the underlying issue: the developer who put this here said it doesn't work for linux and the check is to avoid errors. After removing the check, does it actually work for linux?

@wtanksleyjr
Copy link
Contributor

wtanksleyjr commented Sep 17, 2024 via email

@rmcrackan
Copy link
Owner

Am I correct that this is the case being addressed: Libation is running on a Linux system and saving to a Windows system. The user would like the saved folder and files to be subject only to Windows limitations.

If this is not the case, please try again since I misunderstood.

If this is the case, I need to reject this PR. Taking off the safety rails to support that mismatch will hard more users than it helps.

You should be able to cobble together a script by using the Libation setting to export the image (settings > audio file options > save cover image alongside audiobook). Here's an example for code setting the dir icon:

@wtanksleyjr
Copy link
Contributor

wtanksleyjr commented Sep 17, 2024 via email

@rmcrackan
Copy link
Owner

If I'm understanding correctly:

If I allow this check to be removed, then the check will not be hit by Libation on linux systems and thus Libation on linux will attempt to set the directory image. This is the PR author's desire, per this from their ticket

Even though the app is running in Linux via Docker, it's still desired that I be able to populate the directory with the ico and ini files so that I get the custom thumbnail when viewing the data directory in Windows.

Removing the check will allow them to do this. The other side effect of removing this check is that for typical linux users (who are running libation on linux and who are saving to linux) that this check will no longer be hit. This typical scenario is why the check exists.

@wtanksleyjr
Copy link
Contributor

wtanksleyjr commented Sep 18, 2024 via email

@rmcrackan rmcrackan merged commit 303192c into rmcrackan:master Sep 18, 2024
@rmcrackan
Copy link
Owner

Your PR is now available in pre-release v11.4.1

@muchtall
Copy link
Contributor Author

Sorry I missed the comment party. @wtanksleyjr understood the use-case correctly. I'm a Windows desktop user, but I use Docker on Linux wherever I can. Having the Docker instance populate the Windows-specific folder icon files is still helpful, thus the PR.

Thanks for merging!

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