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 338002 - code cleanup, gif.c
code cleanup, gif.c
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Plugins
git master
Other All
: Normal enhancement
: 2.4
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2006-04-10 18:40 UTC by Clarence Risher
Modified: 2006-05-03 14:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
replacement plug-ins/common/gif.c (70.70 KB, text/x-csrc)
2006-04-10 18:59 UTC, Clarence Risher
  Details
diff -u1 -bp (11.39 KB, patch)
2006-04-10 19:54 UTC, Clarence Risher
needs-work Details | Review
replacement plug-ins/common/gif.c (72.69 KB, patch)
2006-04-11 17:11 UTC, Clarence Risher
committed Details | Review
diff -u1 -bp (19.26 KB, patch)
2006-04-11 17:12 UTC, Clarence Risher
none Details | Review

Description Clarence Risher 2006-04-10 18:40:19 UTC
neater code in plug-ins/common/gif.c

Other information:
(attachment to follow)
Comment 1 Clarence Risher 2006-04-10 18:59:42 UTC
Created attachment 63188 [details]
replacement plug-ins/common/gif.c

This replacement gif.c is functionally identical to the original.  I have gone through and tried to update it to follow the Hackordnung, as well as correcting for internal consistency where possible.  In a few places with wildly varying style I settled for local consistency between nearby lines of the same type.

This cleanup was suggested as a first step before submitting more feature patches to this file.
Comment 2 Clarence Risher 2006-04-10 19:54:49 UTC
Created attachment 63196 [details] [review]
diff -u1 -bp

This is the diff from the old version to the new one.  I used -b because without ignoring whitespace changes diff spits almost the whole file back out due to my tab->space conversion.
Comment 3 Sven Neumann 2006-04-11 12:40:49 UTC
This change is based on the gimp-2-2 branch. For this reason we cannot accept it. We need a patch against CVS HEAD.
Comment 4 Clarence Risher 2006-04-11 17:11:42 UTC
Created attachment 63247 [details] [review]
replacement plug-ins/common/gif.c

Updated replacement for gif.c from cvs HEAD

Once again, local inconsitencies remain.  I have applied all of the solid written rules that I can find, and some unwritten ones gleaned from other more authoritative source files, but a lot is still subjective.  I especially disliked the rule changing this:

foo ||
bar

to this:

foo
|| bar
Comment 5 Clarence Risher 2006-04-11 17:12:55 UTC
Created attachment 63248 [details] [review]
diff -u1 -bp

diff of the above patch, -u1 -bp to make for easy reading.
Comment 6 Sven Neumann 2006-04-12 07:09:54 UTC
Where's the rule saying that

foo ||
bar

should be changed to this:

foo
|| bar

??

We usually use the first style in the GIMP code.
Comment 7 Sven Neumann 2006-04-13 11:18:16 UTC
There is more cleanup needed before the file fully conforms to our coding standards, but this is a nice step into that direction. I have replaced the file in the HEAD branch with the attached version. Closing as FIXED.

2006-04-13  Sven Neumann  <sven@gimp.org>

        * plug-ins/common/gif.c: code cleanup by Clarence Risher
        (bug #338002).

Comment 8 Sven Neumann 2006-05-03 14:53:42 UTC
Looks like this change introduced a regression, see bug #339865. We might have to revert it.
Comment 9 Sven Neumann 2006-05-03 14:59:50 UTC
Or maybe not because bug #339865 deals with a problem to load a GIF file while this change only affected the save plug-in...