codebrainz commented on this pull request.


> @@ -2577,15 +2565,20 @@ void build_save_menu(GKeyFile *config, gpointer ptr, 
> GeanyBuildSource src)
                                g_key_file_remove_key(config, build_grp_name, 
"error_regex", NULL);
                        if (pj->priv->build_filetypes_list != NULL)
                        {
-                               data.config = config;
-                               data.ft_names = g_ptr_array_new();
-                               
g_ptr_array_foreach(pj->priv->build_filetypes_list, foreach_project_filetype, 
(gpointer)(&data));
-                               if (data.ft_names->pdata != NULL)
+                               GPtrArray *ft_names = g_ptr_array_new();
+                               guint i;
+                               
+                               foreach_ptr_array(ft, i, 
pj->priv->build_filetypes_list)

IMO this would read better as:

```c
GPtrArray *build_fts = pj->priv->build_filetypes_list;
for (guint i = 0; i < build_fts->len; i++) {
    ft = build_fts->pdata[i];
    /// other code here...
}
```

so it doesn't hide code behind a macro. It's basically the same amount of code, 
and index iteration is something everyone already knows without thinking about 
it, unlike the macro that is named like a function.

-- 
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/2270#pullrequestreview-278747519

Reply via email to