[Geany-devel] What's with the for loop in get_brace_indent() ?

Lex Trotman elextr at xxxxx
Sat Jul 10 01:01:54 UTC 2010

On 10 July 2010 03:47, Jason Oster <jason.oster at campnavajo.com> wrote:
> Greetings list!
> While doing some preliminary work on a simple patch, I noticed some
> interesting code in editor.c:get_brace_indent() svn r5100.
> It starts out simple enough, with a for loop iterating over the full length
> of the current line of text at the caret position.  The first thing tested
> though does not make sense to me:
>> if (linebuf[i] == '{' && i == (len - 1))
> It checks the character currently being iterated through, and *then*
> requires that it's the last character in the line.  :(
> Assuming this fails (the last character in the line is not '{'), the code
> does something else that is strange:
>> gint k = len - 1;
>> while (k > 0 && isspace(linebuf[k])) k--;
> If finds the last non-white space character in the line ...
>> if (linebuf[k] == '{')
> ... and verifies that it's an opening curly brace.  And this all happens
> within a for loop that iterates forward through linebuf[].
> "But that's not all!"
> The for loop is unconditionally broken on the first iteration!
> As a result, the for loop is useless, the ret++ is useless (the function
> only returns 0 or 1), and the first check (against the last character) is
> useless.  As a result, I recommend the following patch to clean up this
> function.

This keeps it working as is but I believe that the current operation is wrong.

The requirement that the brace be the last non-whitespace character is
there for a specific Perl idiom (according to the comment) but it
causes a problem that indenting doesn't work if there is anything on a
line after the {, for example a comment, or code with a different
style that allows statements after braces.  So in all these cases I
have to adjust the indenting manually.  This has been annoying me for
some time but I havn't had time to look for it.

Brace indenting shouldn't be depending on coding style (there is lots
of old code and I'm not going to re-style it all).

It should check for an unmatched non-comment (check the sytle) brace.
That will fix the perl problem as well.

> I left the return value gint, rather than making it a gboolean.  The only
> code I know of that uses this function
> (editor.c:get_indent_size_after_line()) multiplies the return value against
> the indent width.  On the other hand, what the heck?  I might just opt to
> replace that usage with something like this:
>> size += (get_brace_indent(sci, line) ? iprefs->width : 0);

I'd recommend making it gboolean if you do this for clarity

> That's actually kind of pretty.  ;)  Does anyone know if get_brace_indent()
> has other uses? It doesn't look like it's exported to plugins, for example,
> and grep doesn't find anything else.

I'd consider grep definitive, so long as you searched for the right thing. :-)


> Thanks list!
> _______________________________________________
> Geany-devel mailing list
> Geany-devel at uvena.de
> http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel

More information about the Devel mailing list