Moon-0xff / gnome-mpris-label

A music related GNOME extension.
GNU General Public License v3.0
50 stars 9 forks source link

code update for gnome45 compatibility #84

Closed Batwam closed 9 months ago

Batwam commented 9 months ago

As promised, updated code for gnome45 compatibility ready for testing. It has been designed to require as little code change as possible to minimise the risk of conflict later on.

Regarding how we "manage" the 2 versions going forward, we could code from the gnome 43/44 version but update the install script to automatically pick up the Gnome version and apply a patch if gnome >44 at the time of install? Obviously, that's just for the github version, 2 sets of files will be uploaded to EGO. Another alternative would be for the gnome45 version to be main and the older versions to be where we apply the (reversed) diff. It all depends on what we consider should be the "standard" gnome version for most users.

Moon-0xff commented 9 months ago

I expect an easier time managing the differences by using the 43/44 version as base. I think the diff is small enough to be served as a patch file so I'm going on that route. I will push an experimental branch soon enough.

Moon-0xff commented 9 months ago

Done. See the new branch 45-as-patch.

What it's left to do is an explanation on README and perhaps your idea of updating the install script to the detect if the host is running 45. Though I think doing that automatically might not be a good thing.

Perhaps just a simple message like:
If you are running GNOME 45 you will need to patch the compatibility diff file [instructions here], then run this script again

Batwam commented 9 months ago

Yeah, I think that the patch would work. I have generated a diff based on this PR which we can use if you are happy with the content. We could also update the install script to apply the patch if the version is >44? That would help me as I'll be switching to gnome45 later this month.

Moon-0xff commented 9 months ago

I think we commented at the same time 😅. See message above.

I generated the patch file with git format-patch by the way.

Applying the patch is just a matter of running $ patch < patches/gnome45-compatibility.patch before $ ./install so I don't think it adds much more complexity to the installation procedure, though I'm not against the idea of automating that step.

Batwam commented 9 months ago

saw that, that's great. I think that we have a good solution now by adding this simple patch file.

I was wondering, the index line in the diff looks like a SHA index for the original files. I'm guess that this means that the patch will no longer work if the original files change? If we want this diff to work in the future as we change the files, should be do a diff -–no-index instead?

We will probably need to update the revisions to add gnome46, etc... in the future but we will be able to update the diff file manually for that right?

Moon-0xff commented 9 months ago

I think the index serves the same purpose as the SHA in the commits.

And I know the patch files will work until patch can no longer apply it, same as when merging branches on git.

We will probably need to update the revisions to add gnome46, etc... in the future but we will be able to update the diff file manually for that right?

Yes, provided that 46 will be compatible with 45, it should be just a matter of updating the patch.

Moon-0xff commented 9 months ago

Alright so this PR was technically merged through d43bacd46c5c600b303fd6a7cd8650a422a375e8.

As I said before, it needs an explanation on the README and at least a mention on install.sh.
It also needs to be tested.

I'll close this now.