geany
Add bracket colors plugin
Bracketcolors plugin
- Color { }, [ ], ( ) based on nesting order to make it easier to see the start/end bracket
- Ignore brackets in non source code (strings, comments, doc strings, etc.)
- Language agnostic
How it works
Allocates 3 indicators (starting at INDICATOR_IME - 3) to color each bracket pair as determined by SCI_BRACEMATCH. Ignore non source brackets as determined by highlighting_is_code_style
Dependencies
- Geany 1.38
- Need
INDICATOR_IMEfrom updated scintilla release
- Need
- C++17
- Mainly for
std::map
- Mainly for
Demo
I updated the commit message with a reference to that issue. Raindow brackets sounds cool, I wish I knew of that issue before starting this
While "rainbow colours" sounds nice "Bracketcolors" is more descriptive, so a user looking for it is more likely to find it.
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,
| ^~~~~~~~~~~~~~~
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
@eht16 I incorporated your suggested changes. I'm still learning Github, I apologize if I'm not navigating this pull request correctly 😅 (at my work, I'm used to Gitlab)
@eht16 I incorporated your suggested changes. I'm still learning Github, I apologize if I'm not navigating this pull request correctly sweat_smile (at my work, I'm used to Gitlab)
No worries, it's fine. Thanks for the changes. The PR looks good to me and I would merge it in a few days if nobody objects.
Nah, it happens that people learning Git can stuff it up so bad its easier to start again, no problem.
But unfortunately that disconnects the discussion.
Instead of breaking the flow, just force-push your branch: git push -f origin bracketcolors_v1. You probably can even revive this one by simply reopening it, and if GitHub isn't smart enough, there's a bunch of tricks we could do to force it's hand.
Nah, it happens that people learning Git can stuff it up so bad its easier to start again, no problem.
Well, it usually is not easier, but people just don't know it 😅
Nah, it happens that people learning Git can stuff it up so bad its easier to start again, no problem.
Well, it usually is not easier, but people just don't know it :sweat_smile:
Thats learning :grin:
Instead of breaking the flow, just force-push your branch:
git push -f origin bracketcolors_v1. You probably can even revive this one by simply reopening it, and if GitHub isn't smart enough, there's a bunch of tricks we could do to force it's hand.
This is what I did, along with squashing several commits similar to this. I made the mistake of forgetting that I pressed the "sync with upstream" button (due to a warning I saw on the UI), and I ended up including (and taking ownership) of the upstream commits. Since the commits were all squashed together, I couldn't see a way to cleanly git revert. I then panicked and renamed that branch 'bracketcolors_v1_broken' and started over again with a new 'bracketcolors_v1' branch where I cherry picked the relevant commits I cared about from an older tree. But that ended up closing this pull request 💀
If any of yall can somehow reverse that mess, I owe you a beer.
If any of yall can somehow reverse that mess, I owe you a beer.
Challenge accepted, I'll see if I can convince GitHub quickly :beer: Otherwise don't worry, it's not the end of the world
@asifamin13 actually I can't do it because I don't have permission to push to your branch.
If you want to try it out, I would think you'd have to do this:
- Close #1241 (GitHub doesn't like 2 PRs referring to the same branch)
- Recover this PR with
git push -f asifamin13 1e9a8b0c32d15c27ba448e689545c0bc5c9b97aa:bracketcolors_v1(assuming the remote to your own fork is asifamin13) - Reopen this PR
- Re-add your latest changes:
git push -f asifamin13 bracketcolors_v1.
Do not pull in between the steps, as step 2 is restoring your old state temporarily for GitHub's sake, and step 4 is trying to re-put your new changes there.
But again, if you're feeling out of your depth, there are two options:
- allow maintainers to push to your PR
- don't worry and accept that it's the cost of learning
@b4n I added you as a collaborator, let me know if there are any problems.
I had some trouble with step 2
fatal: bad object 1e9a8b0c32d15c27ba448e689545c0bc5c9b97aa
fatal: the remote end hung up unexpectedly
I'm a bit out of my depth but I'm willing to learn. Where does that hash come from?
@b4n I added you as a collaborator, let me know if there are any problems.
Here you go, that PR is restored and should match what you had in #1241, :beers: You can now push whatever you need (including force-pushing if necessary) and it should properly update this PR.
I had some trouble with step 2
fatal: bad object 1e9a8b0c32d15c27ba448e689545c0bc5c9b97aa fatal: the remote end hung up unexpectedlyI'm a bit out of my depth but I'm willing to learn. Where does that hash come from?
1e9a8b0c32d15c27ba448e689545c0bc5c9b97aa was the commit this PR was at (as seen in the commits history before I force-pushed, and as you can see in the force-push notification above). The idea was to restore the branch to the state it had when this PR was closed so GitHub sees it as matching this PR, and accept re-opening this PR. If you didn't have that hash anymore, it means that Git's garbage collection got rid of it on your end (as it was an old commit not referenced by anything anymore); you could have gotten it back by fetching it from GitHub (as I did myself, git fetch asifamin13 1e9a8b0c32d15c27ba448e689545c0bc5c9b97aa).
@b4n you are a life saver, your next beer is on me 🍻
I didn't realize git had garbage collection, I'll be sure to git fetch next time
This PR is back on track now 😄
Fixed a bug to address a case when nested brackets weren't getting their color updated when their parent gets updated
@asifamin13,
Fixed a bug to address a case when nested brackets weren't getting their color updated when their parent gets updated
Looks like a good improvement.
In the future, though, I would concentrate on just getting a minimal working version merged, then touch it up later in small, traceable iterations. To paraphrase the famous saying of Valéry, a pull request is never finished; it's abandoned.
To paraphrase the famous saying of Valéry, a pull request is never finished; it's abandoned.
Wise(ish) words, and its corollary, 'Every time a PR changes "somebody" has to re-check it and re-test it, and eventually "somebody" gets bored' :-)
In the future, though, I would concentrate on just getting a minimal working version merged, then touch it up later in small, traceable iterations. To paraphrase the famous saying of Valéry, a pull request is never finished; it's abandoned.
Understood. What it currently on this branch is the "minimal working version". There is one more feature I want to add (user defined colors via preferences menu/config file). I'm working on this now, hopefully to be finished this week. I'll keep that on a different commit
Wise(ish) words, and its corollary, 'Every time a PR changes "somebody" has to re-check it and re-test it, and eventually "somebody" gets bored' :-)
I'm fine being that "somebody" haha I've had fun writing this plugin.
Do new plugin PRs getting pulled in on the next geany-plugins release?
I've been daily driving this plugin for several months now. I think it's stable enough to be merged.
The latest CI build failure seems to be from misconfigured runners.
The CI was awaiting a manual approval to run, approved, lets see.
Ideally needs someone who isn't the author to test it, nothing personal, but we all know how badly we test our own software :-)
Hmmm, the other CI builds seem to work without a problem. For some reason this one isn't getting picked up:
This request was automatically failed because there were no enabled runners online to process the request for more than 1 days.
Ideally needs someone who isn't the author to test it, nothing personal, but we all know how badly we test our own software :-)
haha I understand. I'm always open to feedback if yall want to give it a try 😜
Hmmm, the other CI builds seem to work without a problem. For some reason this one isn't getting picked up:
This request was automatically failed because there were no enabled runners online to process the request for more than 1 days.
Maybe this is related to the deprecated Ubuntu 18.04 build configuration in the workflow. Could you try to rebase your branch on current GIT master? The workflow/CI configuration has been changed a lot in the last weeks. With a bit of luck, the CI will finally work then :).
@eht16 I did the rebase, verified it compiles/works as expected on my rhel 8 workstation. Shall we give the CI another shot?
@eht16 I did the rebase, verified it compiles/works as expected on my rhel 8 workstation. Shall we give the CI another shot?
The CI is fine. Though you added quite some new code in the last commit which should be reviewed.
Btw, what is the "#5" referring to you mentioned in the commit message?