>
> Looks good! Thanks. > > > While "rainbow colours" sounds nice "Bracketcolors" is more descriptive, so > > a user looking for it is more likely to find it. > > Full agree. @asifamin13 maybe you could just mention "rainbow colors" in the > README, that might already suffice that users can find it also with this name. > > I did a quick code review without having a deeper look at the logic. From a > quick test, it works as advertised and I got many fancy brackets :). > > A few comments @asifamin13: > > * Could you add the plugin name in > https://github.com/geany/geany-plugins/blob/master/build/geany-plugins.nsi#L176 > (this is for the Windows installer, use `bracketcolors.dll`) > * As Geany itself also colorizes the brackets in an idle callback, did you > check that both implementations do not interfere with each other? > * I got a few compiler warnings which might be worth looking into: > > ``` > BracketMap.cc: In member function 'void BracketMap::Show()': > BracketMap.cc:122:21: warning: loop variable 'it' creates a copy from type > 'const std::pair<const int, std::tuple<int, int> >' [-Wrange-loop-construct] > 122 | for (const auto it : mBracketMap) { > | ^~ > BracketMap.cc:122:21: note: use reference type to prevent copying > 122 | for (const auto it : mBracketMap) { > | ^~ > | & > bracketcolors.cc: In function 'gboolean utils_parse_color(const gchar*, > GdkColor*)': > bracketcolors.cc:276:27: warning: 'gboolean gdk_color_parse(const gchar*, > GdkColor*)' is deprecated: Use 'gdk_rgba_parse' instead > [-Wdeprecated-declarations] > 276 | return gdk_color_parse(spec, color); > | ~~~~~~~~~~~~~~~^~~~~~~~~~~~~ > In file included from /usr/include/gtk-3.0/gdk/gdkcairo.h:26, > from /usr/include/gtk-3.0/gdk/gdk.h:33, > from /usr/include/gtk-3.0/gtk/gtk.h:30, > from /home/enrico/apps/include/geany/gtkcompat.h:30, > from /home/enrico/apps/include/geany/editor.h:27, > from /home/enrico/apps/include/geany/document.h:31, > from /home/enrico/apps/include/geany/build.h:26, > from /home/enrico/apps/include/geany/geanyplugin.h:36, > from bracketcolors.cc:40: > /usr/include/gtk-3.0/gdk/deprecated/gdkcolor.h:79:11: note: declared here > 79 | gboolean gdk_color_parse (const gchar *spec, > | ^~~~~~~~~~~~~~~ > ``` Thanks for the feedback! - The deprecated `gdk*` warning are ok. I removed the unused debugging code that was causing the others. - I added the bracketcolors entry to `geany-plugins.nsi`. - I confirmed my plugin behaves correctly when Geany colors brackets itself, like when brackets are missing -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1418273658 You are receiving this because you are subscribed to this thread. Message ID: <geany/geany-plugins/pull/1221/c1418273...@github.com>
