Martchus / syncthingtray

Tray application and Dolphin/Plasma integration for Syncthing
https://martchus.github.io/syncthingtray/
Other
1.54k stars 43 forks source link

settingsdialog: Automate detection of EXEC_PATH #177

Closed doronbehar closed 1 year ago

doronbehar commented 1 year ago

This should be resilient for the case where a syncthingtray executable is present in the same directory as the applicationFilePath.

Solves #176.

doronbehar commented 1 year ago

Tested the patch applied on v1.3.2 on Nixpkgs.

Martchus commented 1 year ago

Maybe adding an additional option AUTOSTART_EXEC_FILE_NAME would make sense for your specific case:

#ifndef SYNCTHINGWIDGETS_AUTOSTART_EXEC_PATH
#ifdef SYNCTHINGWIDGETS_AUTOSTART_EXEC_FILE_NAME
#define SYNCTHINGWIDGETS_AUTOSTART_EXEC_PATH QCoreApplication::applicationDirPath() % QChar('/') % QStringLiteral(SYNCTHINGWIDGETS_AUTOSTART_EXEC_FILE_NAME)
#else
#define SYNCTHINGWIDGETS_AUTOSTART_EXEC_PATH QCoreApplication::applicationFilePath()
#endif
#endif
doronbehar commented 1 year ago

Now I realize, that even with some version of this patch applied, any Nix user will likely still want to set AUTOSTART_EXEC_PATH, as the path of the wrapper detected can change when one updates their system... Hence, I think I'm going to wait on this for a while, and perhaps ask for advice on our community :)

Thanks in the meantime for the suggestion and the willing to help!

Martchus commented 1 year ago

Hence, I think I'm going to wait on this for a while, and perhaps ask for advice on our community :)

That's a good idea. Removing a feature (or making a breaking change of its behavior) afterwards is always problematic. So it would be good to know what's really wanted in the first place.

doronbehar commented 1 year ago

After thinking about this for a while, I reached the conclusion that setting simply syncthingtray in AUTOSTART_EXEC_PATH is sufficient for our purposes. Also, if any poweruser uses different installation paths, or different executable names, they can always edit the desktop file themselves. Hence I think this can be closed :)

Martchus commented 1 year ago

This makes sense. And for the specific version installed via your packaging I suppose it isn't expected that one renames the binary. At least I wouldn't normally rename a file "owned" by a package (on any distribution). I suppose you package will also always install to PATH.