Re: [PATCH v4 5/6] convert: add 'working-tree-encoding' attribute
- From
Simon Ruderich <simon@ruderich.org>
- Date
- Jan 21, 2018, 14:22 UTC
- Message-ID
- <20180121142222.GA10248@ruderich.org>
- In-Reply-To
- <20180120152418.52859-6-lars.schneider@autodesk.com>
On Sat, Jan 20, 2018 at 04:24:17PM +0100, lars.schneider@autodesk.com wrote:
Show 18 quoted lines
> +static struct encoding *git_path_check_encoding(struct attr_check_item *check)
> +{
> + const char *value = check->value;
> + struct encoding *enc;
> +
> + if (ATTR_TRUE(value) || ATTR_FALSE(value) || ATTR_UNSET(value) ||
> + !strlen(value))
> + return NULL;
> +
> + for (enc = encoding; enc; enc = enc->next)
> + if (!strcasecmp(value, enc->name))
> + return enc;
> +
> + /* Don't encode to the default encoding */
> + if (!strcasecmp(value, default_encoding))
> + return NULL;
> +
> + enc = xcalloc(1, sizeof(struct convert_driver));I think this should be "sizeof(struct encoding)" but I prefer "sizeof(*enc)" which prevents these kind of mistakes.
> + enc->name = xstrdup_toupper(value); /* aways use upper case names! */
"aways" -> "always" and I think the comment should say why uppercase is important.
Show 13 quoted lines
> +test_expect_success 'ensure UTF-8 is stored in Git' ' > + git cat-file -p :test.utf16 >test.utf16.git && > + test_cmp_bin test.utf8.raw test.utf16.git && > + rm test.utf8.raw test.utf16.git > +' > + > +test_expect_success 're-encode to UTF-16 on checkout' ' > + rm test.utf16 && > + git checkout test.utf16 && > + test_cmp_bin test.utf16.raw test.utf16 && > + > + # cleanup > + rm test.utf16.raw
Micro-nit: For consistency with the previous test, remove the empty line and comment (or just keep the files generated from the "setup test repo" phase and don't explicitly delete them)?
Regards Simon
-- + privacy is necessary + using gnupg http://gnupg.org + public key id: 0x92FEFDB7E44C32F9