@b4n requested changes on this pull request.

First pass only based on code review, no actual testing yet.


In src/tagmanager/tm_source_file.c:

> +				TMParserType lang = tm_ctags_get_named_lang(value);
+				if (lang >= 0)
+					tag->lang = lang;
../../../src/tagmanager/tm_source_file.c: In function 'read_ctags_file':
../../../src/tagmanager/tm_source_file.c:394:46: warning: declaration of 'lang' shadows a parameter [-Wshadow]
  394 |                                 TMParserType lang = tm_ctags_get_named_lang(value);
      |                                              ^~~~
../../../src/tagmanager/tm_source_file.c:309:66: note: shadowed declaration is here
  309 | static void read_ctags_file(const gchar *tags_file, TMParserType lang, GPtrArray *file_tags)
      |                                                     ~~~~~~~~~~~~~^~~~

Maybe rename the local tag_lang?

⬇️ Suggested change
-				TMParserType lang = tm_ctags_get_named_lang(value);
-				if (lang >= 0)
-					tag->lang = lang;
+				TMParserType tag_lang = tm_ctags_get_named_lang(value);
+				if (tag_lang >= 0)
+					tag->lang = tag_lang;

In src/tagmanager/tm_source_file.c:

>  	{
-		if ((NULL == fgets(buf, BUFSIZ, fp)) || ('\0' == *buf))
-			return FALSE;
-	}
-	while (strncmp(buf, "!_TAG_", 6) == 0); /* skip !_TAG_ lines */
+		TMTagType type;
+		TMTag *tag;
+		guint i, idx;
../../../src/tagmanager/tm_source_file.c:409:39: warning: declaration of 'i' shadows a previous local [-Wshadow]
  409 |                                 guint i = g_array_index(unknown_fields, guint, idx);
      |                                       ^
../../../src/tagmanager/tm_source_file.c:320:23: note: shadowed declaration is here
  320 |                 guint i, idx;
      |                       ^

Easy solution is to declare those in the for initialization, as we now accept this syntax.

⬇️ Suggested change
-		guint i, idx;

In src/tagmanager/tm_source_file.c:

> -	{
-		p = tab + 2;
-		while (*p && *p != '\n' && *p != '\r')
+		tag = tm_tag_new();
+		tag->refcount = 1;
+		tag->name = g_strdup(entry.name);
+		tag->type = type;
+		tag->lang = lang;
+		tag->local = entry.fileScope;  /* 'f' field */
+		tag->line = entry.address.lineNumber;  /* 'n' field */
+		tag->file = NULL;
+
+		if (tm_parser_is_anon_name(lang, tag->name))
+			tag->flags |= tm_tag_flag_anon_t;
+
+		for (i = 0; i < entry.fields.count; i++)
⬇️ Suggested change
-		for (i = 0; i < entry.fields.count; i++)
+		for (guint i = 0; i < entry.fields.count; i++)

In src/tagmanager/tm_source_file.c:

>  			}
-			else if (0 == strcmp(key, "file")) /* static (local) tag */
-				tag->local = TRUE;
-			else if (0 == strcmp(key, "signature")) /* arglist */
+			else
+				g_array_append_val(unknown_fields, i);
+		}
+
+		if (!tag->scope)
+		{
+			/* search for scope introduced by scope kind name only after going
+			 * through all extension fields and having tag->lang updated when
+			 * "language" field is present */
+			for (idx = 0; !tag->scope && idx < unknown_fields->len; idx++)
⬇️ Suggested change
-			for (idx = 0; !tag->scope && idx < unknown_fields->len; idx++)
+			for (guint idx = 0; !tag->scope && idx < unknown_fields->len; idx++)

Or make this i, and make the local idx, which might be more idiomatic.


In src/tagmanager/tm_source_file.c:

> +				int j;
+				for (j = 0; lang_kinds[j]; j++)

…and while at it

⬇️ Suggested change
-				int j;
-				for (j = 0; lang_kinds[j]; j++)
+				for (gint j = 0; lang_kinds[j]; j++)

In src/tagmanager/tm_source_file.c:

>  			{
-				g_free(tag->arglist);
-				tag->arglist = g_strdup(value);
+				guint i = g_array_index(unknown_fields, guint, idx);
+				const gchar *key = entry.fields.list[i].key;
+				const gchar *value = entry.fields.list[i].value;
+				int j;
+				for (j = 0; lang_kinds[j]; j++)
+				{
+					gchar kind = lang_kinds[j];
⬇️ Suggested change
-					gchar kind = lang_kinds[j];
+					const gchar kind = lang_kinds[j];

In src/tagmanager/tm_source_file.c:

>  			{
-				g_free(tag->arglist);
-				tag->arglist = g_strdup(value);
+				guint i = g_array_index(unknown_fields, guint, idx);
⬇️ Suggested change
-				guint i = g_array_index(unknown_fields, guint, idx);
+				const guint i = g_array_index(unknown_fields, guint, idx);

In src/tagmanager/tm_source_file.c:

>  			{
-				if (*end == ':' && ! value)
+				/* scope:class:A::B::C */
+				gchar *val = strchr(value, ':');
⬇️ Suggested change
-				gchar *val = strchr(value, ':');
+				const gchar *val = strchr(value, ':');

In src/tagmanager/tm_source_file.c:

>  			{
 				g_free(tag->inheritance);
 				tag->inheritance = g_strdup(value);
 			}
-			else if (0 == strcmp(key, "implementation")) /* implementation limit */
+			else if (strcmp(key, "typeref") == 0)  /* 't' field */
+			{
+				/* typeref:typename:int */
+				gchar *val = strchr(value, ':');
⬇️ Suggested change
-				gchar *val = strchr(value, ':');
+				const gchar *val = strchr(value, ':');


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <geany/geany/pull/3049/review/1678868632@github.com>