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.
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);
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.
Thanks list!
