[geany/geany] 652233: Fix the symbols tree hierarchy when several tags have the same name

Colomban Wendling git-noreply at xxxxx
Thu Dec 21 00:40:49 UTC 2017


Branch:      refs/heads/master
Author:      Colomban Wendling <ban at herbesfolles.org>
Committer:   Colomban Wendling <ban at herbesfolles.org>
Date:        Sat, 02 Sep 2017 00:31:56 UTC
Commit:      6522332ba9d54da7d750b1f2d719b07af6e861cf
             https://github.com/geany/geany/commit/6522332ba9d54da7d750b1f2d719b07af6e861cf

Log Message:
-----------
Fix the symbols tree hierarchy when several tags have the same name

Fix the symbols tree hierarchy by considering the whole scope when
adding a tag, avoiding choosing the wrong parent when several tags have
the same name.  Until now, to avoid such misbehavior we only used to
choose the parent candidate that appeared last (line-wise) before the
child.  It works in most typical situations as generally tag names are
fairly unique, and children appear right after their parent.

However, there are cases that are trickier and cannot be handled that
way.  In the following valid C++ snippet, it is impossible to know
whether `function` should be listed under the namespace `A` or the
class `A` without looking at its full scope:

```C++
namespace A {
    namespace B {
        class A {
            void method() {}
        };
    };
    void function() {}
};
```

And it is a real-world problem for some parsers like the JSON parser
that generates numeric indices for array elements name, often leading
to several possibly close duplicates.

Additionally, to prevent trying to set a tag as its own parent, the
code guarded against accepting a parent if the child had the same name,
lading to incorrect hierarchy for `method` in cases like this:

```C++
namespace A {
    class A {
        void method() {}
    };
};
```

So to fix this, consider the whole hierarchy of a tag for choosing its
parent, when that information is available from the parser.

Fixes #1583.


Modified Paths:
--------------
    HACKING
    src/symbols.c
    src/tagmanager/tm_tag.c
    src/tagmanager/tm_tag.h

Modified: HACKING
6 lines changed, 5 insertions(+), 1 deletions(-)
===================================================================
@@ -667,12 +667,16 @@ Method
 * Add TM_PARSER_FOO to src/tagmanager/tm_parser.h.  The list here must follow
   exactly the order in parsers.h.
 
-In tagmanager/src/tm_parsers.c:
+In src/tagmanager/tm_parser.c:
 Add a map_FOO TMParserMapEntry mapping each kind's letter from foo.c's
 FooKinds to the appropriate TMTagType, and add the corresponding
 MAP_ENTRY(FOO) to parser_map.
 (You may want to make the symbols.c change before doing this).
 
+In src/tagmanager/tm_tag.c:
+Update tm_tag_context_separator() and tm_tag_has_full_context() to handle the
+new parser if applicable, by adding a TM_PARSER_FOO case entry.
+
 In filetypes.c, init_builtin_filetypes():
 Set the 2nd argument of the FT_INIT() macro for this filetype to FOO.
 


Modified: src/symbols.c
78 lines changed, 41 insertions(+), 37 deletions(-)
===================================================================
@@ -922,30 +922,9 @@ static gchar *get_symbol_tooltip(GeanyDocument *doc, const TMTag *tag)
 }
 
 
-/* find the last word in "foo::bar::blah", e.g. "blah" */
-static const gchar *get_parent_name(const TMTag *tag, GeanyFiletypeID ft_id)
+static const gchar *get_parent_name(const TMTag *tag)
 {
-	const gchar *scope = tag->scope;
-	const gchar *separator = symbols_get_context_separator(ft_id);
-	const gchar *str, *ptr;
-
-	if (!scope)
-		return NULL;
-
-	str = scope;
-
-	while (1)
-	{
-		ptr = strstr(str, separator);
-		if (ptr)
-		{
-			str = ptr + strlen(separator);
-		}
-		else
-			break;
-	}
-
-	return !EMPTY(str) ? str : NULL;
+	return !EMPTY(tag->scope) ? tag->scope : NULL;
 }
 
 
