Construct a dummy TMTag array containing keywords, filter them similarly to workspace and global tags and show them together with normal tags in the autocomplete popup.
Could probably be optimized by not doing all the stuff all the time but the CPU time spent on this is probably just a noise compared to all the TM stuff. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1146
-- Commit Summary --
* Add keywords to the autocompletion list
-- File Changes --
M src/editor.c (24) M src/highlighting.c (20) M src/highlighting.h (2) M tagmanager/src/tm_workspace.c (6) M tagmanager/src/tm_workspace.h (2)
-- Patch Links --
https://github.com/geany/geany/pull/1146.patch https://github.com/geany/geany/pull/1146.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/1146
@@ -1776,3 +1776,23 @@ gboolean highlighting_is_code_style(gint lexer, gint style) return !(highlighting_is_comment_style(lexer, style) || highlighting_is_string_style(lexer, style)); }
+gchar **highlighting_get_keywords(GeanyFiletypeID filetype_id) +{
- GString *str = g_string_sized_new(1000);
- gchar **keywords;
- gchar **keyword_str;
- foreach_strv(keyword_str, style_sets[filetype_id].keywords)
- {
g_string_append(str, *keyword_str);g_string_append_c(str, ' ');- }
- keywords = g_strsplit(str->str, " ", -1);
huh, what? You build a string out of a list to make a list out of that string? Why not simply copy `style_sets[filetype_id].keywords` if it's already and strv?
--- 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/1146/files/1e74c11aca41274d690d8da0b6e59...
@@ -1776,3 +1776,23 @@ gboolean highlighting_is_code_style(gint lexer, gint style) return !(highlighting_is_comment_style(lexer, style) || highlighting_is_string_style(lexer, style)); }
+gchar **highlighting_get_keywords(GeanyFiletypeID filetype_id) +{
- GString *str = g_string_sized_new(1000);
- gchar **keywords;
- gchar **keyword_str;
- foreach_strv(keyword_str, style_sets[filetype_id].keywords)
- {
g_string_append(str, *keyword_str);g_string_append_c(str, ' ');- }
- keywords = g_strsplit(str->str, " ", -1);
Nope: the thing is style_sets[filetype_id].keywords contains (for C) at index 0 a space-separated string of primary keywords, at index 1 a space separated string of secondary keywords, at index 2 a space separated string of docstrings. I concatenate those so there's one big space-separated string containing everything and split it by space.
--- 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/1146/files/1e74c11aca41274d690d8da0b6e59...
@@ -1776,3 +1776,23 @@ gboolean highlighting_is_code_style(gint lexer, gint style) return !(highlighting_is_comment_style(lexer, style) || highlighting_is_string_style(lexer, style)); }
+gchar **highlighting_get_keywords(GeanyFiletypeID filetype_id) +{
- GString *str = g_string_sized_new(1000);
- gchar **keywords;
- gchar **keyword_str;
- foreach_strv(keyword_str, style_sets[filetype_id].keywords)
- {
g_string_append(str, *keyword_str);g_string_append_c(str, ' ');- }
- keywords = g_strsplit(str->str, " ", -1);
oh, okay. my bad then
--- 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/1146/files/1e74c11aca41274d690d8da0b6e59...
g_return_val_if_fail(editor, FALSE);
- tags = tm_workspace_find_prefix(root, ft->lang, editor_prefs.autocompletion_max_entries);
- /* Get keywords and create a dummy TMTag array from them. These tags are
* then also searched when searching workspace */- keywords = highlighting_get_keywords(ft->id);
- keyword_array = g_ptr_array_new_full(100, (GDestroyNotify)tm_tag_unref);
- foreach_strv(keyword, keywords)
- {
if (**keyword != '\0'){TMTag *tag = tm_tag_new();tag->name = g_strdup(*keyword);tag->lang = ft->lang;
shouldn't it have a type for autoc to work best?
--- 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/1146/files/1e74c11aca41274d690d8da0b6e59...
g_return_val_if_fail(editor, FALSE);
- tags = tm_workspace_find_prefix(root, ft->lang, editor_prefs.autocompletion_max_entries);
- /* Get keywords and create a dummy TMTag array from them. These tags are
* then also searched when searching workspace */- keywords = highlighting_get_keywords(ft->id);
- keyword_array = g_ptr_array_new_full(100, (GDestroyNotify)tm_tag_unref);
- foreach_strv(keyword, keywords)
- {
if (**keyword != '\0'){TMTag *tag = tm_tag_new();tag->name = g_strdup(*keyword);tag->lang = ft->lang;
This is a non-scope search (scope search doesn't make much sense for keywords, they in general don't appear after "." at least for the languages I know) and type isn't used anywhere in tm_workspace_find_prefix().
--- 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/1146/files/1e74c11aca41274d690d8da0b6e59...
@techee pushed 1 commit.
1998dc0 Show different icon for keywords in the autocompletion popup
--- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/geany/geany/pull/1146/files/1e74c11aca41274d690d8da0b6e59...
I've added a patch to show a different icon (I wasn't quite sure which one to use) for keywords in the popup.
(I managed to push the branch by accident to Geany repo - I deleted it immediately after realizing 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/1146#issuecomment-234646104
(I managed to push the branch by accident to Geany repo - I deleted it immediately after realizing it.)
If you want to avoid accidental push, you can make the `origin` read-only (e.g. `git://`), and have a different remote for actually pushing. That's what I have for GP for example.
--- 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/1146#issuecomment-234664125
If you want to avoid accidental push, you can make the origin read-only (e.g. git://), and have a different remote for actually pushing. That's what I have for GP for example.
Good trick, 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/1146#issuecomment-234668285
@@ -1776,3 +1776,23 @@ gboolean highlighting_is_code_style(gint lexer, gint style) return !(highlighting_is_comment_style(lexer, style) || highlighting_is_string_style(lexer, style)); }
+gchar **highlighting_get_keywords(GeanyFiletypeID filetype_id) +{
- GString *str = g_string_sized_new(1000);
- gchar **keywords;
- gchar **keyword_str;
- foreach_strv(keyword_str, style_sets[filetype_id].keywords)
- {
g_string_append(str, *keyword_str);g_string_append_c(str, ' ');- }
- keywords = g_strsplit(str->str, " ", -1);
This is definitively a good candidate for a source code comment explaining what was explained in the comments here. My eyes almost popped out of my head when I saw this code (in the accidentally pushed branch) that @techee of all people would write something so ridiculously indirect and inefficient :)
Maybe something like:
Scintilla requires keywords as a space-separated string so that's how we store them. Concat all of those strings together and then split them once at the end.
--- 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/1146/files/1e74c11aca41274d690d8da0b6e59...
@@ -1776,3 +1776,23 @@ gboolean highlighting_is_code_style(gint lexer, gint style) return !(highlighting_is_comment_style(lexer, style) || highlighting_is_string_style(lexer, style)); }
+gchar **highlighting_get_keywords(GeanyFiletypeID filetype_id) +{
- GString *str = g_string_sized_new(1000);
- gchar **keywords;
- gchar **keyword_str;
- foreach_strv(keyword_str, style_sets[filetype_id].keywords)
This could be replaced with the more succinct and idiomatic:
```c for (const char **kwd = style_sets[filetype_id].keywords; *kwd; kwd++) ```
Those `foreach_` macros only obfuscate the code and offer no benefit, IMO.
--- 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/1146/files/1e74c11aca41274d690d8da0b6e59...
One thing that might be annoying is if this uses all of the keywords lists rather than just the language keywords, for example in the C++ lexer, IIRC one of the keyword lists is for doxygen comments.
--- 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/1146#issuecomment-234693167
The fact that the entries in keywords lists are added to the autocomplete lists should be documented in both autocomplete and filetype configuration sections of the manual.
--- 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/1146#issuecomment-234693290
Another potentially annoying example is the HTML lexer, which has keyword lists for several different unrelated languages that can be embedded in HTML. It would be weird if someone was writing a PHP script and getting Python or VBScript keywords offered in the auto-completion list.
--- 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/1146#issuecomment-234693419
Agree with @codebrainz about limiting to actual keywords, doxygen keywords are only significant in specific syntax positions (in doxycomments) not as general words.
Suggest a filetype setting to say which keyword lists to add to autocomplete, that way it can be adjusted per-filetype (and set to not do any too :)
--- 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/1146#issuecomment-234693794
I don't know about filetype setting, but for just generally having some control over the lists, [a new flag could be added here](https://github.com/geany/geany/blob/master/src/highlightingmappings.h#L60) that says whether the list should be considered for auto-complete, similar to the existing one about whether they should be type names.
--- 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/1146#issuecomment-234697931
IMHO settings should be in the files, not hard coded, but so long as it can be controlled per filetype is a start.
--- 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/1146#issuecomment-234698471
@@ -1776,3 +1776,23 @@ gboolean highlighting_is_code_style(gint lexer, gint style) return !(highlighting_is_comment_style(lexer, style) || highlighting_is_string_style(lexer, style)); }
+gchar **highlighting_get_keywords(GeanyFiletypeID filetype_id) +{
- GString *str = g_string_sized_new(1000);
- gchar **keywords;
- gchar **keyword_str;
- foreach_strv(keyword_str, style_sets[filetype_id].keywords)
- {
g_string_append(str, *keyword_str);g_string_append_c(str, ' ');- }
- keywords = g_strsplit(str->str, " ", -1);
:-D
Yeah, a comment is a good idea because it really looks silly when you expect the keywords list is already separated. I actually expected this myself when I was writing the patch and just returned keywords and was then surprised by the results.
--- 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/1146/files/1e74c11aca41274d690d8da0b6e59...
Yep, agree it should be possible to specify which ones should be used for autocompletion. I think what @codebrainz suggests should be sufficient - the group names (expressing their semantics, i.e. whether it's keyword, docstring or something else) are already hard-coded so there shouldn't be a need to change the autocompletion dynamically inside filetype settings.
(Meh, it will be annoying to do it for all the filetypes.)
--- 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/1146#issuecomment-234707373
@@ -1776,3 +1776,23 @@ gboolean highlighting_is_code_style(gint lexer, gint style) return !(highlighting_is_comment_style(lexer, style) || highlighting_is_string_style(lexer, style)); }
+gchar **highlighting_get_keywords(GeanyFiletypeID filetype_id) +{
- GString *str = g_string_sized_new(1000);
- gchar **keywords;
- gchar **keyword_str;
- foreach_strv(keyword_str, style_sets[filetype_id].keywords)
yet your example replacement has a bug, it doesn't handle `NULL`, which I wouldn't find unexpected in this context (and may even be a valid strv, not sure from the top of my head) :)
But yeah, those macros don't really make the code clearer.
--- 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/1146/files/1e74c11aca41274d690d8da0b6e59...
Yep, agree it should be possible to specify which ones should be used for autocompletion. I think what @codebrainz suggests should be sufficient - the group names (expressing their semantics, i.e. whether it's keyword, docstring or something else) are already hard-coded so there shouldn't be a need to change the autocompletion dynamically inside filetype settings.
Agreed.
@elextr Once highlighting mappings are gone and part of the filetypes files, sure, move that too :wink:
(Meh, it will be annoying to do it for all the filetypes.)
Probably, though most filetypes will just have one keywords set and the exact same change will just have to be copied over and over.
----
BTW, do we want to have a setting for that, like we have for document's words?
--- 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/1146#issuecomment-234717817
Once highlighting mappings are gone and part of the filetypes files ....
Oh good @b4n is working on it :wink:
--- 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/1146#issuecomment-234718337
@@ -1776,3 +1776,23 @@ gboolean highlighting_is_code_style(gint lexer, gint style) return !(highlighting_is_comment_style(lexer, style) || highlighting_is_string_style(lexer, style)); }
+gchar **highlighting_get_keywords(GeanyFiletypeID filetype_id) +{
- GString *str = g_string_sized_new(1000);
- gchar **keywords;
- gchar **keyword_str;
- foreach_strv(keyword_str, style_sets[filetype_id].keywords)
Ok, change the test to `kwd && *kwd`. I'm not actually sure what `foreach_strv` does though, at first I assumed, but after having just looked at its definition, I'm much less sure what it does (loops through each char in each element in the string vector?).
--- 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/1146/files/1e74c11aca41274d690d8da0b6e59...
@@ -1776,3 +1776,23 @@ gboolean highlighting_is_code_style(gint lexer, gint style) return !(highlighting_is_comment_style(lexer, style) || highlighting_is_string_style(lexer, style)); }
+gchar **highlighting_get_keywords(GeanyFiletypeID filetype_id) +{
- GString *str = g_string_sized_new(1000);
- gchar **keywords;
- gchar **keyword_str;
- foreach_strv(keyword_str, style_sets[filetype_id].keywords)
loops through each char in each element in the string vector?
After I manually expanded the macro, I see it uses `foreach_str` to accomplish my above loop, rather than to iterate through the chars in a C string like its document says it's meant for. I guess it was re-used with different semantics to save an additional line of normal everyday readable C code in the macro itself.
--- 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/1146/files/1e74c11aca41274d690d8da0b6e59...
@@ -1776,3 +1776,23 @@ gboolean highlighting_is_code_style(gint lexer, gint style) return !(highlighting_is_comment_style(lexer, style) || highlighting_is_string_style(lexer, style)); }
+gchar **highlighting_get_keywords(GeanyFiletypeID filetype_id) +{
- GString *str = g_string_sized_new(1000);
- gchar **keywords;
- gchar **keyword_str;
- foreach_strv(keyword_str, style_sets[filetype_id].keywords)
The use of the macro within the macro doesn't even save a line, it saves about 11 chars by my calculation :(
--- 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/1146/files/1e74c11aca41274d690d8da0b6e59...
@@ -1776,3 +1776,23 @@ gboolean highlighting_is_code_style(gint lexer, gint style) return !(highlighting_is_comment_style(lexer, style) || highlighting_is_string_style(lexer, style)); }
+gchar **highlighting_get_keywords(GeanyFiletypeID filetype_id) +{
- GString *str = g_string_sized_new(1000);
- gchar **keywords;
- gchar **keyword_str;
- foreach_strv(keyword_str, style_sets[filetype_id].keywords)
@elextr but if C ever changes the way `for` works, you only have to update one place :)
--- 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/1146/files/1e74c11aca41274d690d8da0b6e59...
I repushed the patches with the possibility to enable/disable autocompletion for certain keyword groups. In general I kept autocompletion enabled for everything that seemed to be keyword (type or whatever) in the given language. It's off only for things like docstrings, languages embedded inside HTML and when the keyword group is a specific subset of a language which isn't commonly used (SGML keywords in XML).
XML, PHP and Zephir are using HTML keywords - I had to modify PHP (which means Zephir too) to perform autocompletion for PHP keywords (which differs from HTML where the PHP keywords aren't autocompleted). I also kept HTML autocompletion for PHP files because most commonly they are a HTML (I can change it if anyone thinks it's a bad idea).
As I had to access HLKeyword I would have to make it accessible in StyleSet to be able to perform the check whether to use the given keyword group; instead I decided to construct the autocompletion keyword list already when initializing languages which also reduces the amount of time spent later when performing autocompletion.
Finally I changed the icon in the popup to "classviewer-member" which looks better IMO.
--- 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/1146#issuecomment-234762435
BTW, do we want to have a setting for that, like we have for document's words?
In general I think the less configuration options the better but if the keyword autocompletion turns out annoying in some cases, the option can be added.
--- 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/1146#issuecomment-234762751
Agree with @b4n it must have an option to turn it off.
Because C++ completions are crap I run with autocompletes off. I don't want keywords to suddenly start annoying me when all other autocompletions are off.
--- 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/1146#issuecomment-234762947
Because C++ completions are crap I run with autocompletes off. I don't want keywords to suddenly start annoying me when all other autocompletions are off.
Keyword autocompletion is off when "Autocomplete symbols" is off. The question is whether there should be a separate knob for just keyword autocompletion.
(If there's just one option then "Autocomplete symbols" should probably be renamed to "Autocomplete symbols and keywords".)
--- 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/1146#issuecomment-234763283
Ok, hooking it to symbols is fine for 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/pull/1146#issuecomment-234763584
I've repushed with a fix for a crash for an "unknown" filetype - I used GPtrArray in the previous patch which wasn't initialized in this case and keyword list was NULL which caused the crash. I converted the list to GSList whose NULL value is a valid empty list. Also updated documentation and settings label (for the case when single config option is used both for symbols and keywords).
--- 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/1146#issuecomment-235408405
github-comments@lists.geany.org