Difference between revisions of "Hacking:Merging lisanet.de fork"
(→List of Patches)
(→List of Patches: gimp-user-install.c patch proposed)
|Line 89:||Line 89:|
| Ah! This one looks interesting. It adds some migration code for the config in ~/Library/GIMP (apparently 2.8.2) to /Library/Application Support/GIMP (since 2.8.4).
| Ah! This one looks interesting. It adds some migration code for the config in ~/Library/GIMP (apparently 2.8.2) to /Library/Application Support/GIMP (since 2.8.4). and !
Revision as of 11:35, 4 December 2013
We had a quick check with Mitch and saw that many are outdated with current code. For instance some about a switch to Carbon->Cocoa platforms already in our code tree, or others obsoleted by our menu replacement. Others seem to make sense like the "color display change".
This is a working page for GIMP developers to go through the list and decide what we want to integrate and what we want to leave out.
1/ $ svn checkout svn://svn.code.sf.net/p/gimponosx/code/ gimponosx-code
2/ $ cd gimponosx-code/GimpPorts/ports/graphics/gimp2/files/
3/ Check the patches and update the below lists to keep or not. For instance, if you reviewed a patch and it is not good for us, you can leave a "[exclude]" next to it. If we want to keep it but it needs work, add "[need work]", and so on.
4/ I set up a Bugzilla report. When a patch seems good, make a `git format-patch` and upload it there for review if needed. :-) We can close this report when all the patches of the list will have been either applied in our tree or discarded. :-)
List of Patches
|app-actions-dialogs-actions.c.diff|| Answer: Yes, change to 'About GIMP'.
The specs supersede what other programs do.
The OS X human interface guidelines, Windows human interface guidelines provide for 'About GIMP'. The GNOME human interface guidelines uses only 'About'. More human interface guidelines are listed at Wikipedia.
Peter Sikking advices in a blog posting about cross-platform development to use the platform specific specs.
|Patch in progress (scl)|
|app-actions-file-actions.c.diff||Add "file-hide" ("<primary>H") and "file-hide-others" ("<primary><alt>H") actions. Are they common OSX actions?|
|app-actions-file-commands.c.diff||The code (OSX specific, apparently using Cocoa headers) for the new "file-hide" and "file-hide-others" actions.|
|app-actions-file-commands.h.diff||Headers for the new "file-hide" and "file-hide-others" actions|
|app-actions-Makefile.in.diff||Makefile.in are generated. See the Makefile.am patch.||discarded|
|app-actions-plug-in-actions.c.diff||Change "Repeat Last" action's shortcut from "<primary>F" to "<Alt>F" and "Re-show Last" from "<primary><shift>F" to "<Alt><shift>F". Are they shortcuts which make more sense on an OSX platform maybe? No idea.|
|app-actions-view-actions.c.diff||Changing Fullscreen actions from default F11 to "<primary>F" shortcut. Is it an OSX standard?|
|app-config-gimpcoreconfig.c.diff||Some code relative to the save-xcf-only config added on this fork.|
|app-config-gimpcoreconfig.h.diff||Some code relative to the save-xcf-only config added on this fork.|
|app-config-gimpdisplayconfig.c.diff||Changing DEFAULT_MARCHING_ANTS_SPEED from 200 to 600. Is the current default too slow?|
|app-config-gimprc-blurbs.h.diff||Blurb for the save/export changes: "Uncheck to allow all image formats to be saved via the 'File - Save' menu entry"|
|app-core-gimpimage.c.diff||Code relative to the save/export changes.|
|app-core-gimpimage.h.diff||Code relative to the save/export changes.|
|app-core-gimp-user-install.c.diff||Ah! This one looks interesting. It adds some migration code for the config in ~/Library/GIMP (apparently 2.8.2) to /Library/Application Support/GIMP (since 2.8.4). Code checked and does some bad things (like moving config folder of older versions!) so I basically rewrote it fully.||Patch proposed|
|app-dialogs-preferences-dialog.c.diff||Adding a preference option for the save-xcf-only change + change to the help browser option, where the web browser is apparently always used if GIMP help browser is not installed. The save/export option is not of interest. But maybe this help browser change makes any sense?|
|app-file-file-open.c.diff||Disable some check which looks for a "icc-profile" parasite on our image. Something to look into why she did this?|
|app-file-file-save.c.diff||Some code related to the save/export changes.|
|app-gui-gui.c.diff||Looks like some kind of menu grouping change. I guess someone with OSX platform knowledge will know better what it is.|
|app-gui-gui-unique.c.diff|| Looks interesting, though I guess some OSX dev would need to look closer. Apparently some desktop integration. I copy-paste an in-code comment:
/* The application delegate handles some events * - terminate GIMP * - open files when GIMP is already running by dropping on the icon or Finder's 'Open with' * The event handler * - handles open events when GIMP is launched by dropping on the icon or Finder's 'Open with' * * Now Cocoa based. */
|app-gui-Makefile.in.diff||Makefile.in are generated. See the Makefile.am patch.||discarded|
|app-Makefile.in.diff||Makefile.in are generated. See the Makefile.am patch.||discarded|
|app-menus-menus.c.diff||Not sure. Apparently loading some additional menu or something?|
|app-widgets-gimphelp.c.diff|| Some change in the help browser apparently, and apparently better integration to OSX locales, or something. Copy-paste from code:
/* on OS X there's no need for config->help_locales. The user can handle this * via System Preferences. Even if she wants to use a different help locale than the * default language. Just add this alternative locale in System Preferences and * install the corresponding User manual. */
|app-widgets-Makefile.in.diff||Makefile.in are generated. See the Makefile.am patch.||discarded|
|configure.diff||configure is generated. See the configure.ac patch.||discarded|
|etc-gimprc.diff||Add a config option to allow to save in any format.|
|lcms.c.diff||See plug-ins-common-lcms.c.diff (duplicate).||discarded|
|libgimpbase-gimpenv.c.diff|| Just does this change:
-#include <AppKit/AppKit.h> +#import <Foundation/Foundation.h> Not sure why is that.
|libgimp-gimpui.c.diff||Removes a signal handling on "show" for a window that is made transient. Not sure what this signal handling was supposed to do ([NSApp arrangeInFront: nil] -> pushing it to the top of window layers?).|
|libgimp-Makefile.in.diff||Makefile.in are generated. See the Makefile.am patch.||discarded|
|libgimpthumb-gimp-thumbnail-list.c.diff|| Add support for Freedesktop's Thumbnail standard to this file. We already added support to another file as of bug 646644. It seems we may have forgotten a part of the code. :-)
I opened bug 719830 for this, with a patch based on our existing core code..
|patch to be reviewed|
|libgimpthumb-gimpthumb-utils.c.diff|| Just does this change:
-#include <AppKit/AppKit.h> +#import <Foundation/Foundation.h> Not sure why is that.
|libgimpwidgets-gimpdialog.c.diff||Position a dialog at display center as default.|
|libgimpwidgets-Makefile.am.diff-old.diff||Same as libgimpwidgets-Makefile.am.diff with fixed tabulation.||discarded|
|libgimpwidgets-Makefile.in.diff||Makefile.in are generated. See the Makefile.am patch.||discarded|
|Makefile.in.diff||This patch tries to disable gtk-doc on a `distcheck` target.||discarded|
|modules-display-filter-lcms.c.diff||Looks interesting. If nothing else because it seems this piece of code has been forgotten and is still using Carbon on our master (at least there is a remaining Carbon.h include)!|
|modules-Makefile.in.diff||Makefile.in are generated. See the Makefile.am patch.||discarded|
|plug-ins-common-file-tiff-load.c.diff||See comment on plug-ins-file-jpeg-jpeg.c.diff.||discarded|
|plug-ins-common-file-tiff-save.c.diff||See comment on plug-ins-file-jpeg-jpeg.c.diff.||discarded|
|plug-ins-common-lcms.c.diff|| Slight changes in icc profile handling. One of them is a fallback if lcms_image_get_profile() fails or maybe if it was deleted because not RGB. When it happens, she would extract the profile with the sips command line through system() in a temporary file and loads it as the source profile. I don't know what to think about it. Does that actually mean that lcms_image_get_profile() may fail sometimes but `sips` would succeed? (in this case, why not rather patch lcms?!) Or does that mean that she wants to support some profiles other than RGB?
+ Some other logics changes. I did not review all in details.
|plug-ins-common-web-browser.c.diff||Interesting patch. It adds an OSX specific code to display an URI with the OSX `open` command (would be the standard way to get the user-specified web-browser on OSX), instead of using gtk_show_uri() (which itself uses glib's g_app_info_launch_default_for_uri(), which I think follows Freedesktop's MIME actions spec). This said, I'd think that if this change is really the right one, we want to port it upstream to glib, not just waste it on this plugin's code.|
|plug-ins-file-jpeg-jpeg.c.diff||Try to load and save back metadata (saved in a temporary empty jpeg image) by running the external command exiftool through system()! I don't think we want this, first because we have a new metadata system in master, second because that's a terrible design for just so many reasons!||discarded|
|plugins-filejpeg-jpeg.c.diff||Exactly same as plug-ins-file-jpeg-jpeg.c.diff||discarded|
|plug-ins-help-Makefile.in.diff||Makefile.in are generated. See the Makefile.am patch.||discarded|
|plug-ins-script-fu-Makefile.in.diff||Makefile.in are generated. See the Makefile.am patch.||discarded|
|po-de.po.diff||Answer: Yes, 'Über GIMP' is the proper translation. for Windows and OS X, for GNOME it is 'Über'.||patch in progress (scl)|
|save-export-conf-patch.diff||Code for the save/export changes.|
|save-export-patch.diff|| I don't think anyone has actually checked the save/export patches, but there are some written specs. If this is up to date with the patch, basically it allows to "save" images into non-XCF format if and only if the image has just
one layers. The current opinion is that this feels like "unpredictable magic". It would likely feel like really inconsistent to a user (some times you can "save" to non-XCF, sometimes no).