@techee commented on this pull request.

In src/pluginextension.h:

> +	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);

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.

Nothing like that is necessary - the plugin just invokes it based on whether it determines the highlighting is necessary which is in 2 situations which are quite obvious from the plugin writer's perspective:

• contents of the document changes, i.e. in SC_MOD_INSERTTEXT | SC_MOD_DELETETEXT scintilla notification
• a tab is switched - i.e. document-activate Geany signal

That's it (and I actually don't know when exactly Geany triggers it and don't really care). I think the logic when to do it is pretty clear and much simpler than having to forward it to Geany.

There's another related problem - Geany uses Scintilla's lexer keyword index

https://github.com/geany/geany/blob/9bf5769f582cbf3d82a1faca035e7f63e8908afe/src/document.c#L2722

and highlights those by setting the keywords using sci_set_keywords(). I pretty much copied this part and it's one type of highlighting. The trouble with this approach is:

1. It's limited to only those lexers that support it
2. It highlights all the occurrences of foo (if foo is defined as a type somewhere) no matter whether foo is actually used as a type in the particular file

This is why the colorization using INDIC_TEXTFORE was added in techee/geany-lsp#18 based on @elextr 's suggestion. This allows colorization of only specific positions in the document so when you have foo as a type and foo as a variable in the same document, the first foo gets colorized and the second one doesn't. (One limitation of this approach is that the text cannot be made bold, see the linked issue above for more discussion.)

This second mode wouldn't be possible with current Geany's colorization.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <geany/geany/pull/3849/review/2153611844@github.com>