Re: [PATCH v4 5/6] convert: add 'working-tree-encoding' attribute

On Mon, Jan 22, 2018 at 01:35:25PM +0100, Lars Schneider wrote:

quoted

quoted

+	enc->name = xstrdup_toupper(value);  /* aways use upper case names! */

"aways" -> "always" and I think the comment should say why
uppercase is important.

Would that be better?

	/* Aways use upper case names to simplify subsequent string comparison. */
	enc->name = xstrdup_toupper(value);

AFAIK uppercase and lowercase names are both valid. I just wanted to
ensure that we use one consistent casing. That reads better in error messages
and I don't need to check for the letter case in has_prohibited_utf_bom()
and friends in utf8.c

Sounds good (minus the "Aways" typo Eric already noticed).

quoted

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)?

I would rather add a new line and a comment to the previous test
to be consistent.

I know we could leave the files but these lingering files could
always surprise writers of future tests (at least they surprised
me in other tests).

Sure, that sounds good. Just noticed the inconsistency and wanted
to mention it.

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9