cinnamon-settings.py: Additional modules when Mint modules are absent

[rcalixte]

• Removing trailing whitespace as well

[clefebvre]

don't we test to see if the executable is present before showing the module?

if so we don't need to say "if Mint, else"

it's a bit weird to have Mint on one side and then all other distros on the other within the code.

[rcalixte]

don't we test to see if the executable is present before showing the module?

if so we don't need to say "if Mint, else"

The goal was to avoid collisions and duplicates in Mint where other installations of Cinnamon do not have the additional modules present. Rather than chasing down multiple upstreams to patch Cinnamon's settings modules, this would carve out a space for those modules and expand the list of modules on non-Mint installations of Cinnamon.

it's a bit weird to have Mint on one side and then all other distros on the other within the code.

Most of the additional modules could be moved to the main definition but the main exception is software-properties-gtk. @clefebvre Are the plans in future versions of Mint to use a different namespace for this? Otherwise, this would conflict on Mint installations. If this is incorrect, we can consolidate the additional modules and simplify things. The original goal was to leave Mint undisturbed.

[clefebvre]

OK, an easy way to solve this would be to add a module blacklist in gsettings. Mint could set this to "['software-properties-gtk']", other distros could set this to suit themselves.

[rcalixte]

OK, an easy way to solve this would be to add a module blacklist in gsettings. Mint could set this to "['software-properties-gtk']", other distros could set this to suit themselves.

Wouldn't this be more difficult to maintain? Not only for other downstreams but for users as well? This would create two locations to configure one facet within Cinnamon.

Would it be easier to have just software-properties-gtk as the only additional module in the conditional and move the rest into the main list?

[clefebvre]

Yeah, you could only show software-properties-gtk if mintsources isn't present. That would work and be quite simple.

[fredcw]

I curious about something. As each item seems to have an associated .desktop file, would it not be better to get the icon name for each item from it's .desktop file rather than have to specify it in this file. This would also ensure that the icon used in system settings would always be consistent with the icon used in the menu applet.

[clefebvre]

I curious about something. As each item seems to have an associated .desktop file, would it not be better to get the icon name for each item from it's .desktop file rather than have to specify it in this file. This would also ensure that the icon used in system settings would always be consistent with the icon used in the menu applet.

Yes. Good point.

[rcalixte]

Yeah, you could only show software-properties-gtk if mintsources isn't present. That would work and be quite simple.

Updated. I'm open to modifying the conditional if needed. We can check inside the load_standalone_modules function as well after determining whether the process was found. That was the original implementation when this PR was opened.

I curious about something. As each item seems to have an associated .desktop file, would it not be better to get the icon name for each item from it's .desktop file rather than have to specify it in this file. This would also ensure that the icon used in system settings would always be consistent with the icon used in the menu applet.

I encountered the challenge with icons when I was testing since I had installed so many packages but had no way to differentiate them when they were all the same. If we're going to source .desktop files for icons, would it make sense to source the localized Name as well? (This is all assuming sourcing the .desktop files is trivial enough as well. I know it's possible, I primarily want to try to keep things simple.)

[fredcw]

@rcalixte Oh, I wasn't suggesting you do the icon thing as part of this PR, it's a separate issue for a different PR. Maybe I'll take a look at it sometime. Then again, maybe it's not worth bothering with. "if it ain't broke, don't fix it" as it were.

[lestcape]

I don't think it's a good idea to hardcode a list of modules at all. I thinks that all modules should be provided in a module file and there should be a function implemented by modules to evaluate if a module can be enabled or not. Thus, the responsibility of loading or not a module will be delegated to the module itself, which is the one who really knows its own requirements to be loaded.

Another improvement that I think should be nice is to have something to override the default modules. This is useful, for example, when there is more than one software to do the same thing and both are available on the system, but Mint prefers to use one, while a different Linux distribution prefers to use the other software. This can be done simply by using two different module folders, one which would be the primary "default" and the other would be where the distribution should put its "custom" module version. The cinnamon-settings should look first in the "custom" folder and then in the "default" folder and modules with the same name should only be loaded the first time they found. Using this organization, the modules of a distribution can even be added and maintained in a totally different and exclusive package of the distribution, without any need to modify the default code that comes with Cinnamon. Furthermore, the external distro can even excluded a module they don't like by providing a module witch the same name whose evaluation function is always return false.

The only downside to this is that it can be costly in terms of performance to have to load each module file to test its availability, but I believe the benefit would outweigh the prejudice and the real cost of doing this would be in terms of milliseconds. This if we are careful to separate the module file where the evaluation function is from the actual functionality of the module, so that no additional library needs to be loaded in a module that is not actually going to be loaded because it evaluation function return false.

[rcalixte]

Yeah, you could only show software-properties-gtk if mintsources isn't present. That would work and be quite simple.

This is now done in the latest rebase.

[mtwebster]

Can you rebase this?

The only issue is the addition of this line:
https://github.com/linuxmint/Cinnamon/blob/master/files/usr/share/cinnamon/cinnamon-settings/cinnamon-settings.py#L90
and the comma at the end of the previous line.