[Github-comments] [geany/geany] Improve handling of anonymous tags (PR #3059)

[Thomas Martitz]

@kugel- requested changes on this pull request.



> @@ -818,6 +818,28 @@ void tm_parser_verify_type_mappings(void)
 }
 
 
+/* determine anonymous tags from tag names only when corresponding
+ * ctags information is not available */
+gboolean tm_parser_is_anon_name(TMParserType lang, gchar *name)
+{
+	guint i;
+	char dummy;
+
+	if (lang == TM_PARSER_C || lang == TM_PARSER_CPP)
+	{
+		return sscanf(name, "anon_%*[a-z]_%u%c", &i, &dummy) == 1 ||

Does sscanf really understand regexes? This is also new over the old code in `tm_tag_is_anon()`

> @@ -695,7 +695,7 @@ static void fill_find_tags_array_prefix(GPtrArray *dst, const GPtrArray *src,
 	for (i = 0; i < count && num < max_num; ++i)
 	{
 		if (tm_parser_langs_compatible(lang, (*tag)->lang) &&
-			!tm_tag_is_anon(*tag) &&
+			!((*tag)->flags & tm_tag_flag_anon_t) &&

IMO the this is not an improvement. Might keep `tm_tag_is_anon()` and reduce its implementation to just test the flag

> @@ -51,6 +51,3 @@
 main_diff = set(main_src_files) - set(main_dst_files)
 if main_diff:
     print('Files added to main: ' + str(main_diff))
-
-os.chdir(dstdir)
-os.system('patch -p1 <ctags_changes.patch')

yay

> @@ -120,7 +120,15 @@ static gboolean init_tag(TMTag *tag, TMSourceFile *file, const tagEntryInfo *tag
 	tag->name = g_strdup(tag_entry->name);
 	tag->type = type;
 	tag->local = tag_entry->isFileScope;
-	tag->pointerOrder = 0;	/* backward compatibility (use var_type instead) */
+	tag->flags = tm_tag_flag_none_t;
+	if (isTagExtraBitMarked(tag_entry, XTAG_ANONYMOUS))
+	{
+		tag->flags |= tm_tag_flag_anon_t;
+		/* Temporarily store kind letter for anon name generation to flags
+		 * as we don't have any other place where to store it. This will
+		 * be cleared later in rename_anon_tags() */
+		tag->flags |= (guint)kind_letter << 16;

Can't you just add a member to `TMTag` instead of this workaround?

> +						/* set the name of the original anon tag and pretend
+						 * it wasn't a anon tag */
+						tag->name = g_strdup(typedef_tag->name);
+						tag->flags &= ~tm_tag_flag_anon_t;
+						new_name = tag->name;
+						/* the typedef tag will be removed */
+						g_ptr_array_add(removed_typedefs, GUINT_TO_POINTER(j));
+					}
+				}
+			}
+
+			/* there's no typedef name for the anon tag so let's generate one  */
+			if (!new_name)
+			{
+				gchar buf[50];
+				guint anon_counter = GPOINTER_TO_UINT(g_hash_table_lookup(anon_counter_table, kind_name));

I think a hash_table is a overweight here. Since `kind` is in the range 0...255 a simple array would sufficie

guint anon_counter = anon_counter_table[kind]++;

Alternatively, you could use a hash table that uses `kind` as the key and not a more expensive hash of `kind_name`

> +				if (nested_scope_len <= scope_len)
+					break;
+
+				pos = strstr(nested_tag->scope, orig_name);
+				/* We found the parent name in the nested tag scope - replace it
+				 * with the new name. Note: anonymous tag names generated by
+				 * ctags are unique enough that we don't have to check for
+				 * scope separators here. */
+				if (pos)
+				{
+					gchar *str = g_malloc(nested_scope_len + 50);
+					guint prefix_len = pos - nested_tag->scope;
+
+					strncpy(str, nested_tag->scope, prefix_len);
+					strcpy(str + prefix_len, new_name);
+					strcpy(str + prefix_len + new_name_len, pos + orig_name_len);

Such string building could use `asprintf()` (not requesting a change here; also I see that GLIB only provides `g_vasprintf()` but not `g_asprintf()`, very strange)

> @@ -11,6 +9,8 @@ Named5
 Named6�2�Constants�0
 Named7�2�Constants�0
 a�4�Enum#1�0
+anon_enum_1�2�Constants�1
+anon_enum_2�2�Constants�1

I wonder if the new naming is really appreciated by Fortran users? I guess we'll see ;-)

> +{
+	guint i;
+	char dummy;
+
+	if (lang == TM_PARSER_C || lang == TM_PARSER_CPP)
+	{
+		return sscanf(name, "anon_%*[a-z]_%u%c", &i, &dummy) == 1 ||
+			sscanf(name, "__anon%u%c", &i, &dummy) == 1;
+	}
+	else if (lang == TM_PARSER_FORTRAN || lang == TM_PARSER_F77)
+	{
+		return sscanf(name, "Structure#%u%c", &i, &dummy) == 1 ||
+			sscanf(name, "Interface#%u%c", &i, &dummy) == 1 ||
+			sscanf(name, "Enum#%u%c", &i, &dummy) == 1;
+	}
+	return FALSE;

The unit test files for java script indicate that the naming is changed. Why is that not handled here (I know it also wasn't in the old `tm_tag_is_anon()` but I would like to understand why)?

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

Message ID: <geany/geany/pull/3059/review/836269780 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20211220/8da2aa52/attachment-0001.htm>