@b4n requested changes on this pull request.
First pass only based on *code* review, no actual testing yet.
> + TMParserType lang =
> tm_ctags_get_named_lang(value);
+ if (lang >= 0)
+ tag->lang = lang;
```gcc
../../../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`?
```suggestion
TMParserType tag_lang =
tm_ctags_get_named_lang(value);
if (tag_lang >= 0)
tag->lang = tag_lang;
```
> {
- 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;
```gcc
../../../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.
```suggestion
```
> - {
- 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++)
```suggestion
for (guint i = 0; i < entry.fields.count; i++)
```
> }
- 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++)
```suggestion
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.
> + int j;
+ for (j = 0; lang_kinds[j]; j++)
…and while at it
```suggestion
for (gint j = 0; lang_kinds[j]; j++)
```
> {
- 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];
```suggestion
const gchar kind = lang_kinds[j];
```
> {
- g_free(tag->arglist);
- tag->arglist = g_strdup(value);
+ guint i = g_array_index(unknown_fields, guint,
idx);
```suggestion
const guint i = g_array_index(unknown_fields,
guint, idx);
```
> {
- if (*end == ':' && ! value)
+ /* scope:class:A::B::C */
+ gchar *val = strchr(value, ':');
```suggestion
const gchar *val = strchr(value, ':');
```
> {
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, ':');
```suggestion
const gchar *val = strchr(value, ':');
```
--
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany/pull/3049#pullrequestreview-1678868632
You are receiving this because you are subscribed to this thread.
Message ID: <geany/geany/pull/3049/review/[email protected]>
