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

support easyeffect 7.1.7 #19

Merged
merged 2 commits into from
Jun 22, 2024
Merged

support easyeffect 7.1.7 #19

merged 2 commits into from
Jun 22, 2024

Conversation

remisiki
Copy link
Contributor

closes #18

@ulville
Copy link
Owner

ulville commented Jun 22, 2024

Thanks for informing me about the change #18 and for the fix. But blindly changing the key names will break the extension for the users who are still on the older versions and will stay so for a while (users of native package for easyeffects on fixed release distros). Also we need to handle the flatpak easyeffects the same way. (your fix only changes the key names for native package)

Checking the version of EasyEffects comes to mind first but somehow it doesn't seem to be possible. I tried easyeffects --version in terminal but it doesn't output anything. I searched through the gjs api references to find a way to check an app's version but no luck.

Other options are

  1. Check the GSettings Sceme Keys and check which of our key names are there.
  2. Use a try-catch statement. Try one of the keys, if it fails try the other one.

Option 1 feels more like the right way to me. I also did my research and test, here is how we get the keys for a specified settings scheme using Gio:

const settings = new Gio.Settings({
    schema_id: 'com.github.wwmm.easyeffects',
});
let settingsSchema = settings.settings_schema
let keys = settingsSchema.list_keys(); // Array of strings

then we can check which kind of keys are in the keys array and use the ones we have.

For flatpak we can't access its GSettings object directly but we use gsettings command running inside its flatpak sandbox to get values we need. We can do the same to get the keys. Normally the command is gsettings list-keys com.github.wwmm.easyeffects. But for flatpak it becomes flatpak run --command=/usr/bin/gsettings com.github.wwmm.easyeffects list-keys com.github.wwmm.easyeffects. Here is how we can do it in our code:

let getKeysCommand = [
    'flatpak',
    'run',
    '--command=/usr/bin/gsettings', // command we want to run instead of easyeffects
    'com.github.wwmm.easyeffects', // inside easyeffects' flatpak sandbox
    'list-keys', // argument 1
    'com.github.wwmm.easyeffects', // argument 2
];
let keys = await this.execCommunicate(getKeysCommand); // String (values are seperated by new line)

As a similar approach we can check which kind of key names are in there and use the ones we have.

If you can work on the implementation and update the PR I'll wait, otherwise tell me so I can close this PR and implement it myself

@ulville
Copy link
Owner

ulville commented Jun 22, 2024

We may not need to implement the check logic for the flatpak tho. For the time being it's still at 7.1.6 on Flathub. But probably it'll be updated to 7.1.7 soon. And when it happens practically everyone using the flatpak version will be fine. It's probably fine to just change the key names in

command.concat(['last-used-output-preset'])
and
command.concat(['last-used-input-preset'])

@remisiki
Copy link
Contributor Author

Thanks for your information @ulville, I agree that a simple renaming is not a good solution and can break the extension for users still using older versions of easyeffects. Yes, option 1 should be a better workaround for now, so I implemented a schema key validation block that should be working for both version 7.1.7 and versions before.

I was using native installation of easyeffects so I did not notice the change still not took place for flatpak users, and now I have tested the new code on both native and flatpak platform.

@ulville
Copy link
Owner

ulville commented Jun 22, 2024

Great! The code looks good to me. And you've already tested it on both of the versions , much better! Thank you for your contribution! 👍👍👍👍👍

@ulville ulville merged commit eb6ecaa into ulville:master Jun 22, 2024
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.

Error: GSettings key last-used-output-preset not found in schema com.github.wwmm.easyeffects
2 participants