Thanks for the review. All done.

@puzzlewolf Thanks for the feedback. All done.

@puzzlewolf @baconpaul Thanks!

Now all we need is a way to make zenity work without installing it globally or in the user profile.
I tried the folowing in the patchPhase:

    substituteInPlace src/linux/UserInteractionsLinux.cpp --replace zenity ${gnome3.zenity}/bin/zenity
    substituteInPlace src/common/gui/PopupEditorDialog.cpp --replace zenity ${gnome3.zenity}/bin/zenity

but that gave a build error.
Any ideas?

Yeah, src/linux/UserInteractionsLinux.cpp contains the string zenity in a lot more places than the ones we'd like to change. This builds for me:

substituteInPlace src/linux/UserInteractionsLinux.cpp --replace 'execlp("zenity"' 'execlp("${gnome3.zenity}/bin/zenity"'
substituteInPlace src/common/gui/PopupEditorDialog.cpp --replace zenity ${gnome3.zenity}/bin/zenity

Can you check if it actually works? Even if it works, I really don't know if this is a good idea.

@puzzlewolf That works wonderfully, thank you!
Why would it not be a good idea?
FWIW, steam does it too:

Yeah, src/linux/UserInteractionsLinux.cpp contains the string zenity in a lot more places than the ones we'd like to change. This builds for me:

substituteInPlace src/linux/UserInteractionsLinux.cpp --replace 'execlp("zenity"' 'execlp("${gnome3.zenity}/bin/zenity"'
substituteInPlace src/common/gui/PopupEditorDialog.cpp --replace zenity ${gnome3.zenity}/bin/zenity

Can you check if it actually works? Even if it works, I really don't know if this is a good idea.

Folks: I am more than happy to take changes to upstream so you don't have to do these substitutions to build (in fact I am way way happier to take changes upstream).

For instance we could easily make a quick function which gets the name of the zenity executable in the C++ and defaults to 'zenity' but reads an environment variable. And even have it conditionally build a different version. I would merge that PR happily. Would even put it on my queue if you want me to do it if you just open up an issue over on the surge repo.

The more that we have visibility on changes made to the code in master, even if it ends up being a bit of spaghetti of ifs, the less likely it is we will break you when we have a new release. etc... you know the drill.

The more that we have visibility on changes made to the code in master, even if it ends up being a bit of spaghetti of ifs, the less likely it is we will break you when we have a new release. etc... you know the drill.

An obvious change, for instance, now that I have moved master to 100% cmake, is to have some cmake rule that detects nixos build and adds a -DNIXOS and then change the zenity symbol based on that. Is there some way at build time that you indicate I am in your environment? (And also are you building master or 1.6.6? like I said, master moved entirely to cmake this week)

@baconpaul That's very generous of you, but my C/C++ fu is lacking, and I'd hate to saddle you up with more work on Surge that only benefits a few users.

And also are you building master or 1.6.6?

This PR is building 1.6.6.

Great! So can we merge this after a month+? =)

I have updated the package definition after the switch to cmake (which simplified it a lot).

Thanks!