Re: [PATCH v7 6/6] convert: add "status=delayed" to filter process protocol

[Lars Schneider <larsxschneider@xxxxxxxxx>]

> On 27 Jun 2017, at 22:34, Lars Schneider <larsxschneider@xxxxxxxxx> wrote:
> 
> 
>> On 27 Jun 2017, at 21:00, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> 
>> Lars Schneider <larsxschneider@xxxxxxxxx> writes:
>> 
>>> @@ -533,7 +534,8 @@ static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
>>> 	if (err)
>>> 		goto done;
>>> 
>>> -	err = packet_writel(process->in, "capability=clean", "capability=smudge", NULL);
>>> +	err = packet_writel(process->in,
>>> +		"capability=clean", "capability=smudge", "capability=delay", NULL);
>>> 
>>> 	for (;;) {
>>> 		cap_buf = packet_read_line(process->out, NULL);
>>> @@ -549,6 +551,8 @@ static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
>>> 			entry->supported_capabilities |= CAP_CLEAN;
>>> 		} else if (!strcmp(cap_name, "smudge")) {
>>> 			entry->supported_capabilities |= CAP_SMUDGE;
>>> +		} else if (!strcmp(cap_name, "delay")) {
>>> +			entry->supported_capabilities |= CAP_DELAY;
>>> 		} else {
>>> 			warning(
>>> 				"external filter '%s' requested unsupported filter capability '%s'",
>> 
>> I thought you said something about attempting to make this more
>> table-driven; did the attempt produce a better result?  Just being
>> curious.
> 
> I treated that as low-prio as I got the impression that you are generally OK with
> the current implementation. I promise to send a patch with this change. 
> Either as part of this series or as a follow up patch.

How about this?


 static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
 {
-       int err;
+       int err, i;
        struct cmd2process *entry = (struct cmd2process *)subprocess;
        struct string_list cap_list = STRING_LIST_INIT_NODUP;
        char *cap_buf;
@@ -516,6 +516,15 @@ static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
        struct child_process *process = &subprocess->process;
        const char *cmd = subprocess->cmd;

+       static const struct {
+               const char *name;
+               unsigned int cap;
+       } known_caps[] = {
+               { "clean",  CAP_CLEAN  },
+               { "smudge", CAP_SMUDGE },
+               { "delay",  CAP_DELAY  },
+       };
+
        sigchain_push(SIGPIPE, SIG_IGN);

        err = packet_writel(process->in, "git-filter-client", "version=2", NULL);
@@ -534,8 +543,15 @@ static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
        if (err)
                goto done;

-       err = packet_writel(process->in,
-               "capability=clean", "capability=smudge", "capability=delay", NULL);
+       for (i = 0; i < ARRAY_SIZE(known_caps); ++i) {
+               err = packet_write_fmt_gently(
+                       process->in, "capability=%s\n", known_caps[i].name);
+               if (err)
+                       goto done;
+       }
+       err = packet_flush_gently(process->in);
+       if (err)
+               goto done;

        for (;;) {
                cap_buf = packet_read_line(process->out, NULL);
@@ -547,18 +563,15 @@ static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
                        continue;

                cap_name = cap_list.items[1].string;
-               if (!strcmp(cap_name, "clean")) {
-                       entry->supported_capabilities |= CAP_CLEAN;
-               } else if (!strcmp(cap_name, "smudge")) {
-                       entry->supported_capabilities |= CAP_SMUDGE;
-               } else if (!strcmp(cap_name, "delay")) {
-                       entry->supported_capabilities |= CAP_DELAY;
-               } else {
-                       warning(
-                               "external filter '%s' requested unsupported filter capability '%s'",
-                               cmd, cap_name
-                       );
-               }
+               i = ARRAY_SIZE(known_caps) - 1;
+               while (i >= 0 && strcmp(cap_name, known_caps[i].name))
+                       i--;
+
+               if (i >= 0)
+                       entry->supported_capabilities |= known_caps[i].cap;
+               else
+                       warning("external filter '%s' requested unsupported filter capability '%s'",
+                       cmd, cap_name);

                string_list_clear(&cap_list, 0);