Gtk3 code cleanup for easier maintenance (no visual changes)

[nathan818fr]

Hello,

I have been using the arc theme for a long time (thanks for keeping it up)!

I understand that your goal is only to maintain the theme, without making any unnecessary visual changes, and I totally agree with this choice! Users who want to make changes can do a personal fork of the theme (and this is already what is happening).

In order to facilitate maintenance and fork creation, I think a cleanup of some parts of the code could be interesting.

So here is a proposal that merges the gtk3 duplicated code. Important: The build output is identical (same assets and css files).

Summary of changes:

• meson.build:
  • Duplicate code for asset generation has been merged (using a loop instead)
  • Warnings (due to overwritten files) have been fixed
  • SASS variables ("$variant" and "$transparency") are set from the custom_target command (instead of using one sass file per combination). Note: the script used is POSIX sh compliant; and compatible with all sassc & dart-sass releases.
  • A new SASS variable with the gtk version is also defined (and allows to conditionally manage the differences between the supported versions)
• assets.svg & assets.txt:
  • As these files are similar between all versions, they have been merged into the root.
• gtk-3.20, gtk-3.22 & gtk-3.24:
  • SCSS files have been merged and the differences managed conditionally
  • Moved to gtk-3.2x
• gtk-3.18 :
  • SCSS files have not been modified (the difference with 3.20+ versions is too important)
  • Moved to gtk-3.1x

Changes overview: Before / After

[jnsh]

Thank you for the proposal.

Unfortunately this is pretty much impossible to review, since all the changes are cobbled into a single commit, and I have no idea what any of the individual changes are related to.

Some of the build system changes sound possibly good, but I'm not too excited about the gtk3 code restructuring, as detailed below.

* `meson.build`:

  * Duplicate code for asset generation has been merged (using a loop instead)

Reduced number of lines is nice. However I've been considering adding a build option for optionally disabling hidpi support, and the loop probably wouldn't work nicely with that. I don't really consider the duplicated code that problematic either.

  * Warnings (due to overwritten files) have been fixed

Very nice! But without a split commit I'm not sure how exactly this was achieved,..

  * SASS variables ("$variant" and "$transparency") are set from the custom_target command (instead of using one sass file per combination). Note: the script used is POSIX sh compliant; and compatible with all sassc & dart-sass releases.

I had planned simplifying this by using configure_file() and a single .scss.infile to generate the variant specific main SCSS files. I think that should be much simpler than your solution?

  * A new SASS variable with the gtk version is also defined (and allows to conditionally manage the differences between the supported versions)

* `assets.svg` & `assets.txt`:

  * As these files are similar between all versions, they have been merged into the root.

* `gtk-3.20`, `gtk-3.22` & `gtk-3.24`:

  * SCSS files have been merged and the differences managed conditionally
  * Moved to `gtk-3.2x`

* `gtk-3.18` :

  * SCSS files have not been modified (the difference with 3.20+ versions is too important)
  * Moved to `gtk-3.1x`

Changes overview: Before / After

Avoiding duplicate code would be great, and I understand having to edit a single file for all versions would simplify customization ( I would place customizations to a separate .scss file that could be imported for every version, that would make keeping in sync with upstream easier as well, but this may not work in every situation). However I do not like this at least for the following reasons:

• This is horrible for long term maintenance. The changes between the 3.2x versions are small due to lack of proper maintenance, and since there are only three versions, but imagine the mess with e.g. 10 versions and years of major changes.
• If there was such major restructuring to how versioned theme code works, similar restructuring would have to be applied to all versioned themes, not just GTK3.

3.24 should get fully refactored against upstream soon, so this would probably not be the best timing for such restructuring either. Also the older versions will eventually get removed, as LTS distributions that still ship those versions die out.

I'm open for further discussion if you disagree about the GTK3, but I wouldn't spend too much time on it for now. For the meson changes I can do a proper review, if you can split the changes to commits with single specific purpose. Also please take a look at the few notes about commit messages here.

[nathan818fr]

I understand that the changes for gtk3 can lead to a more difficult code base to maintain. I hadn't thought of it that way.

So I will open separate small pull requests for the reduced version of meson.build with the fixed warning (and I will split the commits).

I had planned simplifying this by using configure_file() and a single .scss.in file to generate the variant specific main SCSS files. I think that should be much simpler than your solution?

Yes, indeed. I can propose a pull request with this solution if you wish.

[jnsh]

So I will open separate small pull requests for the reduced version of meson.build with the fixed warning (and I will split the commits).

That works for me. Thank you!

I had planned simplifying this by using configure_file() and a single .scss.in file to generate the variant specific main SCSS files. I think that should be much simpler than your solution?

Yes, indeed. I can propose a pull request with this solution if you wish.

I'm quite busy with more urgent things for the theme, so all improvements to the build system (and otherwise) are greatly appreciated :)