@@ -1147,21 +1126,46 @@ static void parents_table_tree_value_free(gpointer data)
 
 
 /* adds a new element in the parent table if its key is known. */
-static void update_parents_table(GHashTable *table, const TMTag *tag, const gchar *parent_name,
-		const GtkTreeIter *iter)
+static void update_parents_table(GHashTable *table, const TMTag *tag, const GtkTreeIter *iter)
 {
+	const gchar *name;
+	gchar *name_free = NULL;
 	GTree *tree;
-	if (g_hash_table_lookup_extended(table, tag->name, NULL, (gpointer *) &tree) &&
-		! utils_str_equal(parent_name, tag->name) /* prevent Foo::Foo from making parent = child */)
+
+	if (EMPTY(tag->scope))
+	{
+		/* simple case, just use the tag name */
+		name = tag->name;
+	}
+	else if (! tm_tag_has_full_context(tag->lang))
+	{
+		/* if the parser doesn't use fully qualified scope, use the name alone but
+		 * prevent Foo::Foo from making parent = child */
+		if (utils_str_equal(tag->scope, tag->name))
+			name = NULL;
+		else
+			name = tag->name;
+	}
+	else
+	{
+		/* build the fully qualified scope as get_parent_name() would return it for a child tag */
+		name_free = g_strconcat(tag->scope, tm_tag_context_separator(tag->lang), tag->name, NULL);
+		name = name_free;
+	}
+
+	if (name && g_hash_table_lookup_extended(table, name, NULL, (gpointer *) &tree))
 	{
 		if (!tree)
 		{
 			tree = g_tree_new_full(tree_cmp, NULL, NULL, parents_table_tree_value_free);
-			g_hash_table_insert(table, tag->name, tree);
+			g_hash_table_insert(table, name_free ? name_free : g_strdup(name), tree);
+			name_free = NULL;
 		}
 
 		g_tree_insert(tree, GINT_TO_POINTER(tag->line), g_slice_dup(GtkTreeIter, iter));
 	}
+
+	g_free(name_free);
 }
 
 
@@ -1314,20 +1318,20 @@ static void update_tree_tags(GeanyDocument *doc, GList **tags)
 
 	/* Build hash tables holding tags and parents */
 	/* parent table is GHashTable<tag_name, GTree<line_num, GtkTreeIter>> */
-	parents_table = g_hash_table_new_full(g_str_hash, g_str_equal, NULL, parents_table_value_free);
+	parents_table = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, parents_table_value_free);
 	/* tags table is another representation of the @tags list,
 	 * GHashTable<TMTag, GTree<line_num, GList<GList<TMTag>>>> */
 	tags_table = g_hash_table_new_full(tag_hash, tag_equal, NULL, tags_table_value_free);
 	foreach_list(item, *tags)
 	{
 		TMTag *tag = item->data;
-		const gchar *name;
+		const gchar *parent_name;
 
 		tags_table_insert(tags_table, tag, item);
 
-		name = get_parent_name(tag, doc->file_type->id);
-		if (name)
-			g_hash_table_insert(parents_table, (gpointer) name, NULL);
+		parent_name = get_parent_name(tag);
+		if (parent_name)
+			g_hash_table_insert(parents_table, g_strdup(parent_name), NULL);
 	}
 
 	/* First pass, update existing rows or delete them.
@@ -1354,7 +1358,7 @@ static void update_tree_tags(GeanyDocument *doc, GList **tags)
 				const gchar *parent_name;
 				TMTag *found = found_item->data;
 
-				parent_name = get_parent_name(found, doc->file_type->id);
+				parent_name = get_parent_name(found);
 				/* if parent is unknown, ignore it */
 				if (parent_name && ! g_hash_table_lookup(parents_table, parent_name))
 					parent_name = NULL;
