Re: [PATCH v1 2/2] convert: add alias support for 'working-tree-encoding' attributes

[Junio C Hamano]

larsxschnei...@gmail.com writes:

> From: Lars Schneider 
>
> In 107642fe26 ("convert: add 'working-tree-encoding' attribute",
> 2018-04-15) we added an attribute which defines the working tree
> encoding of a file.
>
> Some platforms might spell the name of a certain encoding differently or
> some users might want to use different encodings on different platforms.
> Add the Git config "encoding..insteadOf = " to
> support these use-cases with a user specific mapping. If the alias
> matches an existing encoding name, then the alias will take precedence.
> The alias is case insensitive.
>
> Example:
>
>   (in .gitattributes)
>   *.c working-tree-encoding=foo
>
>   (in config)
>   [encoding "UTF-16"]
>   insteadOf = foo
>
> Signed-off-by: Lars Schneider 
> ---

Hmph, when I was reading the discussion between you and Peff, I
didn't expect it end up with a change too specific to this single
attribute.  I was instead imagining that a change would come closer
to where we call iconv (perhaps we muck with in/out_encoding
parameters to iconv_open()), so that places other than the
working-tree-encoding (e.g. commit log message encoding) would start
honoring the same configuration variable in a consistent way.

Re: [PATCH v1 2/2] convert: add alias support for 'working-tree-encoding' attributes

[Lars Schneider]

> On Jul 8, 2018, at 8:30 PM, larsxschnei...@gmail.com wrote:
> 
> From: Lars Schneider 
> 
> In 107642fe26 ("convert: add 'working-tree-encoding' attribute",
> 2018-04-15) we added an attribute which defines the working tree
> encoding of a file.
> 
> Some platforms might spell the name of a certain encoding differently or
> some users might want to use different encodings on different platforms.
> Add the Git config "encoding..insteadOf = " to
> support these use-cases with a user specific mapping. If the alias
> matches an existing encoding name, then the alias will take precedence.
> The alias is case insensitive.
> 
> Example:
> 
>   (in .gitattributes)
>   *.c working-tree-encoding=foo
> 
>   (in config)
>   [encoding "UTF-16"]
>   insteadOf = foo
> 
> Signed-off-by: Lars Schneider 
> ---
> Documentation/gitattributes.txt  | 19 
> convert.c| 50 
> t/t0028-working-tree-encoding.sh | 28 ++
> 3 files changed, 97 insertions(+)
> 
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 92010b062e..3628f0e5cf 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -366,6 +366,25 @@ command to guess the encoding:
> file foo.ps1
> 
> 
> ...
> 
>   return 0;
> @@ -1225,6 +1264,17 @@ static const char *git_path_check_encoding(struct 
> attr_check_item *check)
>   die(_("true/false are no valid working-tree-encodings"));
>   }
> 
> + /* Check if an alias was defined for the encoding in the Git config */
> + if (encoding_aliases_initialized) {
> + struct alias2enc hashkey;
> + struct alias2enc *entry;
> + hashmap_entry_init(, strihash(value));
> + hashkey.alias = value;
> + entry = hashmap_get(_map, , NULL);
> + if (entry)
> + value = entry->encoding;

Here I reuse the char* pointer from the hashmap.
The hashmap is static and no entry is ever removed.
Is this OK or should I rather create a copy of the string?

Thanks,
Lars