Patch from https://github.com/geany/geany/pull/1177#issuecomment-241438625, plus an attempt at considering the current scope (which is likely riddled with bad stuff, but still a POC).
I can drop the second (toy) patch if the first is interesting enough, and possibly make a separate PR afterward if it's somehow interesting still. In any case, reviewing each commit separately is likely a better approach.
@techee @krogank9 You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1188
-- Commit Summary --
* Add support for scoped calltips * FXIME: Attempt at using the current (implicit) scope for symbol scope search
-- File Changes --
M src/editor.c (155)
-- Patch Links --
https://github.com/geany/geany/pull/1188.patch https://github.com/geany/geany/pull/1188.diff
@b4n pushed 2 commits.
cd5c383 Don't dedup tags used for calltips in order to support overloading 342e636 Make sure to sort get_tags_for_current_scope() result
Neat, the scoped completion is definitely a step in the right direction. Havn't tried the last two commits though.
}g_ptr_array_free(tags, TRUE);}if (! sep_pos)break;}g_free(scope);/* root tags */if (prefix)tags = tm_workspace_find_prefix(name, ft->lang, 0xffff);elsetags = tm_workspace_find(name, NULL, tm_tag_max_t, NULL, ft->lang);
tm_workspace_find() doest dedup which is probably not what you want for autocompletion
}g_ptr_array_free(tags, TRUE);}if (! sep_pos)break;}g_free(scope);/* root tags */if (prefix)tags = tm_workspace_find_prefix(name, ft->lang, 0xffff);elsetags = tm_workspace_find(name, NULL, tm_tag_max_t, NULL, ft->lang);
I later altered everything to manually dedup (in the autoc side) so I can chose depending on calltip/autoc
@@ -794,6 +888,8 @@ static gboolean autocomplete_scope(GeanyEditor *editor, const gchar *root, gsize g_ptr_array_add(filtered, tag); }
tm_tags_dedup(filtered, sort_attr, FALSE);
Ah, didnt see that, nevermind above
@@ -1906,9 +2002,27 @@ static gchar *find_calltip(const gchar *word, GeanyFiletype *ft)
g_return_val_if_fail(ft && word && *word, NULL);
- /* use all types in case language uses wrong tag type e.g. python "members" instead of "methods" */
- tags = tm_workspace_find(word, NULL, tm_tag_max_t, NULL, ft->lang);
- if (tags->len == 0)
- tags = get_scoped_tags(editor, pos - strlen(word));
- if (tags)
- {
GPtrArray *filtered = g_ptr_array_new();foreach_ptr_array(tag, i, tags){if (strcmp(tag->name, word) == 0)g_ptr_array_add(filtered, tag);
What does this filter do? get_scoped_tags() already limits the found tags to the word at pos, doesn't it?
The code could be simpler with #1187 :-) E.g. get_tags_for_current_scope() could be lots simpler: 1) setup query with name and no scope for the root tags, then 2) just add the scope filter and re-execute the query to get the scoped ones.
@@ -1906,9 +2002,27 @@ static gchar *find_calltip(const gchar *word, GeanyFiletype *ft)
g_return_val_if_fail(ft && word && *word, NULL);
- /* use all types in case language uses wrong tag type e.g. python "members" instead of "methods" */
- tags = tm_workspace_find(word, NULL, tm_tag_max_t, NULL, ft->lang);
- if (tags->len == 0)
- tags = get_scoped_tags(editor, pos - strlen(word));
- if (tags)
- {
GPtrArray *filtered = g_ptr_array_new();foreach_ptr_array(tag, i, tags){if (strcmp(tag->name, word) == 0)g_ptr_array_add(filtered, tag);
No it does not, it searches members of a scope, doesn't filter by names (it supports getting members without specifying a name). And this part is basically just copied from L880 onwards
Well, that code is pretty hacky for the moment, and I guess it shouldn't use `get_scope_members` at all but instead just use the scope parameter to `tm_workspace_find()` -- though sadly `tm_wokspace_find_prefix()` doesn't have it, but it's fairly easy to fix -- and I believe your proposed PR does it anyway, with or without the extra query API.
Not saying your module wouldn't help, but that the code here is likely to be easily improved a lot in any case. I hacked that very carelessly just as a proof of concept.
@@ -1906,9 +2002,27 @@ static gchar *find_calltip(const gchar *word, GeanyFiletype *ft)
g_return_val_if_fail(ft && word && *word, NULL);
- /* use all types in case language uses wrong tag type e.g. python "members" instead of "methods" */
- tags = tm_workspace_find(word, NULL, tm_tag_max_t, NULL, ft->lang);
- if (tags->len == 0)
- tags = get_scoped_tags(editor, pos - strlen(word));
- if (tags)
- {
GPtrArray *filtered = g_ptr_array_new();foreach_ptr_array(tag, i, tags){if (strcmp(tag->name, word) == 0)g_ptr_array_add(filtered, tag);
Still confused. get_scope_tags() finds the function name (since pos is setup up to point to it). This is then passed to tm_workspace_find_scope_members() (and then tm_workspace_find()). Therefore the returned tags are already filtered by the function name (assuming scope_sep_typed being FALSE).
@@ -1906,9 +2002,27 @@ static gchar *find_calltip(const gchar *word, GeanyFiletype *ft)
g_return_val_if_fail(ft && word && *word, NULL);
- /* use all types in case language uses wrong tag type e.g. python "members" instead of "methods" */
- tags = tm_workspace_find(word, NULL, tm_tag_max_t, NULL, ft->lang);
- if (tags->len == 0)
- tags = get_scoped_tags(editor, pos - strlen(word));
- if (tags)
- {
GPtrArray *filtered = g_ptr_array_new();foreach_ptr_array(tag, i, tags){if (strcmp(tag->name, word) == 0)g_ptr_array_add(filtered, tag);
hum… no, `get_scope_tags()` should find `this` in `this.member()`. And anyway, `tm_workspace_find_scope_members()` finds *all children on the given scope*, not filtered by any name.
Though, I can't really fully defend this, `pos - strlen(word)` might be a bit of luck, and might only work for 1-byte scope separators. But it odes seem to work somehow :]
@@ -1906,9 +2002,27 @@ static gchar *find_calltip(const gchar *word, GeanyFiletype *ft)
g_return_val_if_fail(ft && word && *word, NULL);
- /* use all types in case language uses wrong tag type e.g. python "members" instead of "methods" */
- tags = tm_workspace_find(word, NULL, tm_tag_max_t, NULL, ft->lang);
- if (tags->len == 0)
- tags = get_scoped_tags(editor, pos - strlen(word));
- if (tags)
- {
GPtrArray *filtered = g_ptr_array_new();foreach_ptr_array(tag, i, tags){if (strcmp(tag->name, word) == 0)g_ptr_array_add(filtered, tag);
Though, I can't really fully defend this, pos - strlen(word) might be a bit of luck, and might only work for 1-byte scope separators. But it odes seem to work somehow :]
It's alright - you just want to skip the function name here, the scope separators get skipped in get_scoped_tags().
@@ -1906,9 +2002,27 @@ static gchar *find_calltip(const gchar *word, GeanyFiletype *ft)
g_return_val_if_fail(ft && word && *word, NULL);
- /* use all types in case language uses wrong tag type e.g. python "members" instead of "methods" */
- tags = tm_workspace_find(word, NULL, tm_tag_max_t, NULL, ft->lang);
- if (tags->len == 0)
- tags = get_scoped_tags(editor, pos - strlen(word));
- if (tags)
- {
GPtrArray *filtered = g_ptr_array_new();foreach_ptr_array(tag, i, tags){if (strcmp(tag->name, word) == 0)g_ptr_array_add(filtered, tag);
@kugel- This is a good example why I suggested the new query interface operates on tag arrays instead - especially with scope completion you have to work with tag arrays which are results of a much more complicated query than what your query interface can handle and further filtering isn't possible using your API.
- tags = tm_workspace_find(word, NULL, tm_tag_max_t, NULL, ft->lang);
- tags = get_scoped_tags(editor, pos - strlen(word));
- if (tags)
- {
GPtrArray *filtered = g_ptr_array_new();foreach_ptr_array(tag, i, tags){if (strcmp(tag->name, word) == 0)g_ptr_array_add(filtered, tag);}g_ptr_array_free(tags, TRUE);tags = filtered;- }
- else
To be sure the number of filtered tags from above could be checked here and if there are 0, fall back to searching all tags.
- if (tags)
- {
GPtrArray *filtered = g_ptr_array_new();foreach_ptr_array(tag, i, tags){if (strcmp(tag->name, word) == 0)g_ptr_array_add(filtered, tag);}g_ptr_array_free(tags, TRUE);tags = filtered;- }
- else
- {
/* use all types in case language uses wrong tag type e.g. python "members" instead of "methods" */
I know this is from before this patch but is there some reason not to use the correct tag type for Python methods?
- if (tags)
- {
GPtrArray *filtered = g_ptr_array_new();foreach_ptr_array(tag, i, tags){if (strcmp(tag->name, word) == 0)g_ptr_array_add(filtered, tag);}g_ptr_array_free(tags, TRUE);tags = filtered;- }
- else
- {
/* use all types in case language uses wrong tag type e.g. python "members" instead of "methods" */
Isn't that determined by` ctags/parsers/python.c` so it needs to be "fixed" upstream?
What worries me in eeac1ca is it will drop tags where explicit scope gets removed by e.g.
``` using namespace Foo; ```
or
``` from math import * ```
and similar so it won't show all the tags available at the given scope.
@techee ATM it shows so much irrelevant shit that its not useful, at least this trims it somewhat.
Including something in the list because it _might_ be relevant due to a local import we don't parse is not right. The right thing is to parse local declarations and that would include imports, then we know which names are available, including the locals which are _not_ included ATM.
Basically this PR and the other similar ones is what we call here "putting lipstick on a pig", they are not addressing the real problem and spending lots of effort fiddling around the edges for no gain.
@elextr You are welcome to "address the real problem" and submit patches. Create 30 great parsers for all the languages that do everything right, create a fantastic tag management system that holds the complete contextual information and that autocompletes the right symbol 100% of the time. Maybe instead of saying it's shit every time TM is mentioned you could actually do something about it.
Now I am an ordinary human with limited resources and will keep "putting lipstick on a pig" by both improving the ctags parsers and the tag manager.
Also I have a very different experience with the parsers and tag management than you - I'm super happy about the results. The most important for me is goto symbol/tag definition which works flawlessly but I quite like the "stupid" autocompletion where on the one hand one has to type quite a lot to reduce the number of symbols in the list but where on the other hand one can be sure that the symbol he types exists when the autocompletion popup is still open (more like typo-correction than autocompletion). And this is something I'm afraid could be gone with eeac1ca.
... (more like typo-correction than autocompletion). And this is something I'm afraid could be gone with eeac1ca.
But maybe it could work like scope completion does now - first show a popup with narrowed selection and if what has been typed doesn't match anything in the popup any more, switch back to normal autocompletion.
What worries me in eeac1ca is it will drop tags where explicit scope gets removed by e.g.
Good point, it can be annoying
But maybe it could work like scope completion does now - first show a popup with narrowed selection and if what has been typed doesn't match anything in the popup any more, switch back to normal autocompletion.
That's what it does now for the autocomplete AFAICT. But not for calltips I think.
Anyway, I'll create a separate PR for the scoped calltip part, so we can discuss the rest to the bone without making that part ignored in the process :)
@elextr yeah we don't have something too intelligent for completion, as @techee said it's somewhat more "autocorrect" with enhanced capabilities. But like him, I still like it :)
If you want true awesome completion, I'm afraid there's only one direction to go: make the completion pluggable in a way that allows plugins to provide their own completion as they wish, their own symbol tree, etc, so it's easy to implement plugins using things like libclang and others. It has annoying downsides like the need to setup a complete "project" with build options and all -- goodbye "build system is enough". But that means specific work for *each* language, with virtually nothing that could be shared. I guess such an API needs to allow plugins to show the autocomplete and calltip boxes, fill them, and provide TMTags. Maybe other things.
Still, as @techee said, if we get a nicely groomed pig, "all" we need need are good parsers (which we work on, and everyone at *ctags does, too, which profits many apparently), and that gives something still very nice to many people and more exotic languages for which nobody will have written a full plugin with true language insight.
It has annoying downsides like the need to setup a complete "project" with build options and all -- goodbye "build system is enough".
This is exactly what I love about Geany - whatever sources you feed it with, it parses them without knowing how to build them - whatever build system is used. And most opensource projects use autoconf/automake and it's impossible to have a look at configure.ac all m4 and makefiles and know which way ti will be built this way (moreover it depends on what's installed in the system, the config flags given during configure etc.). So every project would have to be setup manually by providing all compiler flags, defines, dependencies, etc. That's undoable for anything bigger.
This doesn't mean I would be against e.g. the clang parser @codebrainz did but I'd really want to keep the current way of parsing preserved for people who want it for the reasons above (it could be disabled by a plugin if someone wishes to do something else).
yeah we don't have something too intelligent for completion, as @techee said it's somewhat more "autocorrect" with enhanced capabilities. But like him, I still like it :)
Clearly our experiences are different, that may depend on what you mostly code and how its coded, but my experience with autocomplete is that its so bad I almost always run with it off. And its perfectly legitimate for me to keep saying its bad, so you guys don't forget that your experience is not the same for everyone :)
goodbye "build system is enough"
Totally agree that losing this from Geany would be sad, I don't even use the project plugins ...
make the completion pluggable in a way that allows plugins to provide their own completion as they wish
... but this means you don't have to lose the existing system unless you want to. Any project capabilities that a specific language needs can be handled by the plugin. Just because its pluggable doesn't mean that a simple default isn't available in Geany. But we don't like making language things flexible and available to plugins, just look at the reactions to #1187 and previous discussions on autoindentation plugins etc. And thats part of why I keep reminding that the current system isn't adequate, and therefore that per language flexibility is needed. [warning standard elextr proclamation] Not all languages are C.
I'm not sure I would agree that a better (but not neccessarily perfect) system needs the full-fat project capabilities anyway, I would guess that, for me, maybe 90% of name strings start with a _local_ variable or type name, so simply parsing the locals, which the ctags parsers (for C/C++) can actually do, but we don't use, and restricting names when locals are found would go a huge way to making it usable again.
It has annoying downsides like the need to setup a complete "project" with build options and all -- goodbye "build system is enough".
It's almost surely unique to libclang, but a "build system is enough" is actually accurate. The libclang API can read so-called "[compilation database](http://clang.llvm.org/docs/JSONCompilationDatabase.html)" (a simple JSON file) which is designed to be generated by the build system so there's nothing extra to configure. I know at least CMake supports generating the compilation database and I don't expect it to be terribly difficult to add support to an Autotools build system, or using something like [Bear](https://github.com/rizsotto/Bear) for Autotools or plain make.
I'm not sure I would agree that a better (but not neccessarily perfect) system needs the full-fat project capabilities anyway, I would guess that, for me, maybe 90% of name strings start with a local variable or type name, so simply parsing the locals, which the ctags parsers (for C/C++) can actually do, but we don't use, and restricting names when locals are found would go a huge way to making it usable again.
We need to first sync universal-ctags with our ctags for this (https://github.com/geany/geany/pull/1160 isn't sufficient to get the new cxx parser working)
- if (tags)
- {
GPtrArray *filtered = g_ptr_array_new();foreach_ptr_array(tag, i, tags){if (strcmp(tag->name, word) == 0)g_ptr_array_add(filtered, tag);}g_ptr_array_free(tags, TRUE);tags = filtered;- }
- else
- {
/* use all types in case language uses wrong tag type e.g. python "members" instead of "methods" */
Nope, we map the ctags types to our own representation in Geany now:
https://github.com/geany/geany/blob/master/src/tagmanager/tm_parser.c#L96
And methods seem to get mapped to tm_tag_method_t so maybe this comment is just outdated.
- if (tags)
- {
GPtrArray *filtered = g_ptr_array_new();foreach_ptr_array(tag, i, tags){if (strcmp(tag->name, word) == 0)g_ptr_array_add(filtered, tag);}g_ptr_array_free(tags, TRUE);tags = filtered;- }
- else
- {
/* use all types in case language uses wrong tag type e.g. python "members" instead of "methods" */
Oh I thought that was only for symbols display, not for storing tags inside TM.
- if (tags)
- {
GPtrArray *filtered = g_ptr_array_new();foreach_ptr_array(tag, i, tags){if (strcmp(tag->name, word) == 0)g_ptr_array_add(filtered, tag);}g_ptr_array_free(tags, TRUE);tags = filtered;- }
- else
- {
/* use all types in case language uses wrong tag type e.g. python "members" instead of "methods" */
In symbols.c there's yet another table which defines section names and icons used for individual tags:
https://github.com/geany/geany/blob/master/src/symbols.c#L463
- if (tags)
- {
GPtrArray *filtered = g_ptr_array_new();foreach_ptr_array(tag, i, tags){if (strcmp(tag->name, word) == 0)g_ptr_array_add(filtered, tag);}g_ptr_array_free(tags, TRUE);tags = filtered;- }
- else
- {
/* use all types in case language uses wrong tag type e.g. python "members" instead of "methods" */
ok.
github-comments@lists.geany.org