After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 719667 - Integrate patches from lisanet.de fork
Integrate patches from lisanet.de fork
Status: RESOLVED OBSOLETE
Product: GIMP
Classification: Other
Component: General
git master
Other Mac OS
: Normal normal
: ---
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2013-12-02 01:28 UTC by Jehan
Modified: 2018-05-24 14:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to use 'About GIMP' in help menu, German translation (2.07 KB, patch)
2013-12-03 21:51 UTC, Max Mustermann
rejected Details | Review
handle migration on OSX of config folder of GIMP 2.8.2. (1.97 KB, patch)
2013-12-04 11:29 UTC, Jehan
committed Details | Review

Description Jehan 2013-12-02 01:28:34 UTC
Hey,

The release of GIMP on OSX from Simone Karin Lehmann has a list of patches. Many are obsolete, some we obviously don't want (like the save/export ones), but some would apparently make a lot of sense, according to Mitch.

The patches are here:
https://sourceforge.net/p/gimponosx/code/HEAD/tree/GimpPorts/ports/graphics/gimp2/files/

In the list, we could see that the carbon->cocoa patches seems outdated (we already use Cocoa platform), the save/export are unwanted, "many patches are obsoleted by our menu replacement" (dixit Mitch, not sure what is this menu replacement), "and some really really make sense", like "the color display change" (dixit Mitch again).
The code seems good, but the style should be fixed.

Unfortunately I have no Mac OS. Could a OSX dev go through this list, and select the interesting pieces?

I have set up a wiki page so that developers can keep track of the reviewed patches: http://wiki.gimp.org/index.php/Hacking:Merging_lisanet.de_fork
Comment 1 Michael Natterer 2013-12-02 08:10:10 UTC
I wished not everything I said on IRC made it into bugzilla ;)
Comment 2 Michael Natterer 2013-12-02 08:13:44 UTC
The save/export patches are not immediately obvious, we'd need an
explanation what exactly they do.
Comment 3 Michael Schumacher 2013-12-02 13:54:05 UTC
If the following description of the save & export changes in this fork is still accurate, then: https://mail.gnome.org/archives/gimp-developer-list/2012-November/msg00318.html
Comment 4 Jehan 2013-12-02 21:46:28 UTC
Well, reading this feature spec indeeds makes some sense to me. Basically it allows to "save" images into non-XCF format if and only if the image has just one layers (thus no layer information loss).

Of course it still provides some risk, like if you save as 'jpeg', you may have quality loss. If you ran some filters, painted or anything which changed the image, do you really want to overwrite your original?

In any case, I discarded these patches from the start because I was convinced you were kind of set on not touching save/export at all unless Peter proposes to. Glad to see there are still some interests in third party propositions. :-)
Comment 5 Michael Natterer 2013-12-02 22:30:26 UTC
I am _very_ skeptic. This seems all like unpredictable magic to me. I think
the recent usability improvements made save/export almost completely
painless except for those who absolutely refuse to consider adjusting
their workflow even the tiniest bit :)
Comment 6 Jehan 2013-12-02 22:33:52 UTC
I agree. Unless you would read the feature spec (and who read them except us?), you would just go and think "well sometimes saving works fine (= I can save by overwritting), sometimes it doesn't! I don't understand! GIMP is broken!".
Comment 7 Max Mustermann 2013-12-03 21:51:17 UTC
Created attachment 263450 [details] [review]
Patch to use 'About GIMP' in help menu, German translation

see http://wiki.gimp.org/index.php/Hacking:Merging_lisanet.de_fork#To_be_Reviewed for further details.
Comment 8 Michael Natterer 2013-12-03 22:18:59 UTC
Comment on attachment 263450 [details] [review]
Patch to use 'About GIMP' in help menu, German translation

This needs to be done different, according to platform standards. "_About" is right on GNOME.
Comment 9 Max Mustermann 2013-12-04 05:40:42 UTC
Ok. My solution took the short way, because I found the effort too much for just a single, not so important menu item. But to be consequent you're right.

On overview of relevant guidelines can be found in this Wikipedia article: http://en.wikipedia.org/wiki/Human_interface_guidelines
Comment 10 Jehan 2013-12-04 06:30:43 UTC
Well, so couldn't you just add a #ifdef for the OSX and Windows platforms then, and keep the current text on Linux and other platforms (where we assume GNOME guidelines are the right ones to follow, in absence of more specific ones)?
That's a very small change on your proposed patch. :-)

