aunetx / deezer-linux

An universal linux port of deezer, supporting Flatpak, Appimage, Snap, RPM, DEB...
https://flathub.org/apps/dev.aunetx.deezer
288 stars 14 forks source link

Fix #21 app must stay in background by default #22

Closed adynemo closed 2 years ago

adynemo commented 2 years ago

Hello ! 🙂

This PR is to fix #21 After closing the app, it is not stayed in background and, obviously, the systray disappeared. I noticed that the quit.patch was not applied like the other patches. With this change, it is finally applied. I think this is caused by the fix-isDev-usage.patch. It is applied in first, shifts the lines and the patch provided by quit.patch is not found.

So, with this PR, it works fine. But, it's possible that the bug will come back after merging the PR #20 for example... Is there another way to apply patches?

aunetx commented 2 years ago

Thank you again for your work!

To be honest, I guess the only alternative way to apply patches without fearing that one of them would prevent the others from being applied would be to "pack" all the patches in one, but that would be unmaintainable when more and more are added...

One thing I could do though is making the build entirely fail when one patch is not applied; right now the patching silently fails and this prevents me from seeing it. So, I will look about how to do this, maybe this would help enough!

adynemo commented 2 years ago

It's a pleasure to work on that. Thank you for this app! 🙂

Your idea sounds good. I thought about a naming convention to apply patches from the last line to the first (for a same file), but it could be hard to maintained too. The build failed should be easier and do the job.

aunetx commented 2 years ago

To be honest, I did not find how to stop when a patch fails for the moment ':) it should stop the build when patch returns an error, but something is not happening as it should...

It would require some investigations, but the number at the beginning of the file is a good idea! I think make already lists the files alphanumerically, so that could be good enough

adynemo commented 2 years ago

Ah too bad for your idea, I'd like it 😕 For the naming, I will try to submit you a PR in the next days 🙂

adynemo commented 2 years ago

Ok I get it! 😀 The problem is not due to the lines move. I tried with random lines (like 30 instead of 3080) and patch finds anyway. In fact, the real problem was on the numbers after the comma (after the lines): @@ -3079,7 +3080,7 @@. Exactly, the second 7. It's the number of lines, plus the number of additions lines, minus the number of deletions lines (number of lines in output, after applying the patch). So, 10 not 7. It was why patch failed. Now, all is good, so no need to naming or whatever 🥳