Re: [Github-comments] [geany/geany] highlighting: added API call to query GeanyLexerStyle by name (#2336)
elextr approved this pull request. -- 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/2336#pullrequestreview-297214791
Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)
> If it did proper analysis (ie. look into callee) it would see it's passed to > memcpy and fully written. 1. "if" :) 2. and if it, did the return_ifs can shortcut return and leave it uninitialised. -- 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-plugins/pull/916#issuecomment-538177110
Re: [Github-comments] [geany/geany] Changed default title length from 30 to 80. (#2335)
IMO, we should just remove the ellipsizing from the window title altogether. It used to be an arbitrary 30 chars, now an arbitrary 80, and there's still the directory path and project name which could be really long, it's totally dependent on window size and user's font size, and sane window managers already handle this. Moreover, the original issue on SF was an extreme edge case where the user's filename somehow had embedded in it all of the query string parameters which shouldn't even be part of a filename. -- 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/2335#issuecomment-538167465
Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)
> But or not I think setting a variable to a default is a good thing. I haven't done a proper survey, but I'd wager if we did, we'd find hundreds if not thousands of uninitialized variables in Geany and GP's code. > there is nothing to tell cppcheck that argument 2 is an out If it did proper analysis (ie. look into callee) it would see it's passed to `memcpy` and fully written. In any case, I'm fine with this PR if it shuts up `cppcheck`; this isn't performance critical in anyway, and it doesn't affect readability. -- 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-plugins/pull/916#issuecomment-538164617
Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)
codebrainz approved this pull request. -- 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-plugins/pull/916#pullrequestreview-297200041
Re: [Github-comments] [geany/geany] highlighting: added API call to query GeanyLexerStyle by name (#2336)
To be honest I dislike the idea of exposing anything colour scheme related to plugins as the current system is rather a mess due to how it evolved, and I'd very much like to be able to overhaul it - something I started some time ago, and will eventually continue with - without regard for breaking plugins (in addition to user schemes). That said, I don't want to block progress for something that will not happen any time soon, so whatever. -- 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/2336#issuecomment-538163328
Re: [Github-comments] [geany/geany] highlighting: added API call to query GeanyLexerStyle by name (#2336)
LarsGit223 commented on this pull request.
> + * @return A pointer to the style struct.
+ */
+GEANY_API_SYMBOL
+const GeanyLexerStyle *highlighting_get_named_style(const gchar *named_style)
+{
+ GeanyLexerStyle *cs;
+ gchar *comma, *name = NULL;
+
+ g_return_val_if_fail(named_style, NULL);
+ name = utils_strdupa(named_style); /* named_style must not be
written to, may be a static string */
+
+ comma = strstr(name, ",");
+ if (comma)
+ {
+ *comma = '\0'; /* terminate name to make lookup work */
+ }
I removed it. It was a leftover copied in from ```read_named_style()```. But it
doesn't make sense any more.
--
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/2336#discussion_r331244133
Re: [Github-comments] [geany/geany] highlighting: added API call to query GeanyLexerStyle by name (#2336)
@LarsGit223 pushed 1 commit. e096b0f080ebe92c4c4d9b9b442174212fe4f096 highlighting: added API call to query GeanyLexerStyle by name -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/2336/files/9ff6031fb01b9e49cd92dc3221ac21a2e34c736f..e096b0f080ebe92c4c4d9b9b442174212fe4f096
Re: [Github-comments] [geany/geany] highlighting: added API call to query GeanyLexerStyle by name (#2336)
elextr requested changes on this pull request.
The use-case makes sense now that its explained clearly.
> + * @return A pointer to the style struct.
+ */
+GEANY_API_SYMBOL
+const GeanyLexerStyle *highlighting_get_named_style(const gchar *named_style)
+{
+ GeanyLexerStyle *cs;
+ gchar *comma, *name = NULL;
+
+ g_return_val_if_fail(named_style, NULL);
+ name = utils_strdupa(named_style); /* named_style must not be
written to, may be a static string */
+
+ comma = strstr(name, ",");
+ if (comma)
+ {
+ *comma = '\0'; /* terminate name to make lookup work */
+ }
Whats this doing? If you pass a name why does it need to terminate at a comma?
Either this is functionality specific to the particular useage, in which case
it should be in the caller, or it should be documented behaviour with an
explanation of why.
--
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/2336#pullrequestreview-297130510
Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)
It would appear to be a correct warning, there is nothing to tell cppcheck that argument 2 is an out only argument so it has to assume the value is used in the function and hence an uninitialised value is being passed. -- 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-plugins/pull/916#issuecomment-538110962
Re: [Github-comments] [geany/geany-plugins] geanyprj: fixed cut-off sidebar tab under gtk3 (#917)
Merged #917 into master. -- 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-plugins/pull/917#event-2685102855
Re: [Github-comments] [geany/geany-plugins] Geanyprj's Project dock is extremely small (#744)
Closed #744 via #917. -- 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-plugins/issues/744#event-2685102869
Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)
But or not I think setting a variable to a default is a good thing. -- 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-plugins/pull/916#issuecomment-538054220
[Github-comments] [geany/geany] highlighting: added API call to query GeanyLexerStyle by name (#2336)
This PR adds a new API call to ```highlighting.c```: ``` C const GeanyLexerStyle *highlighting_get_named_style(const gchar *named_style); ``` This shall enable plugins to query styles dedicated to plugin specific highlighting. Plugins could have their own styles with dedicated keys/names and instruct users to simply add them to a colorscheme. Another option would be a configuration setting in a plugin but then the setting is separated from the colorscheme which may result in bad readability. Also see the discussion at https://github.com/geany/geany/issues/2331. You can view, comment on, or merge this pull request online at: https://github.com/geany/geany/pull/2336 -- Commit Summary -- * highlighting: added API call to query GeanyLexerStyle by name -- File Changes -- M src/highlighting.c (24) M src/highlighting.h (2) M src/plugindata.h (2) -- Patch Links -- https://github.com/geany/geany/pull/2336.patch https://github.com/geany/geany/pull/2336.diff -- 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/2336
Re: [Github-comments] [geany/geany] Scrollbar doesn't move in reloaded file after compilation (#1893)
Steps to reproduce here: 1. Open two files in Geany; switch to the tab for file B 2. "touch" file A externally, so Geany sees it as changed 3. Switch to the tab for file A in Geany 4. Click "Reload" in the info bar 5. Try scrolling (with the scroll wheel): the scrollbar doesn't update -- 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/issues/1893#issuecomment-538000140
Re: [Github-comments] [geany/geany] Scrollbar doesn't move in reloaded file after compilation (#1893)
Still reproducible here, Ubuntu 19.10 beta, Geany 1.35-1, GTK 3.24.11-1ubuntu1 -- 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/issues/1893#issuecomment-537998417
Re: [Github-comments] [geany/geany] Changed default title length from 30 to 80. (#2335)
@elextr: > There may be lots of complicated things that could be done here, but hey if a > fixed 30 has worked for most people to date then a fixed 80 is no worse. @elextr Thanks, my sentiments exactly. Eventually the hardcoded value could/should probably be moved to a defined constant. Maybe a refactoring beautification PR? FYI: this value is hardcoded twice, it's also in document.c: ``` 419 if (length < 0) 420 length = 30; ``` I left it 30 since a filename length less than zero probably doesn't need 80. -- 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/2335#issuecomment-537978528
Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)
Maybe it's a bug in cppcheck? -- 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-plugins/pull/916#issuecomment-537971260
Re: [Github-comments] [geany/geany-plugins] Geanyprj's Project dock is extremely small (#744)
@codebrainz: yes, your suggested change fixes the issue. Posted a PR. -- 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-plugins/issues/744#issuecomment-537913556
[Github-comments] [geany/geany-plugins] geanyprj: fixed cut-off sidebar tab under gtk3 (#917)
Fixes #744. You can view, comment on, or merge this pull request online at: https://github.com/geany/geany-plugins/pull/917 -- Commit Summary -- * geanyprj: fixed cut-off sidebar tab under gtk3 -- File Changes -- M geanyprj/src/sidebar.c (2) -- Patch Links -- https://github.com/geany/geany-plugins/pull/917.patch https://github.com/geany/geany-plugins/pull/917.diff -- 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-plugins/pull/917
Re: [Github-comments] [geany/geany] Changed default title length from 30 to 80. (#2335)
God, please don't add prefs for stuff like this. -- 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/2335#issuecomment-537887424
Re: [Github-comments] [geany/geany] Changed default title length from 30 to 80. (#2335)
298 is about tab, dialogs and title bar (at least according to my comment in the issue :D). I think my point would be valid even if not. Just let's add a (probably hidden) preference for this. The OP prefers 80 characters, the original requestor needed less, the next will need some value in between. -- 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/2335#issuecomment-537885679
Re: [Github-comments] [geany/geany] Changed default title length from 30 to 80. (#2335)
@eht16 if I read it right 298 is for tabs, this is title bar. -- 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/2335#issuecomment-537864908
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
Great. FWIW, I've been using this patch since a while and have no problems so far. -- 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/2114#issuecomment-537854753
Re: [Github-comments] [geany/geany] Save main and project configuration whenever documents are opened/closed (#2114)
> Hopefully you'll squash or rebase to avoid the back merge? Done. -- 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/2114#issuecomment-537834548
Re: [Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)
Newer cppcheck versions (probably 1.89 which is in Debian Sid since a few days) complained about it and so the nightly builds broke. -- 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-plugins/pull/916#issuecomment-537828269
[Github-comments] [geany/geany-plugins] Overview: initialize color variables (#916)
While this might not be necessary technically, cppcheck complained: overviewscintilla.c:800:53: note: Calling function overview_scintilla_get_overlay_color, 2nd argument color value is Uninit overview_scintilla_get_overlay_color (self, color); ^ overviewscintilla.c:1146:11: note: Uninitialized variable: color You can view, comment on, or merge this pull request online at: https://github.com/geany/geany-plugins/pull/916 -- Commit Summary -- * Overview: initialize color variables -- File Changes -- M overview/overview/overviewscintilla.c (4) -- Patch Links -- https://github.com/geany/geany-plugins/pull/916.patch https://github.com/geany/geany-plugins/pull/916.diff -- 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-plugins/pull/916
Re: [Github-comments] [geany/geany-plugins] [New feature] autoclose: remove pairing quotes (#687)
This has now been implemented in PR #896 and merged into master. So it will be included in the next Geany release 1.37. Please test. -- 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-plugins/issues/687#issuecomment-537818519
Re: [Github-comments] [geany/geany] Enhancement Request - Multiselect Multipaste Option (#850)
> @Akronix see #2328 for recent developments grin Yep, I saw it and subscribed to the PR updates. Nice job @AdamDanischewski ! :clap: -- 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/issues/850#issuecomment-537816184
Re: [Github-comments] [geany/geany] Changed default title length from 30 to 80. (#2335)
As 80 characters is as arbitary as 30, this change would break the behaviour which was requested in https://sourceforge.net/p/geany/bugs/298/. I think if at all, we should make it configurable (and maybe enlarge the default a it). -- 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/2335#issuecomment-537814835
Re: [Github-comments] [geany/geany-plugins] autoclose: remove pairing quotes (#896)
Yepp. Good point. Still some time to go for next release and no feedback by maintainer. Scrolling code did not show big issues to me :) -- 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-plugins/pull/896#issuecomment-537814002
Re: [Github-comments] [geany/geany-plugins] [New feature] autoclose: remove pairing quotes (#687)
Closed #687 via #896. -- 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-plugins/issues/687#event-2682985844
Re: [Github-comments] [geany/geany-plugins] autoclose: remove pairing quotes (#896)
Merged #896 into master. -- 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-plugins/pull/896#event-2682985839
Re: [Github-comments] [geany/geany-plugins] tableconvert: disable menu item on not supported filetypes (#915)
Good point. Will have a look onto it. -- 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-plugins/issues/915#issuecomment-537812976
