Re: [PATCH v1 1/2] convert: refactor conversion driver config parsing

2018-07-10 Thread Junio C Hamano
larsxschnei...@gmail.com writes:

> From: Lars Schneider 
>
> Refactor conversion driver config parsing to ease the parsing of new
> configs in a subsequent patch.
>
> No functional change intended.
>
> Signed-off-by: Lars Schneider 
> ---

The change unfortunately makes everything indented one level deeper,
but I can see why "we look for the only thing we are interested in
and return early when given anything else" that allowed us to avoid
the deep indentation would get in the way of the second patch.


Re: [PATCH v1 1/2] convert: refactor conversion driver config parsing

2018-07-09 Thread Junio C Hamano
Lars Schneider  writes:

>> On Jul 8, 2018, at 8:30 PM, larsxschnei...@gmail.com wrote:
>> 
>> From: Lars Schneider 
>> 
>> Refactor conversion driver config parsing to ease the parsing of new
>> configs in a subsequent patch.
>> 
>> No functional change intended.
>> 
>> Signed-off-by: Lars Schneider 
>> ---
>> convert.c | 64 +++
>> 1 file changed, 32 insertions(+), 32 deletions(-)
>> 
>> diff --git a/convert.c b/convert.c
>> index 64d0d30e08..949bc783e4 100644
>> --- a/convert.c
>> +++ b/convert.c
>> @@ -1003,43 +1003,43 @@ static int read_convert_config(const char *var, 
>> const char *value, void *cb)
>>  int namelen;
>>  struct convert_driver *drv;
>> 
>> ...
>> 
>> -/*
>> - * filter..smudge and filter..clean specifies
>> - * the command line:
>> - *
>> - *  command-line
>> - *
>> - * The command-line will not be interpolated in any way.
>> - */
>> +/*
>> + * filter..smudge and filter..clean specifies
>> + * the command line:
>> + *
>> + *  command-line
>> + *
>> + * The command-line will not be interpolated in any way.
>> + */
>
> I stumbled over this comment introduced in aa4ed402c9 
> ("Add 'filter' attribute and external filter driver definition.", 2007-04-21).
>
> Is the middle "command-line" intentional?

I think it was a deliberate but ineffective attempt to emphasize the
fact that the command line is used as-is, and does not get split at
SP nor goes through interpolation of placeholders using API such as
strbuf_expand().


Re: [PATCH v1 1/2] convert: refactor conversion driver config parsing

2018-07-08 Thread Lars Schneider



> On Jul 8, 2018, at 8:30 PM, larsxschnei...@gmail.com wrote:
> 
> From: Lars Schneider 
> 
> Refactor conversion driver config parsing to ease the parsing of new
> configs in a subsequent patch.
> 
> No functional change intended.
> 
> Signed-off-by: Lars Schneider 
> ---
> convert.c | 64 +++
> 1 file changed, 32 insertions(+), 32 deletions(-)
> 
> diff --git a/convert.c b/convert.c
> index 64d0d30e08..949bc783e4 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -1003,43 +1003,43 @@ static int read_convert_config(const char *var, const 
> char *value, void *cb)
>   int namelen;
>   struct convert_driver *drv;
> 
> ...
> 
> - /*
> -  * filter..smudge and filter..clean specifies
> -  * the command line:
> -  *
> -  *  command-line
> -  *
> -  * The command-line will not be interpolated in any way.
> -  */
> + /*
> +  * filter..smudge and filter..clean specifies
> +  * the command line:
> +  *
> +  *  command-line
> +  *
> +  * The command-line will not be interpolated in any way.
> +  */

I stumbled over this comment introduced in aa4ed402c9 
("Add 'filter' attribute and external filter driver definition.", 2007-04-21).

Is the middle "command-line" intentional?

- Lars

[PATCH v1 1/2] convert: refactor conversion driver config parsing

2018-07-08 Thread larsxschneider
From: Lars Schneider 

Refactor conversion driver config parsing to ease the parsing of new
configs in a subsequent patch.

No functional change intended.

Signed-off-by: Lars Schneider 
---
 convert.c | 64 +++
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/convert.c b/convert.c
index 64d0d30e08..949bc783e4 100644
--- a/convert.c
+++ b/convert.c
@@ -1003,43 +1003,43 @@ static int read_convert_config(const char *var, const 
char *value, void *cb)
int namelen;
struct convert_driver *drv;
 
-   /*
-* External conversion drivers are configured using
-* "filter..variable".
-*/
-   if (parse_config_key(var, "filter", , , ) < 0 || !name)
-   return 0;
-   for (drv = user_convert; drv; drv = drv->next)
-   if (!strncmp(drv->name, name, namelen) && !drv->name[namelen])
-   break;
-   if (!drv) {
-   drv = xcalloc(1, sizeof(struct convert_driver));
-   drv->name = xmemdupz(name, namelen);
-   *user_convert_tail = drv;
-   user_convert_tail = &(drv->next);
-   }
+   if (parse_config_key(var, "filter", , , ) >= 0 && 
name) {
+   /*
+* External conversion drivers are configured using
+* "filter..variable".
+*/
+   for (drv = user_convert; drv; drv = drv->next)
+   if (!strncmp(drv->name, name, namelen) && 
!drv->name[namelen])
+   break;
+   if (!drv) {
+   drv = xcalloc(1, sizeof(struct convert_driver));
+   drv->name = xmemdupz(name, namelen);
+   *user_convert_tail = drv;
+   user_convert_tail = &(drv->next);
+   }
 
-   /*
-* filter..smudge and filter..clean specifies
-* the command line:
-*
-*  command-line
-*
-* The command-line will not be interpolated in any way.
-*/
+   /*
+* filter..smudge and filter..clean specifies
+* the command line:
+*
+*  command-line
+*
+* The command-line will not be interpolated in any way.
+*/
 
-   if (!strcmp("smudge", key))
-   return git_config_string(>smudge, var, value);
+   if (!strcmp("smudge", key))
+   return git_config_string(>smudge, var, value);
 
-   if (!strcmp("clean", key))
-   return git_config_string(>clean, var, value);
+   if (!strcmp("clean", key))
+   return git_config_string(>clean, var, value);
 
-   if (!strcmp("process", key))
-   return git_config_string(>process, var, value);
+   if (!strcmp("process", key))
+   return git_config_string(>process, var, value);
 
-   if (!strcmp("required", key)) {
-   drv->required = git_config_bool(var, value);
-   return 0;
+   if (!strcmp("required", key)) {
+   drv->required = git_config_bool(var, value);
+   return 0;
+   }
}
 
return 0;
-- 
2.18.0