[Github-comments] Re: [geany/geany] Plugin extensions (aka LSP API) (PR #3849)
As long as we don't put out a release I wouldn't worry much with API changes. Whoever uses the development head gets the development head. But speaking of releases, wasn't there the plan make one in the near future? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3849#issuecomment-758161 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Plugin extensions (aka LSP API) (PR #3849)
I have made some progress, in that peasy currently cannot generate GI stuff against Geany's master. Need to repair that first. Then, I would probably try to implement a python-lsp plugin in python, or do you have a better idea? I don' think this needs to block this PR, if you're OK that the API may need to be changed after the fact. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3849#issuecomment-2220269613 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Plugin extensions (aka LSP API) (PR #3849)
@kugel- commented on this pull request. > + gboolean (*goto_perform)(GeanyDocument *doc, gint pos, gboolean > definition, gpointer data); + + /** +* Pointer to function called by Geany to check whether the plugin implements +* additional symbol (e.g. type) highlighting in Scintilla. +* +* @see @c autocomplete_provided() for more details. +* @note There is no function in the @c PluginExtension structure informing +* plugins to perform symbol highlighting. Plugins +* implementing symbol highlighting should perform it at the appropriate +* moments based on Scintilla and Geany events such as when the document +* becomes visible or when the document is modified. +* +* @since 2.1 +**/ + gboolean (*symbol_highlight_provided)(GeanyDocument *doc, gpointer data); It makes me sad that significant improvements like that are only available on your lsp plugin. This suggests lots of future new hotness only hits one or the other LSP plugin and Geany's general functionality is left behind. This (colorization of keywords vs types) has always been a (minor) annoyance and I'm wondering why Geany can't do this right (I always assumed that it's a strict Scintilla limitation)? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3849#discussion_r1671662404 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Plugin extensions (aka LSP API) (PR #3849)
@kugel- commented on this pull request. > + gboolean (*goto_perform)(GeanyDocument *doc, gint pos, gboolean > definition, gpointer data); + + /** +* Pointer to function called by Geany to check whether the plugin implements +* additional symbol (e.g. type) highlighting in Scintilla. +* +* @see @c autocomplete_provided() for more details. +* @note There is no function in the @c PluginExtension structure informing +* plugins to perform symbol highlighting. Plugins +* implementing symbol highlighting should perform it at the appropriate +* moments based on Scintilla and Geany events such as when the document +* becomes visible or when the document is modified. +* +* @since 2.1 +**/ + gboolean (*symbol_highlight_provided)(GeanyDocument *doc, gpointer data); Hmm perhaps we should find a generic solution for the LSP support to do some stuff asynchronously (not necessarily now, but gradually and in the meantime we define the LSP API "not stable yet"). For example, for colorization, the LSP plugin can signal readyness to Geany's mainloop, and when Geany handles that "ready event", it would finally call `symbol_highlight_perform()`. This could even be used to complete Geany's own highlighting as well. I find it highly problematic that it's up to the LSP plugins to get the places, from where highlighting is triggered, right. At best the author looks where Geany calls `document_highlight_tags()` and draw conclusions from there. However, he may miss calls and/or it may even change between Geany versions and the plugin will regress in one version or the other. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3849#discussion_r1662113659 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Plugin extensions (aka LSP API) (PR #3849)
@kugel- commented on this pull request.
> + * to the members ending with @c _perform are called by Geany at appropriate
+ * moments to inform the plugin when to perform the given feature.
+ *
+ * The extension is defined by the pointers in the PluginExtension structure
and
+ * is registered in Geany using the @c plugin_extension_register() function.
+ *
+ * @warning The API provided by this file is subject to change and should not
be
+ * considered stable at this point. That said, it is highly probable that if
+ * a change of this API is required in the future, it will not be of a major
+ * nature and should not require major modifications of the affected plugins
+ * (e.g. added/modified parameter of a function and similar changes).
+ **/
+typedef struct
+{
+ /**
+* Pointer to function called by Geany to check whether the plugin
Perhaps confusing for you, but to me it's clear: This is a table of callbacks
implemented by the plugin. It's obvious to me that the plugin can chose any
name internally.
For reference, see `GeanyProxyFuncs` that we already have documented
```
/** Hooks that need to be implemented by every proxy
*
* @see geany_plugin_register_proxy() for a full description of the proxy
mechanism.
*
* @since 1.26 (API 226)
**/
struct GeanyProxyFuncs
{
/** Called to determine whether the proxy is truly responsible for the
requested plugin.
* A NULL pointer assumes the probe() function would always return @ref
GEANY_PROXY_MATCH */
gint(*probe) (GeanyPlugin *proxy, const gchar
*filename, gpointer pdata);
/** Called after probe(), to perform the actual job of loading the
plugin */
gpointer(*load) (GeanyPlugin *proxy, GeanyPlugin
*subplugin, const gchar *filename, gpointer pdata);
/** Called when the user initiates unloading of a plugin, e.g. on Geany
exit */
void(*unload)(GeanyPlugin *proxy, GeanyPlugin
*subplugin, gpointer load_data, gpointer pdata);
};
```
Anyway I don't request a change here, just that it's worded strangely to my
programmer eyes.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3849#discussion_r1660179187
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany] Plugin extensions (aka LSP API) (PR #3849)
@kugel- commented on this pull request.
> + * to the members ending with @c _perform are called by Geany at appropriate
+ * moments to inform the plugin when to perform the given feature.
+ *
+ * The extension is defined by the pointers in the PluginExtension structure
and
+ * is registered in Geany using the @c plugin_extension_register() function.
+ *
+ * @warning The API provided by this file is subject to change and should not
be
+ * considered stable at this point. That said, it is highly probable that if
+ * a change of this API is required in the future, it will not be of a major
+ * nature and should not require major modifications of the affected plugins
+ * (e.g. added/modified parameter of a function and similar changes).
+ **/
+typedef struct
+{
+ /**
+* Pointer to function called by Geany to check whether the plugin
I got that @elextr requested this kind of wording, but to me it sounds
absolutely weird. I would expect simpler language like "callback called by
Geany to …" or even just "called by Geany to…"
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3849#pullrequestreview-2150047738
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany] Plugin extensions (aka LSP API) (PR #3849)
@kugel- commented on this pull request. > + gboolean (*goto_perform)(GeanyDocument *doc, gint pos, gboolean > definition, gpointer data); + + /** +* Pointer to function called by Geany to check whether the plugin implements +* additional symbol (e.g. type) highlighting in Scintilla. +* +* @see @c autocomplete_provided() for more details. +* @note There is no function in the @c PluginExtension structure informing +* plugins to perform symbol highlighting. Plugins +* implementing symbol highlighting should perform it at the appropriate +* moments based on Scintilla and Geany events such as when the document +* becomes visible or when the document is modified. +* +* @since 2.1 +**/ + gboolean (*symbol_highlight_provided)(GeanyDocument *doc, gpointer data); IMO this is a recipe for getting it wrong. Why doesn't Geany define "appropriate moments based on Scintilla and Geany events" and calls a `symbol_highlight_perform()` then? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3849#pullrequestreview-2150047412 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Plugin extensions (aka LSP API) (PR #3849)
FWIW, I would like to ensure peasy plugins can use this APIs. I'm going to try -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3849#issuecomment-2198541335 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Possible plugin extension interface for symbol tree (PR #3910)
GBoxed works for non-C. You just have to add a non-C-friendly allocation/constructor because there's no generic `g_boxed_new()`. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3910#issuecomment-2175505360 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Possible plugin extension interface for symbol tree (PR #3910)
Generally speaking, non-C is supported well only for GObject-based interfaces. A python plugin can easily define a GObject-derived type purely implemented in Python. For anything else you need at least some supporting C code around. This is essentially what the "lib"-part of peasy does. It defines GObject-derived wrappers around existing Geany interfaces so that plugins make instances and inherit properly. Not all Geany interfaces need wrappers. We declare some of them as "GBoxed" such that the gobject-introspection magic makes them available to non-C languages automatically. We feed the gobject-introspection machinery mostly by converting the doxygen output to gtkdoc, and then have it generate the typelib from that. So for `PluiginExtension` we would need to extend peasy for sure. I asked specifically for `GeanySymbol`. At the very least it needs to be a GBoxed, but if it has a refcount and getters/setters for anything anyway it could also be a real GObject. Then peasy wouldn't need to wrap this type much. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3910#issuecomment-2175369898 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Possible plugin extension interface for symbol tree (PR #3910)
Would need to check in detail. Have you looked the gtkdoc that's generated from the new interfaces? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3910#issuecomment-2173696891 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Possible plugin extension interface for symbol tree (PR #3910)
If it has a refcount, why not make it a `GObject` so that it's easy to create from non-C plugins (python via peasy for example). -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3910#issuecomment-2173659025 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use GtkFileChooserNative for opening files on Windows and macOS (PR #3861)
Didn't we have native dialogs a while ago, at least on Windows? So this brings that back, basically? > To me at least this isn't the most important thing and using native dialogs > under Windows and macOS is more important IMO. You hard-coded the native dialogs. As they don't offer all the features of our dialog (renaming a file, for example) you should be able to use that. I can also imagine not everyone likes an "alien" dialog in a GTK application (it's native to Windows but alien Geany). -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3861#issuecomment-2084950388 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: Use host instead of target (PR #3853)
@kugel- approved this pull request. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3853#pullrequestreview-2016909248 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] meson: Prevent showing console after running geany in Windows (PR #3811)
IMO meson is still experimental and we don't need to support Ubuntu 20.04. Pick whatever meson makes the least problems for us. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3811#issuecomment-2068841524 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] [RFC] API for LSP plugins (PR #3571)
@elextr please read all the comments again, mine and from @b4n . Boiling them all down to "I don't like it" is ignorant and outright rude! You seem to be biased towards this change, perhaps because it works so well for you, and this is OK. But be honest about it and don't play down other people's voice. Maybe I didn't do a good job but I always tried to be objective and suggested alternatives or improvements. Also accept that not even the author deems the current form as ready for merging. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3571#issuecomment-1963659889 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Cannot open uncompressed PDF file (Issue #3677)
base64 then, the solution to just about anything? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/issues/3677#issuecomment-1797890095 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Skip `tests/test_sidebar` if GTK initialisation fails (PR #3676)
@kugel- requested changes on this pull request.
> /* Not sure if we can really continue without DISPLAY. Fake X display
> perhaps?
*
* This test seems to work, at least.
*/
- gtk_init_check(, );
+ if(!gtk_init_check(, )) {
+ printf("GTK initialisation failed; skipping. Running inside a
headless environment?\n");
Please adapt the above comment to the new code. I'm also unsure if printf
output is really desirable during test runs.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3676#pullrequestreview-1716355387
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany] [RFC] API for LSP plugins (PR #3571)
> > So to my understanding both sidebar features have to be implemented using > > the fallback. > > I think you misunderstand the purpose of the goto-implementation, signature > and documentSymbol calls: I think understand the intention of the LSP interfaces. That's why I'm pretty clear that they cannot be used itself for implementing goto-definition and tooltip on the symbols sidebar. These functionalities in Geany have to be implemented using other LSP interfaces or the TM fallback. I now looked at your changes in more detail and found that you accordingly don't use the goto-implementation and signature interfaces for the sidebar. My previous understanding of the PR was that these sidebar features are implemented using the TM fallback (`doc->tm_file->tags_array`) but that was a false understanding. Sorry for that! > > But the fallback, as proposed in this PR, relies entirely on TM backed by > > ctags parsing. > > No, this PR uses `documentSymbol` symbols for the sidebar (shows symbol name > in the tree, shows "detail" as the calltip, generates tree based on children > and assigns icon based on kind). I was just suggesting that if this PR is > considered too big, I could sacrifice this LSP feature and could live with > the TM implementation. But this is essentially what I want that we do generally. I looked at your code in more detail and you're actually doing what I'm asking for all the time: you're generating `TMTag`s from the LSP data. Now can we not go one step further and not only attach these tags to the sidebar menu items but to the GeanyDocument itself? And please don't re-iterate the "documentSymbol" support is not guaranteed. To me this is basic functionality for any language server. If that's not provided the server is worthless (we couldn't have the symbols sidebar at all!). Please show me one server that does not provide it at all. The *quality* of the implementation is a different matter but that's no different to our ctags parser. These also vary widely, not just in terms of parser results but also performance. We are never guaranteed good support for some languages. And this is something we accept and get along with. It's always "best effort". The same applies to whatever language server we talk to. You said that just documentSymbol might give inferior results for `doc->tm_file->tags_array`, compared to our ctags parsers. Is that always true or does it depend on the server? What about C/C++ and the `clangd` server for example? I want to get a feeling if the documentSymbol interface itself is lacking or if existing servers aren't good enough. Speaking about superior ctags parsing: I understand your implementation is susceptible for this scenario for the sidebar use cases: If the language server does not provide `detail` in documentSymbol, then we won't have a tooltip. Similarly, if the server does not provide range/location then we won't have goto-definition. Now if we would have that information in the TM fallback because the parser recorded this information in `doc->tm_file->tags_array`, then these two use cases are effectively regressed. I don't have a feeling how realistic this is because I don't have a good picture about the quality of server implementations, but generelly speaking this is a situation that I want to avoid if possible as it would make our LSP support look poor compared to the status quo, even if it excels in other use cases. On the other hand, if we follow-up on your other idea and use LSP even to interface with the ctags parsers then documentSymbol *ought to be* sufficient for all existing features (except maybe where we can specialized LSP interfaces like goto-implementation), right? > > > I can believe that you or @techee don't care about goto-implementation and > > calltips enough on the sidebar but I do. > > But this works, I really don't know what you are talking about. Have you > actually tried the bloody plugin? It's a few minutes of your time to install > and test it - would really save hours of pointless discussions. I never questioned that your plugin works, at least for C/C++ where we have a solid TM-based fallback as well. I could find the above only by code study not just by testing. Actually testing requires me to setup an LSP server (or perhaps multiple ones) which I don't yet know how to do so I'm afraid it takes more than a few minutes of my time. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3571#issuecomment-1793868060 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] 2.0: `test_sidebar` failing on macOS in headless environment (Issue #3674)
I still recommend Xvfb. Geany is a GTK program, so you can expect that future tests will also require successful GTK initialization. But a PR that skips the test would probably be accepted. Do you know if the `exit 77` trick works on meson as well? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/issues/3674#issuecomment-1793845656 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] 2.0: `test_sidebar` failing on macOS in headless environment (Issue #3674)
Sorry, I mixed it up. I mean [Xvfb](https://www.x.org/releases/X11R7.6/doc/man/man1/Xvfb.1.xhtml). That's a pure software emulated X environment. Why do you execute the tests at all? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/issues/3674#issuecomment-1793801724 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use Scintilla lexer names rather than deprecated IDs (PR #3668)
@kugel- commented on this pull request. > SSM(sci, SCI_SETILEXER, 0, (uintptr_t) lexer); - if (old != (gint)lexer_id) + if (old != sci_get_lexer(sci)) Well we'll have to start somewhere. Other places can be fixed incrementally I would say. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3668#discussion_r1382575002 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Geany 4.0, the GTK4 edition (and more) (Discussion #3675)
> Use above to move TM to plugins, perhaps with LSP like API, see > https://github.com/geany/geany/pull/3571#issuecomment-1793696182 I think we always need something like TM to have an in-process cache for symbols/tags. We're still a lightweight IDE so a my requirement would be to not exchange megabytes of json text on every keystroke. I call it TM2 because it doesn't necessarily need to be API-compatible to the current TM. So I think my idea would be to keep TM2 in the core and call out to the ctags-server on some suitable opportunities, then cache the response and move forward. Would have to make sure that TM2 is also compatible with other major language servers. Question: How does a language server get access to the documents? Does it have to be saved on disk so that the server can `open()` it? If yes, how can LSP possible act on unsaved buffers? If no, do we have to constantly send the entire document via IPC or network? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/discussions/3675#discussioncomment-7479130 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] [RFC] API for LSP plugins (PR #3571)
@techee > I don't know what exactly you want to see in this class summary, it sounds > like you could obtain the necessary information We don't have to deep dive into that. I just made up an example that doesn't have a specialized LSP interface. Your reply was expected: Use other interfaces (maybe multiple ones) that provide the necessary information. This is alright. I just want to get past the "use only LSP interfaces for their original intention" argument because it would stop you from implementing new features until a specialized LSP interface arrives. > But why would I mix different LSP interfaces when there are specific > interfaces for the purposes we need? That's nuts. Because, for example the goto-definition LSP interfaces does not work in all situations (needs a source code location as input, see below). When it's applicable then it should be preferred, that's for sure. > And nobody says we should drop the TM symbols I think we're clear on that, nobody suggests that. We're disagreeing whether LSP data should augment the TM-ctags-based fallback. I maintain it should be done so that use cases that require the fallback don't regress or remain unavailable entirely. > I think it would make a lot of sense to make a LSP server from the tag > manager and access it only through the LSP interface. I think that's a fine idea and it goes much in my direction because it ensures that we can realize all existing functionality over LSP (unless you want to drop some functionality in the process?). Lets discuss that idea elsewhere, though, this PR is already becoming confusing. @elextr > @kugel- as @techee said, but more bluntly, you do not seem to understand what > the [Language Server > Protocol](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/) > (LSP, not Language Server Process for removal of doubt) is designed for and > are making yourself look stupid by arguing without that knowledge. Please > stop being disruptive, if you have suggestions to improve the implementation > please make them, but I feel your "change it all" is not helpful. > Sorry if I sound stupid but I don't really care what LSP was originally designed for. I care if we can make LSP and Geany fit together. Perhaps that may require using LSP interfaces for other purposes than their original intention. I don't want to turn Geany up-side-down just to make it fit the LSP design (at least not all at once). This is why I want to keep TM infrastructure working *and* consistent when LSP is in effect. I'm not even too attached to the ctags parsing itself. But lots of functionality requires TM even with this PR and I don't want to lose it if LSP is in effect, and ideally I want to have this functionality for languages where LSP is the only option (where we have no parser). The way I use Geany depends on TM but I too want to benefit from LSP. Is that hard to understand? > The LSP is designed for the server to hold a current version of the semantic > annotated abstract syntax tree for the program, and accurately answer > questions using that. There is no API (that I can see) that allows that to be > exported, just a flat or hierarchical list of symbols suitable for the > sidebar. > To my understanding, LSP just exposes interfaces that give information about the documents or projects. Whether the server keeps an AST is an implementation detail and the server may have other means to provide the interfaces. > The approach of having Geany call a plugin for specific functionality, or > fallback to its own, is in principle a good one. Just that Geany has never > called a plugin before so its a new concept and has no design yet. Maybe we > need to think about it more generally. But it allows for progressive > development as individual items of functionality can be converted one at a > time and features that the LSP does not support can also fallback to the > existing implementation. And if it all goes pear shaped most of the code is > in the plugin, not Geany, so it can be removed relatively easily or an > alternate implementation plugged in. The alternative is to build LSP into > Geany and be much more disruptive. I don't disagree. But entire point is that the "fallback to its own" is consistent with LSP data. I don't want use cases that cannot be mapped to LSP interfaces to regress when LSP is in effect. Because I think 1) LSP support is not going to be successful if it creates regressions on other areas and 2) it creates support nightmares because users may run on inconsistent data backends. The "if you use an LSP plugin you're on your own" excuse does not work always if we support a plugin API specifically for that and the issues get created anyway. > > To answer your last questions: > > > How do you plan to handle other various goto-definition cases that don't > > come from a document context? > > The LSP works on a project basis, in
[Github-comments] Re: [geany/geany] Geany 4.0, the GTK4 edition (and more) (Discussion #3675)
Can we have threads here? I would like to separate GTK4 and LSP discussions. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/discussions/3675#discussioncomment-7478754 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] 2.0: `test_sidebar` failing on macOS in headless environment (Issue #3674)
On headless systems you should run the tests under Xephyr or something similar. That would be preferable over skipping the tests. That's what I do when I build geany on my local Jenkins instance. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/issues/3674#issuecomment-1793687347 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] [RFC] API for LSP plugins (PR #3571)
> Using LSP only as an alternative source of tags doesn't make sense to me > because it won't bring any better autocompletion or symbol goto so why bother. Sorry, I didn't want to suggest to only grab tags from LSP. My point is that we should grab tags from LSP at a minimum (and maybe use LSP in other opportunities additionally). For example, we can probably use the goto-definition LSP interface where possible (i.e. ctrl+click in the editor). I guess it has the advantage that the LSP can infer whatever foo is in `foo->func()` and then jump to the right definition, instead of presenting the user the various `func()` overloads to chose from. But we must make sure that other goto-defintion use-cases (no document context) are also backed by LSP data and not by a completely different parser that perhaps only parses half of the document compared to the language server. Otherwise the LSP integration is going to be a source of user frustration and bug reports. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3571#issuecomment-1793583102 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] [RFC] API for LSP plugins (PR #3571)
I like LSP in general. I have a problem with your narrow view ("in a way that
the LSP interface is supposed to be used", whoever defines what "supposed to be
used" means) that leaves the TM infrastructure behind for anyone that depends
on it. TM is still active but there is no provision that TMTags is anywhere
consistent with LSP data. And the user is going to interact with both backends
in your current design.
What you call "Frankenstein LSP" is my attempt to envision an LSP integration
that does not regress those other use case by ensuring a consistent backend.
Please answer these two questions:
- How do you plan to handle other various goto-definition cases that don't come
from a document context?
- And for call tips (mouse over in sidebar symbols)?
Unless I'm reading the LSP spec wrong you cannot use the corresponding LSP
interfaces for those. And if you still rely on current TM (ctags parsing) for
those then *that's* Frankenstein.
Also, AFAICS your own ProjectOrganizer interfaces heavily with TM. What's your
plan on that front? You aren't ending support for that plugin, are you?
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3571#issuecomment-1793581783
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany] [RFC] API for LSP plugins (PR #3571)
> > I just don't like this proposal because it "hacks" LSP support into each > > relevant use case instead of augmenting the existing TM infrastructure with > > LSP information. It adds individual band-aids for each use case and leaves > > behind other uses of the TM infrastructure (in other plugins). It's OK to > > get familiar with LSP and have something to showcase "geany-on-lsp" but > > it's not something that I comfortably support going forward. > > Once again, this is how LSP works - if you want an autocomplete list, you ask > the server to provide it for you using the completion request: > … > Similarly, if you want go to symbol definition/declaration, you use > … > When you want to get information for colorization of types or other symbols, > you use > … > For calltips you call > … > For symbol tree, you call > > https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_documentSymbol > > And so on. So you're suggesting that any one editor or IDE may only implement features for that a specialized interface in the LSP exists, and no more? I.e. features that are blessed by the the LSP (=VSCode) folks? Seems horrid. Maybe if LSP severely limits the features we may implement it's not the right tool for us. Or we use LSP in a way that allows us to implement our features using the interfaces that are available, regardless of how someone else declares "that's how LSP works". Put another way: Assume I want to implement a "class summary" tab in the sidebar, a HTML rendering of the current class (defined by cursor position), including all members (types, signatures) and their visibility. I don't see a special `ClassSummary` interface in the LSP spec. Does that mean I cannot implement this feature? What are the options, assuming we didn't have TM anymore. > > > LSP servers ought to provide the symbols for files via DocumentSymbol or > > SymbolInformation. That should all be needed to build up a TMTags array > > attached to TMSourceFiles (without actually parsing via ctags) [1]. Sure, > > an LSP might not provide all the bits we want, but as @elextr said that may > > be the case with poor ctags parsers as well, or this very server instance > > is not worth asking anyway. > > You can surely do that but the question is why would you do that. You get > identical (or worse) info regarding individual symbols than from TM but > enough for the symbol tree (which is the purpose of this particular LSP > call). But you can't derive any extra info from it for the "smart" features > like goto declaration/definition or autocompletion - you don't get info about > types of the symbols, their visibility etc. - this is all hidden in the > implementation of the LSP server. Depending on the language this might give more or more accurate (w.r.t. to build configuration). Probably you can also mix different LSP interfaces to get a more accurate picture about individual symbols. > > Further, you are not even guaranteed that the server implements the > `textDocument/documentSymbol` call in which case you won't implement anything > on top of that - yet the server may provide the goto declaration/definition > call or autocompletion functionality. I truly think that `DocumentSymbol`/`SymbolInformation` is the very basic interface that any worthwhile LSP server implements. How else do you get to learn about any symbol in a file? You need a symbol list to pick one for advanced stuff like goto-definition don't you? So if those aren't implemented the server is useless. Do you know an example of such a limited server? > > > You already implement complex features like symbol tree and call tips and > > more using LSP so the protocol itself must be sufficiently suitable for us, > > right? > > Symbol tree isn't a complex feature in terms of symbol processing - you just > display them (it's just all the annoyance with GtkTreeView). But if you want > to implement a _good_ autocompletion or symbol goto taking into account the > symbol visibility, you just have to call the corresponding LSP call - without > this you'd degrade the whole LSP to the not-so-good TM-style goto or > autocompletion and the LSP doesn't make sense here. > Why do you think I want to take visibility into account for go-to? That would mean I cannot go to the definition of a private member function from a foreign class. Yes, that code may be illegal, but in the editor I still want to perform that jump to resolve the situation, e.g. by moving the function to the public section. Does the LSP interface even allow that or am I constrained to what LSP thinks how my implementation of goto-definition should work? My point is: It may be nice that LSP provides specialized interface for some features. But it doesn't do for all imaginable features, and the existing interface might also not fit perfectly to our requirements. So we have to think about fallback solutions
[Github-comments] Re: [geany/geany] [RFC] API for LSP plugins (PR #3571)
You seem to misunderstand my point. I'm not at all against LSP, quite the opposite. I like LSP because it seems to be industry standard by now with lots of server-implementations (of varying quality) that loosens our "ctags lock-in". I just don't like *this* proposal because it "hacks" LSP support into each relevant use case instead of augmenting the existing TM infrastructure with LSP information. It adds individual band-aids for each use case and leaves behind other uses of the TM infrastructure (in other plugins). It's OK to get familiar with LSP and have something to showcase "geany-on-lsp" but it's not something that I comfortably support going forward. LSP servers ought to provide the symbols for files via `DocumentSymbol` or `SymbolInformation`. That should all be needed to build up a `TMTags` array attached to `TMSourceFile`s (without actually parsing via ctags) [1]. Sure, an LSP might not provide all the bits we want, but as @elextr said that may be the case with poor ctags parsers as well, or this very server instance is not worth asking anyway. You already implement complex features like symbol tree and call tips and more using LSP so the protocol itself must be sufficiently suitable for us, right? You use `DocumentSymbol` in your reference plugin as well, after all. [1]: The asynchronous architecture is an obstacle, that's right, but to me that's just necessary refactoring. GIO would allow us to use TM in an async fashion as well. Just need to teach code that tags are not immediately available after `document_open_file_full()` but rather after some "document-tags-ready" signal. That would probably also allow us to improve start-up and project-loading scenarios. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3571#issuecomment-1793457233 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] [RFC] API for LSP plugins (PR #3571)
OK, TM is still active, but that means I may get different results when using LSP vs TM. I.e. the list of symbols on the GUI may be different to what a plugin gets from quering TM. This potential inconsistency makes me nervous. This is especially true since LSP may have more build system insights and perhaps knows which ifdefs are active and which are not, leading to some symbols being hidden or different locations for definitions. >> Instead I think we should use LSP to augment tagmanager so that we have all >> the needed bits within our problem space when "goto definition" is >> triggered. Then we can reason about the result. If TM data store is not >> suitable we should make the necessary modifications there to make LSP >> realistic. > I have absolutely no idea what you mean by this. What do you mean by "reason > about the result"? The LSP server just returns the file name and line number > and I don't know what "reasoning" you want to do about it. I mean with TM we have all data required to implement the features locally available. We can do smart things before the user triggers things or after based on that. With your LSP proposal we ask the external entity only when the user triggers the action and we have to hope for the best. I would rather see that we mix-in LSP data (even if that overrides TM) beforehand/in the background. AFAIK you can get all symbols in a document through LSP. Then we can import that to our local data storage and implement goto-definition on-top of that (and here LSP may replace TM's document parsing or we invent some smart combination). That way other plugins can use LSP data the same way as TM data right now and there's no room for inconsistency. Basically I'm saying: TM can use LSP data, either exclusively (instead of parsing the file itself) or as an extra data source. Rest of Geany and plugins still interface with TM (may require changes here and there if LSP provides less data and someone was depending on that missing pieces, but I hope not). -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3571#issuecomment-1793228746 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] [RFC] API for LSP plugins (PR #3571)
I'm not a huge fan of this. I use plugins that depend on tagmanager, therefore I cannot use LSP. I think this going the wrong direction. Now, for example, when "goto definition" is requested we start some IPC or network action which can block and/or fail, and this is going to be a nightmare to debug. We don't know beforehand what will happen, like if the language server even provides any useful information. Instead I think we should use LSP to augment tagmanager so that we have all the needed bits within our problem space when "goto definition" is triggered. Then we can reason about the result. If tagmanager data store is not suitable we should make the necessary modifications there to make LSP realistic. Last but not least, I think we should have a unified API towards plugins so that they can work with tags and symbols regardless of LSP or tagmanager (or a combination). -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3571#issuecomment-1793149958 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use Scintilla lexer names rather than deprecated IDs (PR #3668)
@kugel- commented on this pull request. > SSM(sci, SCI_SETILEXER, 0, (uintptr_t) lexer); - if (old != (gint)lexer_id) + if (old != sci_get_lexer(sci)) Must be out-dated. Clearly, because of > Some lexers may not have a lexer ID, just a lexer name in which case 0 is > returned. `SCI_GETLEXER` cannot be reliably used anymore, if using any recent lexer anyway. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3668#discussion_r1382203937 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use Scintilla lexer names rather than deprecated IDs (PR #3668)
@kugel- commented on this pull request. > SSM(sci, SCI_SETILEXER, 0, (uintptr_t) lexer); - if (old != (gint)lexer_id) + if (old != sci_get_lexer(sci)) We should probably deprecate `sci_get_lexer()` and stop using it internally. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3668#pullrequestreview-1713475789 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] `Use Windows native dialogs` is broken (Issue #3627)
Still a bug if the pref is still on the UI -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/issues/3627#issuecomment-1774024049 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] `AddGeanyLexers()` omits the AsciiDoc module since 19336d2 (Issue #3615)
sorry -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/issues/3615#issuecomment-1770894859 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Fix crash opening an AsciiDoc document (PR #3616)
@kugel- approved this pull request. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3616#pullrequestreview-1687593173 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Release 2.0 documentation (PR #3593)
LGTM -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3593#issuecomment-176945 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Fix startup files order when placing tabs next to the current one (PR #3611)
@kugel- commented on this pull request. > /* necessary to set it to TRUE for project session support */ main_status.opening_session_files++; - i = file_prefs.tab_order_ltr ? 0 : (session_files->len - 1); - while (TRUE) + file_prefs.tab_order_beside = FALSE; + file_prefs.tab_order_ltr = TRUE; yes (although I had a hard time to decode the new block in my brain) -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3611#discussion_r1364494255 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Fix startup files order when placing tabs next to the current one (PR #3611)
> And having added the feature back then, I can tell you I didn't plan for this > :) Alright, I am convinced it's a bug now :-) -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3611#issuecomment-1769240269 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Fix startup files order when placing tabs next to the current one (PR #3611)
@kugel- commented on this pull request. > /* necessary to set it to TRUE for project session support */ main_status.opening_session_files++; - i = file_prefs.tab_order_ltr ? 0 : (session_files->len - 1); - while (TRUE) + file_prefs.tab_order_beside = FALSE; + file_prefs.tab_order_ltr = TRUE; Then I think `notebook_new_tab()` should look at `main_status.opening_session_files`. This is the primary indication that we are in "batch open mode" if some code must behave differently. I also used that flag to open docs in a idle callback instead of synchronously (see #3267 / commit 23367de0c5) -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3611#discussion_r1364474548 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Fix startup files order when placing tabs next to the current one (PR #3611)
@kugel- commented on this pull request. > /* necessary to set it to TRUE for project session support */ main_status.opening_session_files++; - i = file_prefs.tab_order_ltr ? 0 : (session_files->len - 1); - while (TRUE) + file_prefs.tab_order_beside = FALSE; + file_prefs.tab_order_ltr = TRUE; Why do you override the pref temporarily? Is there some nested function call that looks at the pref? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3611#pullrequestreview-1686005701 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Fix startup files order when placing tabs next to the current one (PR #3611)
Actually if it's like this in 1.38 already (and possibly earlier) I see no compelling reason to rush this into 2.0, considering that tomorrow is release. It's not even clear to me if this is even a bug (as on "not working as intended"). I surely never used that pref. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3611#issuecomment-1769214237 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Change default tab_label_length from 99999 to 1000 (PR #3612)
BTW, what's the unit of the setting? pixels, chars, apples? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3612#issuecomment-1768237069 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Release 2.0 documentation (PR #3593)
@kugel- commented on this pull request. > +* Add a confirmation dialog on search & replace for the whole session + (PR#3033). +* Filter entry for symbol tree (PR#3055). +* Simplify project creation from existing directories with sources + (PR#3042). +* Add option to show symbols in symbol tree without category groups + (PR#3172). +* Add option to only show line endings if they differ from file default + (PR#3287). +* Make tab label length and window title length configurable + (Abdul Rafey, #3365). +* Make Go to Symbol commands show signature list (PR#3475). + +Editor +* Update Scintilla to 5.3.7 and Lexilla to 5.2.7 (PR#2867, PR#3551). +* Add "Change history" feature (PR#3551). That's why I said "(without explaining why)" ;-) -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3593#discussion_r1362753305 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Release 2.0 documentation (PR #3593)
Awesome! Can't find words for how much I appreciative your effort. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3593#issuecomment-1767128585 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Release 2.0 documentation (PR #3593)
@kugel- commented on this pull request. > +* Add a confirmation dialog on search & replace for the whole session + (PR#3033). +* Filter entry for symbol tree (PR#3055). +* Simplify project creation from existing directories with sources + (PR#3042). +* Add option to show symbols in symbol tree without category groups + (PR#3172). +* Add option to only show line endings if they differ from file default + (PR#3287). +* Make tab label length and window title length configurable + (Abdul Rafey, #3365). +* Make Go to Symbol commands show signature list (PR#3475). + +Editor +* Update Scintilla to 5.3.7 and Lexilla to 5.2.7 (PR#2867, PR#3551). +* Add "Change history" feature (PR#3551). Might mention that it's disabled by default (without explaining why) -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3593#pullrequestreview-1683326769 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Release 2.0 documentation (PR #3593)
@kugel- commented on this pull request. On doc/images/main_window.png: I like the proportions of the older screenshot better (msgwin is huge now) -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3593#pullrequestreview-1683324927 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Release 2.0 documentation (PR #3593)
@kugel- commented on this pull request. On doc/images/pref_dialog_various.png: Hm, the older screenshot shows some contrast between the list items. We kind of regressed here. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3593#pullrequestreview-1683319978 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@kugel- approved this pull request. LGBI but I only understand a fraction of it anyway -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#pullrequestreview-1680935257 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Enable EOL-filled Markdown headings (PR #3602)
agree -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3602#issuecomment-1764367172 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Windows: Include "Prof-Gnome" GTK theme from geany-osx (PR #3129)
I NAK'd the theme but I don't want to block any Adwaita-replacement so please go for it. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3129#issuecomment-1763146890 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] New UI on windows is bloated and scaled up (Issue #3063)
I don't have enough time and motivation to play with themes on windows anytime soon. Go with whatever you agreed. While I NAK'd #3129 on based on my preference (so biased) I was also clear that I won't block it (or any other theme) as long as we swap out Adwaita. So I'm in favor of merging #3129 over post-poning. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/issues/3063#issuecomment-1763144730 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Update meson.build also on version bumps (PR #3599)
@kugel- commented on this pull request. > @@ -23,6 +23,7 @@ s/^\(#define VER_FILEVERSION_STR *\)[^ ].*$/\1"'"$VER"'"/ ' -i geany_private.rc sed -e 's/^\(AC_INIT([^,]*, *\[\)[^]]*\(\],\)/\1'"$VER"'\2/' -i configure.ac +sed -e 's/^\( *version: *\)[^,]*\(,\)/\1'"\'$VER\'"'\2/' -i meson.build Rather greedy. For OK, but I *will* add another line that matches one day or another. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3599#pullrequestreview-1678401351 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Update meson.build also on version bumps (PR #3599)
@kugel- approved this pull request. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3599#pullrequestreview-1678401360 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Do not enable Scintilla's Change History by default (PR #3591)
Fine with me -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3591#issuecomment-1757418527 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] C++ goto declaration and goto definition show same list (Issue #3585)
We could introduce the new action and map to ctrl+t, and "go to definition" becomes unbound. If "go to declaration" is essentially the same thing (as of now, until it's fixed) then unbind it too. If I explicitly use "go to definition" when I'm on a definition, then I probably don't want to go to the declaration but rather see the list of other definitions. Likewise with "go to declaration". This is currently impossible if IIUC? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/issues/3585#issuecomment-1757195120 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] C++ goto declaration and goto definition show same list (Issue #3585)
There is also ctrl+click on the symbol. This is the way what I mostly "it" and works like @techee describes -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/issues/3585#issuecomment-1754433029 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
@kugel- requested changes on this pull request. > - /* tag name */ - if (! (tab = strchr(p, '\t')) || p == tab) - return FALSE; - tag->name = g_strndup(p, (gsize)(tab - p)); - p = tab + 1; + if (entry.kind[0] && entry.kind[1]) + type = tm_parser_get_tag_type(tm_ctags_get_kind_from_name(entry.kind, lang), lang); // 'K' field C-style comments, please : `/* 'K' field */` (more of that below) > @@ -154,7 +154,7 @@ gboolean tm_tag_is_anon(const TMTag *tag); const char *tm_tag_type_name(const TMTag *tag); -TMTagType tm_tag_name_type(const char* tag_name); +TMTagType tm_tag_name_type(const char* tag_name, TMParserType lang); can't find the corresponding change in the C file- -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#pullrequestreview-1664057269 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use the official 'libreadtags' library from ctags for parsing ctags files (PR #3049)
I wonder why PRs like this are open or so long and then rushed into the release. Not enough pinging? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3049#issuecomment-1752640119 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Bump GTK version requirement to 3.24 (PR #3580)
@kugel- approved this pull request. Sorry -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3580#pullrequestreview-1663708673 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] CI: Use mingw posix crosscompiler (PR #3568)
Go for it, it's approved after all. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3568#issuecomment-1752163855 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Bump GTK version requirement to 3.24 (PR #3580)
@kugel- commented on this pull request.
> @@ -841,8 +827,8 @@ on_font_dialog_response(GtkDialog *dialog, gint response,
> gpointer user_data)
{
gchar *fontname;
- fontname = gtk_font_selection_dialog_get_font_name(
-
GTK_FONT_SELECTION_DIALOG(ui_widgets.open_fontsel));
+ fontname = gtk_font_chooser_get_font(
+ GTK_FONT_CHOOSER(ui_widgets.open_fontsel));
Okay, I didnt see the calls inside the macros.
But still, they are constructed with `gtk_font_chooser_dialog_new()` so I would
think `GTK_FONT_CHOOSER_DIALOG()` is correct anyway
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3580#discussion_r1349641185
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany] Fix goto popup location (PR #3316)
@techee @b4n do we still want this in 2.0? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3316#issuecomment-1751845522 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Bump GTK version requirement to 3.24 (PR #3580)
@kugel- requested changes on this pull request.
> @@ -841,8 +827,8 @@ on_font_dialog_response(GtkDialog *dialog, gint response,
> gpointer user_data)
{
gchar *fontname;
- fontname = gtk_font_selection_dialog_get_font_name(
-
GTK_FONT_SELECTION_DIALOG(ui_widgets.open_fontsel));
+ fontname = gtk_font_chooser_get_font(
+ GTK_FONT_CHOOSER(ui_widgets.open_fontsel));
`GTK_FONT_CHOOSER_DIALOG()`
> @@ -883,8 +869,8 @@ void dialogs_show_open_font(void)
gtk_window_set_transient_for(GTK_WINDOW(ui_widgets.open_fontsel),
GTK_WINDOW(main_widgets.window));
}
- gtk_font_selection_dialog_set_font_name(
- GTK_FONT_SELECTION_DIALOG(ui_widgets.open_fontsel),
interface_prefs.editor_font);
+ gtk_font_chooser_set_font(
+ GTK_FONT_CHOOSER(ui_widgets.open_fontsel),
interface_prefs.editor_font);
`GTK_FONT_CHOOSER_DIALOG`
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3580#pullrequestreview-1663117013
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany] Add templates to Config Files menu (PR #3396)
This touches strings (doesn't it?) and we're in string freeze so I'm afraid it's too late now. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3396#issuecomment-1751834172 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Fix meson build without git repo (PR #3578)
I don't really want to support meson from the tarball. It's still too experimental. If it seems to work from the tarball some guy or distro is going to rely on it. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3578#issuecomment-1751683826 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Disable autocompletion for most non-programming/non-scripting languages (PR #3575)
> I suggest this be a setting in the filetype file, not a hard coded magic list. Agree. And the user must be able to override this. For Latex, I definitely disagree with this change. When I used Latex for my master's thesis I enjoyed auto-completion for my custom macros as well as long words. Generally I'm not sure this change is a good idea. If you don't need auto-completion then the pop-up is relatively easy to ignore or cancel if it obscures something important. On the other hand, if you enjoy auto-completion it'll be very hard to get it back. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3575#issuecomment-1750076406 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] [RFC] API for LSP plugins (PR #3571)
Your call. I would welcome a first class, reference plugin for LSP support, to show off how to do it right and to ensure Geany's support for LSP plugins remains fully intact. I think optional dependencies are fine, Geany still works without the plugin for the foreseeable future. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3571#issuecomment-174770 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] [RFC] API for LSP plugins (PR #3571)
I wonder why we can't have an LSP plugin shipped with the core to ensure support this kind of plugins properly. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3571#issuecomment-1747667145 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Add "document-before-save-as" signal (PR #3572)
@kugel- commented on this pull request. > @@ -92,6 +92,17 @@ signal void (*document_reload)(GObject *obj, GeanyDocument > *doc, gpointer user_d */ signal void (*document_before_save)(GObject *obj, GeanyDocument *doc, gpointer user_data); +/** Sent before save as is performed with the original document. + * + * @param obj a GeanyObject instance, should be ignored. + * @param doc the original document. The document with the new file name is still + *reported by the "document-save" signal sent afterwards. + * @param user_data user data. + * + * @since TODO + */ +signal void (*document_before_save_as)(GObject *obj, GeanyDocument *doc, gpointer user_data); Perhaps add a `boolean` return value to potentially allow the plugin to block saving the file? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3572#pullrequestreview-1658506031 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] [RFC] API for LSP plugins (PR #3571)
@kugel- commented on this pull request.
> +G_BEGIN_DECLS
+
+typedef struct {
+ gboolean (*autocomplete_available)(GeanyDocument *doc);
+ void (*autocomplete_perform)(GeanyDocument *doc);
+
+ gboolean (*calltips_available)(GeanyDocument *doc);
+ void (*calltips_show)(GeanyDocument *doc);
+
+ gboolean (*goto_available)(GeanyDocument *doc);
+ void (*goto_perform)(GeanyDocument *doc, gboolean definition);
+} Lsp;
+
+
+void lsp_register(Lsp *lsp);
+void lsp_unregister(Lsp *lsp);
I would also pass the size of the `Lsp`. Reason: If you extend the struct for
new function points you can't be sure if the plugin that's registering is
already compiled against the new size, unless you also bump the ABI. To avoid
having to bump the ABI the plugin can pass `sizeof(Lsp)` that it was compiled
against, and Geany can pad with NULLs up to the actual size.
Furthermore, these kind of register functions should also pass the
`GeanyPlugin` to give Geany some context.
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3571#pullrequestreview-1658498920
You are receiving this because you are subscribed to this thread.
Message ID:
[Github-comments] Re: [geany/geany] Update Scintilla to 5.3.7 and Lexilla to 5.2.7 (PR #3551)
Merged #3551 into master. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3551#event-10555615072 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Update Scintilla to 5.3.7 and Lexilla to 5.2.7 (PR #3551)
@kugel- pushed 8 commits. 19336d22946b77bbcb5131544ea0e6317cdcf6b5 Update Scintilla to version 5.3.7 9aa597e945876aa61024de1ff6398fa5eba281f0 Map new python "attribute" mapping to identifier_2, by default. 7f78ddf694cda6a50f93dacb2d6008d6c6bcfdfe Map new ruby lex classes to existing styles, for now. 0f04b2437c4f6e6dd9bd4ed5ace1c27dd16c1efb Handle new styles for strings in R (rawstrings, backticks, espaces) 59d8aee1a8bbb125c38257b60da85a5ae46de36f Handle new nodepath style for gdscript 560ec26d280253b177631dbae8ecdf92ec2fd88f Update scintilla_changes.patch 25c568726693baa9a15347f743f5b0dbbb656ca0 Enable new Scintilla "Change History" 75d528db86cc6f7a77024b39a0afe73fe74f0c4c Bump plugin API -- View it on GitHub: https://github.com/geany/geany/pull/3551/files/e4feb5cab6e4e2b878fae7be7cbd20ceb729fedf..75d528db86cc6f7a77024b39a0afe73fe74f0c4c You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] CI: Use mingw posix crosscompiler (PR #3568)
CI still checks out the PR branch and runs the scripts within, doesn't it? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3568#issuecomment-1746328972 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] CI: Use mingw posix crosscompiler (PR #3568)
> How should we fix it in Geany? I don't have an answer (hence I approved this PR) but my perspective is that Geany should the CI requirements, otherwise regular contributors might be unable to propose certain changes without breaking CI (because they cannot fix the CI in the same context). Imagine a regular contributor wanted to update to a scintilla version that has new requirements. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3568#issuecomment-1746296392 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use ISO 8601 date format by default (yyyy-mm-dd) (PR #3570)
I would merge this just before the string freeze (which is not yet announced, also due to #3551), but yeah I would also exclude the .po changes. Does anyone object? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3570#issuecomment-1746280818 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Update Scintilla to 5.3.7 and Lexilla to 5.2.7 (PR #3551)
Fine. I'll merge and bump the plugin API separately. Regarding https://groups.google.com/g/scintilla-interest/c/_tiE_nSaiG4/m/v0e8lzTjAQAJ, I think it's a bit too early to jump on that patch. The problem seems to an extreme edge case and the proposed solution is not yet released or even in Scintilla mainline. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3551#issuecomment-1746273751 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] RFC: Pre-release version bump to 2.0 (PR #3569)
I'm on the 2.0 band wagon since a long time :-) -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3569#issuecomment-1746221351 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] RFC: Pre-release version bump to 2.0 (PR #3569)
@kugel- approved this pull request. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3569#pullrequestreview-1656752990 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/infrastructure] Builders: Use "posix" variant of the mingw64 cross compiler (PR #11)
@kugel- approved this pull request. -- Reply to this email directly or view it on GitHub: https://github.com/geany/infrastructure/pull/11#pullrequestreview-1656752200 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/infrastructure] Builders: Use "posix" variant of the mingw64 cross compiler (PR #11)
Following the meta-criticism on https://github.com/geany/geany/pull/3568, why can't we make such a change on Geany? -- Reply to this email directly or view it on GitHub: https://github.com/geany/infrastructure/pull/11#issuecomment-1746219630 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] CI: Use mingw posix crosscompiler (PR #3568)
@kugel- approved this pull request. Meta-criticism: I dislike that that we (apparently) cannot fix this in Geany alone but need a change on geany/infrastructure. Regular contributors are not really able to propose such changes. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3568#pullrequestreview-1656750638 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Update Scintilla to 5.3.7 and Lexilla to 5.2.7 (PR #3551)
Apart from that (we can merge that patch individually if we agree on it), is there anything blocking? The PR adds new string thanks to the change history feature so need to merge before the string freeze -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3551#issuecomment-1745489734 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Renamed pep8 to pycodestyle (#1776)
Merged #1776 into master. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1776#event-10504179477 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Renamed pep8 to pycodestyle (#1776)
@kugel- approved this pull request. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1776#pullrequestreview-1649689904 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Renamed pep8 to pycodestyle (#1776)
Seems trivial and enough time has passed. @Akronix can you resolve the conflict? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/1776#issuecomment-1738071821 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Update Scintilla to 5.3.7 and Lexilla to 5.2.7 (PR #3551)
AFAIK scintilla needs C++17 since version 4? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3551#issuecomment-1736868344 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Update Scintilla to 5.3.7 and Lexilla to 5.2.7 (PR #3551)
I added a commit to enable the change history since it was trivial. Seems like a nice, user-visible change to take along (gives a bit of a modern feeling). Before saving  After saving  -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3551#issuecomment-1736776043 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Update Scintilla to 5.3.7 and Lexilla to 5.2.7 (PR #3551)
@kugel- pushed 1 commit. e4feb5cab6e4e2b878fae7be7cbd20ceb729fedf Enable new Scintilla "Change History" -- View it on GitHub: https://github.com/geany/geany/pull/3551/files/4e4b3ed43d594577d1738759e2d99aae1358049a..e4feb5cab6e4e2b878fae7be7cbd20ceb729fedf You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Update Scintilla to 5.3.7 and Lexilla to 5.2.7 (PR #3551)
https://github.com/ScintillaOrg/lexilla/issues/206 fixed after the 5.2.7 release. I did not backport the fix. I think R is not relevant enough to carry an additional change to Lexilla, or is it? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3551#issuecomment-1734930329 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Update Scintilla to version 5.3.6 (PR #3551)
@kugel- pushed 3 commits. e8505313619d77a162f4a0cb620b9aee28dbb22c Update scintilla_changes.patch b7655b6920d213f8de8dfd102fe8aacae69a70ab Update to Scintilla 5.3.7 and Lexilla 5.2.7 4e4b3ed43d594577d1738759e2d99aae1358049a Update scintilla/scintilla_changes.patch and update-scintilla.sh -- View it on GitHub: https://github.com/geany/geany/pull/3551/files/1caf9e0708c1ee4f3747fa6005fcc55aca35bb40..4e4b3ed43d594577d1738759e2d99aae1358049a You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Update Scintilla to version 5.3.6 (PR #3551)
@eht16 do you know what's wrong with the Windows CI? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3551#issuecomment-1734884952 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Use gtk_show_uri_on_window() in utils_open_browser() by default (PR #3178)
Merged #3178 into master. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3178#event-10470363906 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Update Scintilla to version 5.3.6 (PR #3551)
> @kugel- don't forget > [this](https://sourceforge.net/p/scintilla/code/ci/652df80a41a010f404be51410d921f2df8b7175a/), > fixes crasher in > [geany/geany-plugins#1272](https://github.com/geany/geany-plugins/issues/1272) Scintilla 5.3.7 was released including the fix. I'll bump to that version. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3551#issuecomment-1732677726 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Hide autocompletion and calltip popups when code scrolled (PR #3560)
LGBI -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3560#issuecomment-1732530464 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Update Scintilla to version 5.3.6 (PR #3551)
@kugel- pushed 4 commits. 32a450f4922419d0771be2479b7b97b6a363fa3a fixup! Map new python "attribute" mapping to identifier_2, by default. aeee043f1942e2c3e88fcda3cff5f89fdf35c773 Map new ruby lex classes to existing styles, for now. 37b15b0cf9739c04ff50f53944e2874b095e52b6 Handle new styles for strings in R (rawstrings, backticks, espaces) 1caf9e0708c1ee4f3747fa6005fcc55aca35bb40 Handle new nodepath style for gdscript -- View it on GitHub: https://github.com/geany/geany/pull/3551/files/55aabfa14d91751dca373c9e982dce3d9dea97d6..1caf9e0708c1ee4f3747fa6005fcc55aca35bb40 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Update lexilla to 5.2.4 (PR #3441)
#3551 already includes lexilla 5.2.6, no need to apply this on-top. This the author isn't replying anyway I would just close this. We must not forget the GDScript fix though. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3441#issuecomment-1722315549 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Fix keyword colorization on filetype change (PR #3553)
I read that but it's not clear to me how that explains the change here. You talk about a failed parser but that is not talked about here. So I assumed @techee came to a different conclusion. -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3553#issuecomment-1713242424 You are receiving this because you are subscribed to this thread. Message ID:
[Github-comments] Re: [geany/geany] Fix keyword colorization on filetype change (PR #3553)
Have you checked if this is perhaps a bug in scintilla and maybe fixed by #3551? -- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/3553#issuecomment-1713227952 You are receiving this because you are subscribed to this thread. Message ID:
