refactor(Application Search): Improve Linux compatibility

oliverschwendener / ueli

Cross-Platform Keystroke Launcher

https://ueli.app

MIT License

3.62k stars 239 forks source link

refactor(Application Search): Improve Linux compatibility #1097

Closed ke1v closed 3 months ago

ke1v commented 4 months ago

Changes:

Refactored LinuxAppIconExtractor to use sharp instead of calling convert on the command line as some Linux distros (notably Linux Mint) does not come with the convert package installed by default.
Refactored LaunchDesktopFile to use gio launch as it appears to be installed on all Linux distros I tested (even KDE based ones)

@oliverschwendener I import sharp directly in LinuxAppIconExtractor. Let me know if you'd like me to create a module to handle image conversion. I'm also unfamiliar with electron-builder so I'm not sure if asar is the best way to handle packaging sharp (it wouldn't bundle without it)

oliverschwendener commented 4 months ago

Can you try packaging the app and check if it works with the newly added package?

rm -rf node_modules

pnpm config set shamefully-hoist true --location=project
pnpm config set auto-install-peers true --location=project
pnpm config set strict-peer-dependencies true --location=project

pnpm install
pnpm build
pnpm package

ke1v commented 4 months ago

Tried it and it still doesn't package correctly without asarUnpack, which I think is because sharp uses C binaries. I could try switching to a pure Javascript image library if you don't want to use libraries with binary dependencies.

oliverschwendener commented 4 months ago

So, if this doesn't work when packaged I don't think we can merge it 🤷

oliverschwendener commented 4 months ago

If you want, you can open a separate PR for the refactoring in LaunchDesktopFile so we can merge that.

ke1v commented 3 months ago

Sorry about the late reply, but I think I may have misworded my initial issue. sharp works and packages without issue with the addition of asarUnpack but I thought you had disabled asar for electron. However I believe it is enabled by default so this shouldn't be an issue.

oliverschwendener commented 3 months ago

Generally I am very conservative when it comes to adding more dependencies, especially when they're only used on one platform. But let's give this a try as it simplifies the code a lot regarding the icon generation on Linux. Nevertheless, I reserve the right to remove this package anytime if it proves to be a burden.