@@ -1376,7 +1380,7 @@ static void update_tree_tags(GeanyDocument *doc, GList **tags)
 					g_free(tooltip);
 				}
 
-				update_parents_table(parents_table, found, parent_name, &iter);
+				update_parents_table(parents_table, found, &iter);
 
 				/* remove the updated tag from the table and list */
 				tags_table_remove(tags_table, found);
@@ -1407,7 +1411,7 @@ static void update_tree_tags(GeanyDocument *doc, GList **tags)
 			gchar *tooltip;
 			GdkPixbuf *icon = get_child_icon(store, parent);
 
-			parent_name = get_parent_name(tag, doc->file_type->id);
+			parent_name = get_parent_name(tag);
 			if (parent_name)
 			{
 				GtkTreeIter *parent_search = parents_table_lookup(parents_table, parent_name, tag->line);
@@ -1435,7 +1439,7 @@ static void update_tree_tags(GeanyDocument *doc, GList **tags)
 			if (G_LIKELY(icon))
 				g_object_unref(icon);
 
-			update_parents_table(parents_table, tag, parent_name, &iter);
+			update_parents_table(parents_table, tag, &iter);
 
 			if (expand)
 				tree_view_expand_to_iter(GTK_TREE_VIEW(doc->priv->tag_tree), &iter);


Modified: src/tagmanager/tm_tag.c
41 lines changed, 41 insertions(+), 0 deletions(-)
===================================================================
@@ -693,6 +693,47 @@ const gchar *tm_tag_context_separator(TMParserType lang)
 	}
 }
 
+gboolean tm_tag_has_full_context(TMParserType lang)
+{
+	switch (lang)
+	{
+		/* These parsers include full hierarchy in the tag scope, separated by tm_tag_context_separator() */
+		case TM_PARSER_C:
+		case TM_PARSER_CPP:
+		case TM_PARSER_CSHARP:
+		case TM_PARSER_D:
+		case TM_PARSER_FERITE:
+		case TM_PARSER_GLSL:
+		case TM_PARSER_JAVA:
+		case TM_PARSER_JAVASCRIPT:
+		case TM_PARSER_JSON:
+		case TM_PARSER_PHP:
+		case TM_PARSER_POWERSHELL:
+		case TM_PARSER_PYTHON:
+		case TM_PARSER_RUBY:
+		case TM_PARSER_RUST:
+		case TM_PARSER_SQL:
+		case TM_PARSER_TXT2TAGS:
+		case TM_PARSER_VALA:
+		case TM_PARSER_ZEPHIR:
+			return TRUE;
+
+		/* These make use of the scope, but don't include nested hierarchy
+		 * (either as a parser limitation or a language semantic) */
+		case TM_PARSER_ASCIIDOC:
+		case TM_PARSER_CONF:
+		case TM_PARSER_ERLANG:
+		case TM_PARSER_F77:
+		case TM_PARSER_FORTRAN:
+		case TM_PARSER_GO:
+		case TM_PARSER_OBJC:
+		case TM_PARSER_REST:
+		/* Other parsers don't use scope at all (or should be somewhere above) */
+		default:
+			return FALSE;
+	}
+}
+
 gboolean tm_tag_is_anon(const TMTag *tag)
 {
 	guint i;


Modified: src/tagmanager/tm_tag.h
2 lines changed, 2 insertions(+), 0 deletions(-)
===================================================================
@@ -141,6 +141,8 @@ gboolean tm_tags_equal(const TMTag *a, const TMTag *b);
 
 const gchar *tm_tag_context_separator(TMParserType lang);
 
+gboolean tm_tag_has_full_context(TMParserType lang);
+
 gboolean tm_tag_is_anon(const TMTag *tag);
 
 gboolean tm_tag_langs_compatible(TMParserType lang, TMParserType other);



--------------
This E-Mail was brought to you by github_commit_mail.py (Source: https://github.com/geany/infrastructure).


More information about the Commits mailing list