codebrainz commented on this pull request.


> @@ -2289,6 +2290,32 @@ static gint get_fold_header_after(ScintillaObject 
> *sci, gint line)
 }
 
 
+/* returns the line after following all brace match for @brace on @line */
+static gint resolve_matching_braces(ScintillaObject *sci, gint line, gint 
brace)
+{
+       gint pos = sci_get_position_from_line(sci, line);
+       gint line_end = sci_get_line_end_position(sci, line);
+       gint lexer = sci_get_lexer(sci);

> I find this overlong precondition hides the logic of the loop (condition and 
> post-body).

You could insert another line break instead of tucking it at the end of the 
last initializer line.

> Anyway, the new code is different, do you still think I should so something 
> like that now the 2 other variables are constant?

My opinion is that doing so should be the default unless there is a reason not 
to, for example to keep it out of the loop for performance reasons, or I 
suppose in this case to have them be `const`. That being said, using `const` 
like that is also not used often in Geany and is something else that should 
either be done all the time or not. I think we should update `HACKING` to 
indicate both of these (though I'm not convinced we want to sprinkle `const` 
everywhere, personally).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/1280

Reply via email to