Re: [PATCH] Per-path attribute based hunk header selection.
From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:19
Possibly related (same subject, not in this thread)
- 2016-06-15 · Re: [PATCH] Per-path attribute based hunk header selection. · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH] Per-path attribute based hunk header selection. · しらいしななこ <hidden>
- 2016-06-15 · Re: [PATCH] Per-path attribute based hunk header selection. · Junio C Hamano <hidden>
- 2016-06-15 · Re: [PATCH] Per-path attribute based hunk header selection. · Johannes Schindelin <hidden>
- 2016-06-15 · Re: [PATCH] Per-path attribute based hunk header selection. · Junio C Hamano <hidden>
Hi, On Fri, 6 Jul 2007, Junio C Hamano wrote:
quoted hunk ↗ jump to hunk
diff --git a/diff.c b/diff.c@@ -1143,6 +1154,82 @@ int diff_filespec_is_binary(struct diff_filespec *one) return one->is_binary; } +static struct hunk_header_regexp { + char *name; + char *regexp; + struct hunk_header_regexp *next; +} *hunk_header_regexp_list, **hunk_header_regexp_tail; + +static int hunk_header_config(const char *var, const char *value) +{ + static const char funcname[] = "funcname."; + struct hunk_header_regexp *hh; + + if (prefixcmp(var, funcname)) + return 0; + var += strlen(funcname); + for (hh = hunk_header_regexp_list; hh; hh = hh->next) + if (!strcmp(var, hh->name)) { + free(hh->regexp); + hh->regexp = xstrdup(value); + return 0; + }
Heh. By reordering your code, you could say
if ((hh = hunk_header_regexp(var))) {
free(hh->regexp);
hh->regexp = xstrdup(value);
return 0;
}
+ hh = xcalloc(1, sizeof(*hh)); + hh->name = xstrdup(var); + hh->regexp = xstrdup(value); + hh->next = NULL; + *hunk_header_regexp_tail = hh; + return 0; +}
Is that tail expansion not overly complex? Why not just set "hh->next = hunk_header_regexp_list; hunk_header_regexp_list = hh"; Yes, your code seems correct, but I took some extra cycles to get at that impression. A "static int parsed_config_for_hunk_headers" would have helped, instead of reusing _tail for two purposes. And this variable could be set at the beginning of hunk_header_config(), so that hunk_header_regexp() is usable from inside hunk_header_config().
+static const char *hunk_header_regexp(const char *ident)
+{
+ struct hunk_header_regexp *hh;
+
+ if (!hunk_header_regexp_tail) {
+ hunk_header_regexp_tail = &hunk_header_regexp_list;
+ git_config(hunk_header_config);
+ }
+ for (hh = hunk_header_regexp_list; hh; hh = hh->next)
+ if (!strcmp(ident, hh->name))
+ return hh->regexp;
+ return NULL;
+}Another thing. These long names are a bit inconsistent. In the config, you name it "funcname". In xdiff, we name them "FUNCNAMES". Yes, here they are hunk_headers. Also, since the expressions are not strictly regular expressions, but lists of them, and with your idea they are even more different, why not just go for "funcname_list"? It's easier to read, and static anyway. Rest looks fine to me... Ciao, Dscho