The link of Peter Sikking blog you added in the wiki page basically says this:
« The lesson from all this is that there is no other choice then to grit your teeth, comply with the law (UI guidelines), traditions and mores of each desktop platform and focus on doing the right thing for users within the context of each platform. »
Comment 11 Jehan 2013-12-04 11:29:18 UTC
Created attachment 263498 [details] [review]
handle migration on OSX of config folder of GIMP 2.8.2.

Attached a patch handling the migration of OSX config from GIMP 2.8.2. It will search in the right order, first the new emplacement, then in ~/Library and finally in the home, since this is the order that GIMP followed on the OSX platform. This way, no version is left out, and there is never a need for manual update.

I rewrote fully the patch and basically did not use the code from the fork though, first because it had a different logics (apparently they were patching the config dir already in GIMP 2.6), but in particular because it was doing some terrible things (like moving the older config folders!) and was not using OSX standard code.
Comment 12 Michael Natterer 2013-12-04 20:26:48 UTC
Comment on attachment 263498 [details] [review]
handle migration on OSX of config folder of GIMP 2.8.2.

I think this one is fine to push to both 2-8 and master.
Comment 13 Michael Natterer 2013-12-04 20:27:45 UTC
Generally, I think we should use --author=skl for these patches unless
they are massively changed, in order to preserve credit.
Comment 14 Jehan 2013-12-04 22:50:37 UTC
I agree about authorship and I was planning to do so. But the 2 patches I fixed until now were so heavily modified that they have no much in common with the original proposition. This one for instance has not a single common line of code. Also even logics is very different because it seems Simone had changed the configuration folder in 2.6 already (and she never cared about not breaking code for other platforms since she compiled only for OSX). Actually I even think she will want to patch over our patch again now to handle her former patched GIMP 2.6 config folder.
Her patchs were apparently really not meant to be contributed upstream.

So I think I will just leave a note in the commit description.

commit 7ef45be8902a4b7403d1d1736ca4f7c95b6449a8
Author: Jehan <jehan@girinstud.io>
Date:   Thu Dec 5 00:20:32 2013 +1300

    app: handle migration on OSX of GIMP 2.8.2's config folder.
    
    GIMP 2.8.2's config was "~/Library/Gimp/x.y", before it got moved to
    "~/Library/Application Support/x.y" and after being saved in the home
    like other UNIXes. The migration code will now check all 3 places in the
    right order on OSX.
    Thanks to Simone Karin Lehmann for the original proposition.
Comment 15 Michael Natterer 2013-12-04 23:20:18 UTC
Of course. I just wanted to mention it because I sometimes forget myself.
Comment 16 Jehan 2013-12-05 02:21:20 UTC
By the way, I changed the wiki page display (a table, as requested) and commented a whole bunch of the patches
http://wiki.gimp.org/index.php/Hacking:Merging_lisanet.de_fork#List_of_Patches

So now there are most of the patches that you can decide on without checking the code. I just wrote down what they do, but most of them, I don't know if that is a good change or not (like changing some shortcuts, adding new actions, etc.). So it needs someone else's input to decide if we want these.
Comment 17 Jehan 2016-02-13 14:41:38 UTC
No news for more than 2 years. Do we care or not? I would look closer at these if I had a MacOSX machine to check if any of these patches made sense. But since I don't, I am relying on people who work with OSX.

So I remind I listed the patches with some comments and the state of merge/discard last I checked:
http://wiki.gimp.org/index.php/Hacking:Merging_lisanet.de_fork#List_of_Patches

Actual patches can be read there:
https://sourceforge.net/p/gimponosx/code/HEAD/tree/GimpPorts/ports/graphics/gimp2/files/

There don't seem to have been much changes since then (some in 2014, that's all). Now I could check what are the changes and update the list — if needed — in the wiki page, but not if it just rots for 2 more years.
So do we care at the changes of this fork, or should we just close the ticket? :-/
Comment 18 GNOME Infrastructure Team 2018-05-24 14:12:13 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gimp/issues/514.