[Github-comments] [geany/geany] Enable local variables for C/C++ and improve autocompletion (PR #3185)

[Thomas Martitz]

@kugel- requested changes on this pull request.

I'm not really happy with b710cf3aa1300dd43e4edfcf152f7f3a10f9a686

- it makes a lot of assumption
- it seems really expensive
- works only for C/C++ by hardcoding
- has nothing to do with local variables

Please lets discuss that feature in a separate PR. Also, if we want that, then it may make sense to expose that "find header" logic somehow to implement a "go to header" feature (I think ProjectOrganizer already has something like that?)

>  	foreach_ptr_array(tmtag, i, tags)
 	{
+		if (tmtag->type & tm_tag_local_var_t &&
+			(doc->tm_file != tmtag->file ||
+			 current_line < tmtag->line ||

`tm_parser_var_valid_before_declare()` should be called here, no?

Also, the whole condition is basically a duplicate of the same in `is_valid_autocomplete_tag()`. Can `is_valid_autocomplete_tag()` be made callable from here (perhaps with a different name)?

> @@ -601,6 +601,7 @@ static void show_autocomplete(ScintillaObject *sci, gsize rootlen, GString *word
 	}
 	/* store whether a calltip is showing, so we can reshow it after autocompletion */
 	calltip.set = (gboolean) SSM(sci, SCI_CALLTIPACTIVE, 0, 0);
+	SSM(sci, SCI_AUTOCSETORDER, SC_ORDER_CUSTOM, 0);

I read the docs but I couldn't grasp the difference between `SC_ORDER_PRESORTED` and `SC_ORDER_CUSTOM`. In either case scintilla does not sort by itself. `SC_ORDER_PRESORTED` seems to assume the list is presorted alphabetically but what's the deal with that?

> +		!(tag->type & tm_tag_local_var_t) &&
+		is_autocomplete_tag(tag, info);
+}
+
+
+static guint copy_tags(GPtrArray *dst, TMTag **src, guint src_len, GHashTable *name_table,
+	gint num, gboolean (*predicate) (TMTag *, CopyInfo *), CopyInfo *info)
+{
+	guint i;
+
+	g_return_val_if_fail(src && dst, 0);
+
+	for (i = 0; i < src_len && num > 0; i++)
+	{
+		TMTag *tag = *src;
+		if (predicate(tag, info) &&

Every predicate function contains the is_autocomplete_tag() check. I think it makes sense to have that call here (before calling the predicate). The function could be suitably named `copy_autocomplete_tags()` 

That should perform a bit better

-- 
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3185#pullrequestreview-958518326
You are receiving this because you are subscribed to this thread.

Message ID: <geany/geany/pull/3185/review/958518326 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20220430/f4590b74/attachment.htm>