[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
Any chance this could make it into geany-plugins v2.0? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1754053448 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
It's from [the pull request](https://github.com/asifamin13/geany-plugins/pull/5) on my fork. I'd appreciate any feedback, I added some GUI elements to specify custom colors on the plugin preferences page. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1627764648 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
> @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? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1627750676 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@eht16 I did the rebase, verified it compiles/works as expected on my rhel 8 workstation. Shall we give the CI another shot? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1622136258 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@asifamin13 pushed 2 commits. 694d69d7402125f91a567734c16828f67978d0b2 Bracket Colors 44de64d1050b2f32c574c842e7ffdc80dda59084 Bracket Colors: Support user defined colors (#5) -- View it on GitHub: https://github.com/geany/geany-plugins/pull/1221/files/f176f5c7713be823f03a172393361b8a8fd61e9f..44de64d1050b2f32c574c842e7ffdc80dda59084 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
> 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 :). -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1585543342 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
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 -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1577436751 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
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 :-) -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1555406533 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
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](https://github.com/geany/geany-plugins/actions/runs/4631643800) seems to be from misconfigured runners. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1554852206 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
I added support for loading/saving settings via a `GKeyFile` -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1522541119 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@asifamin13 pushed 1 commit. f176f5c7713be823f03a172393361b8a8fd61e9f Bracket Colors: Support user defined colors (#5) -- View it on GitHub: https://github.com/geany/geany-plugins/pull/1221/files/7bef4fd162d61057ed8df89d9a47a6473e89830e..f176f5c7713be823f03a172393361b8a8fd61e9f You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
> 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](https://quoteinvestigator.com/2019/03/01/abandon), 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? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1513502773 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
> To paraphrase [the famous saying of > Valéry](https://quoteinvestigator.com/2019/03/01/abandon), 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' :-) -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1499873849 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@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. [the famous saying of Valéry]: https://quoteinvestigator.com/2019/03/01/abandon -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1499867052 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
Fixed a bug to address a case when nested brackets weren't getting their color updated when their parent gets updated -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1499424966 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@asifamin13 pushed 1 commit. 48fdf9e06f6ebdbb6ba7af8d9e649927a9173469 Bracket Colors -- View it on GitHub: https://github.com/geany/geany-plugins/pull/1221/files/50ae4036e73836f54a37d4636d2ecfc448eeac22..48fdf9e06f6ebdbb6ba7af8d9e649927a9173469 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@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 -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1475581774 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@asifamin13 pushed 1 commit. 50ae4036e73836f54a37d4636d2ecfc448eeac22 Bracket Colors -- View it on GitHub: https://github.com/geany/geany-plugins/pull/1221/files/e8a1d9845fa2a6976420a678df812aaa53207614..50ae4036e73836f54a37d4636d2ecfc448eeac22 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
> @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 > > ```shell > 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? 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`). -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1469634100 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
Reopened #1221. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#event-8752871523 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@b4n I added you as a collaborator, let me know if there are any problems. I had some trouble with step 2 ```shell 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? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1469247318 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@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: 1. Close #1241 (GitHub doesn't like 2 PRs referring to the same branch) 2. Recover this PR with `git push -f asifamin13 1e9a8b0c32d15c27ba448e689545c0bc5c9b97aa:bracketcolors_v1` (assuming the remote to your own fork is *asifamin13*) 3. Reopen this PR 4. 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 -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1467655572 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
> 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 -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1467644009 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
> 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](https://stackoverflow.com/questions/5667884/how-to-squash-commits-in-git-after-they-have-been-pushed). 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. 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. But that ended up closing this pull request If any of yall can somehow reverse that mess, I owe you a beer. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1467320646 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
> > 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: -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1465349693 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
> 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 -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1465191272 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
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. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1465190977 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
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. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1465065800 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
> ? why removed To get [your attention](https://github.com/geany/geany-plugins/pull/1241#issuecomment-1464731699), it seems. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1465062083 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
? why removed -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1464723023 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
Closed #1221. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#event-8723027077 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@asifamin13 pushed 1 commit. 7cd6454107f45540a1f62f4d471d78fecdd9583c Bracketcolors v1 codereview 3/10/23 (#3) -- View it on GitHub: https://github.com/geany/geany-plugins/pull/1221/files/0850594b6132956e8a813db85020473e33c7aa6a..7cd6454107f45540a1f62f4d471d78fecdd9583c You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@asifamin13 commented on this pull request.
> +static gchar char_at(ScintillaObject *sci, gint pos)
+/*
+
+-
*/
+{
+return sci_get_char_at(sci, pos);
+}
When I was writing this, I was using the `autoclose` plugin as a template
[which has this wrapper as
well](https://github.com/geany/geany-plugins/blob/dc6980e5df24434597b37a5a7064837b3f5158e8/autoclose/src/autoclose.c#L93).
I suspect the authors of `autoclose` used the wrapper for convenience since
it's used a lot, I don't really need it as bad to be honest.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1221#discussion_r1132943357
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@b4n commented on this pull request.
> +static gchar char_at(ScintillaObject *sci, gint pos)
+/*
+
+-
*/
+{
+return sci_get_char_at(sci, pos);
+}
Just curious: why the wrapper?
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1221#pullrequestreview-1333787397
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
> @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. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1427055544 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@eht16 commented on this pull request. > @@ -0,0 +1,35 @@ +# = I don't know if there is a better way :(. Asked primarily for curiosity. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#discussion_r1103817412 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@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) -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1418276063 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
Reopened #1221. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#event-8441755413 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
>
> 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 >' [-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:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
Closed #1221. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#event-8441754664 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@asifamin13 commented on this pull request. > @@ -0,0 +1,32 @@ +Color brackets, parenthesis, and braces +=== thanks, fixed! -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#discussion_r1096813044 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@asifamin13 commented on this pull request.
> +// nested bracket
+orderStack.push(endPos);
+}
+
+GetOrder(bracket) = orderStack.size() - 1;
+}
+}
+
+
+
+//
-
+void BracketMap::Show()
+/*
+
+-
*/
+{ g_debug("%s: Showing bracket map ...", __FUNCTION__);
they aren't needed anymore. I removed the commented ones as well
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1221#discussion_r1096812976
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@asifamin13 commented on this pull request. > @@ -0,0 +1,35 @@ +# = yes, is there a better way to handle this? I'm new to the autotools world, so any recommendations would be appreciated -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#discussion_r1096812891 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@asifamin13 commented on this pull request.
> +
+static Length& GetLength(Bracket ) {
+return std::get<0>(bracket);
+}
+static Order& GetOrder(Bracket ) {
+return std::get<1>(bracket);
+}
+
+static const Length& GetLength(const Bracket ) {
+return std::get<0>(bracket);
+}
+static const Order& GetOrder(const Bracket ) {
+return std::get<1>(bracket);
+}
+};
+
this was from a standard template I use, fixed
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1221#discussion_r1096812788
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@asifamin13 commented on this pull request. > @@ -0,0 +1,143 @@ +/* + * BracketMap.cc + * + * Copyright 2013 Asif Amin thanks for catching this, fixed! -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#discussion_r1096812623 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@asifamin13 pushed 1 commit. 0ac738a0532a8638bbe9d9bd55aed0eac671df78 Bracket Colors, initial code review -- View it on GitHub: https://github.com/geany/geany-plugins/pull/1221/files/b75167c3c1d56d476abc06458a84ee84b7d907ca..0ac738a0532a8638bbe9d9bd55aed0eac671df78 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@eht16 commented on this pull request.
> +// nested bracket
+orderStack.push(endPos);
+}
+
+GetOrder(bracket) = orderStack.size() - 1;
+}
+}
+
+
+
+//
-
+void BracketMap::Show()
+/*
+
+-
*/
+{ g_debug("%s: Showing bracket map ...", __FUNCTION__);
Should those `g_debug` statements remain in the code? Also the commented ones?
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1221#pullrequestreview-1284256497
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
@eht16 requested changes on this pull request.
Oh, and could you please add your plugin to the list in the top README?
> @@ -0,0 +1,32 @@
+Color brackets, parenthesis, and braces
+===
RestructuredText (the text format used here) requires that underlines are
exactly as long as the heading above.
> @@ -0,0 +1,143 @@
+/*
+ * BracketMap.cc
+ *
+ * Copyright 2013 Asif Amin
2013? I guess it's a typo :).
If so, ideally change all occurrences.
> @@ -0,0 +1,35 @@
+# =
Are these two "ax_cxx_*" M4 macros copied from Geany?
> +
+static Length& GetLength(Bracket ) {
+return std::get<0>(bracket);
+}
+static Order& GetOrder(Bracket ) {
+return std::get<1>(bracket);
+}
+
+static const Length& GetLength(const Bracket ) {
+return std::get<0>(bracket);
+}
+static const Order& GetOrder(const Bracket ) {
+return std::get<1>(bracket);
+}
+};
+
Why so many empty lines?
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1221#pullrequestreview-1284250581
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
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 >' [-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,
| ^~~
```
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/1221#issuecomment-1418077202
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
While "rainbow colours" sounds nice "Bracketcolors" is more descriptive, so a user looking for it is more likely to find it. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1416876357 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
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 -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1416857959 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany-plugins] Add bracket colors plugin (PR #1221)
Perhaps the description and/or commit message should mention #1148? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany-plugins/pull/1221#issuecomment-1416838077 You are receiving this because you are subscribed to this thread. Message ID:
