[Geany-devel] Geany Plugin with Libxml

[Chow Loong Jin]

On Monday 10,August,2009 05:56 PM, Cédric Tabin wrote:
> Hi Franck,
> 
> I put my plugin where you said already =>
> http://geany-plugins.svn.sourceforge.net/viewvc/geany-plugins/trunk/pretty-printer/
> I'll wait for your commit of the build system, so I can try it on my
> computer :-)

Hi Cédric,

I've just integrated your plugin into the Autotools build system.
However, your plugin still does not compile. Here are the issues
preventing it from compiling:
1. Declaration of variables inside a for loop
In C++, we like this:
for (int i = 0; ...

In C, we don't. We do this:
int i;
for (i = 0; ...

2. Bad #include.
We don't like to #include .c files. I suspect this is a typo, which
causes multiple definitions of a function, resulting in problems
linking. See line 27 of pretty-printer/src/Plugin.c: #include
"PrettyPrinter.c".

3. Missing decls for TRUE and FALSE.
TRUE and FALSE are defined in the header "glib/gmacros.h", found in
/usr/include/glib-2.0 on Debian and its derivatives. I suspect you
forgot to include this in PrettyPrinter.c

A minor issue that should be fixed anyway:
1. Usage of the "boolean" type
Rather than #define boolean int, how about using the gboolean datatype,
found in the header "glib/gtypes.h", found in /usr/include/glib-2.0?

Now for some cosmetic issues:
1. Long lines
Long lines are a pain to read and destroy the entire purpose of
indentation if they wrap. I believe the general consensus is 80
characters per line. I'd suggest reducing your lines to fit that length,
and chopping some lines up into smaller lines.

Trivia: A dot matrix printer can only have 80 characters in a line
before they get wrapped!

2. Copyright headers
Your files have a license header, which is good. However, for GPL files,
it is generally customary to have something like this:

    Copyright (C) <year>  <name of author>

    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
    the Free Software Foundation; either version 2 of the License, or
    (at your option) any later version.

    This program is distributed in the hope that it will be useful,
    but WITHOUT ANY WARRANTY; without even the implied warranty of
    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
    GNU General Public License for more details.

    You should have received a copy of the GNU General Public License along
    with this program; if not, write to the Free Software Foundation, Inc.,
    51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.

This block is found near the end of the GPL license text. A copy can be
found in /usr/share/common-licenses/GPL-2 on Debian and its derivatives.



I would have fixed the compilation issues for you, but I figured that
since you mentioned you'd like to hear suggestions so you can learn, it
would be good for you to fix these yourself. :-)

-- 
Kind regards,
Chow Loong Jin

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 261 bytes
Desc: OpenPGP digital signature
URL: <http://lists.geany.org/pipermail/devel/attachments/20090810/1005079f/attachment.pgp>