From: Simon Ruderich <simon@ruderich.org>
To: Lars Schneider <larsxschneider@gmail.com>
Cc: lars.schneider@autodesk.com, git@vger.kernel.org,
gitster@pobox.com, tboegi@web.de, j6t@kdbg.org,
sunshine@sunshineco.com, peff@peff.net,
ramsay@ramsayjones.plus.com, Johannes.Schindelin@gmx.de
Subject: Re: [PATCH v4 5/6] convert: add 'working-tree-encoding' attribute
Date: Tue, 23 Jan 2018 10:27:49 +0100 [thread overview]
Message-ID: <20180123092749.GA6308@ruderich.org> (raw)
In-Reply-To: <05265803-BD74-4667-ABB5-9752E55A5015@gmail.com>
On Mon, Jan 22, 2018 at 01:35:25PM +0100, Lars Schneider wrote:
>>> + 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).
>> 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
next prev parent reply other threads:[~2018-01-23 9:27 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-20 15:24 [PATCH v4 0/6] convert: add support for different encodings lars.schneider
2018-01-20 15:24 ` [PATCH v4 1/6] strbuf: remove unnecessary NUL assignment in xstrdup_tolower() lars.schneider
2018-01-20 15:24 ` [PATCH v4 2/6] strbuf: add xstrdup_toupper() lars.schneider
2018-01-20 15:24 ` [PATCH v4 3/6] utf8: add function to detect prohibited UTF-16/32 BOM lars.schneider
2018-01-20 15:24 ` [PATCH v4 4/6] utf8: add function to detect a missing " lars.schneider
2018-01-20 15:24 ` [PATCH v4 5/6] convert: add 'working-tree-encoding' attribute lars.schneider
2018-01-21 14:22 ` Simon Ruderich
2018-01-22 12:35 ` Lars Schneider
2018-01-23 0:54 ` Jeff King
2018-01-23 10:25 ` Simon Ruderich
2018-01-23 16:20 ` Jeff King
2018-01-23 21:12 ` Junio C Hamano
2018-01-23 9:27 ` Simon Ruderich [this message]
2018-01-22 18:00 ` SQUASH convert: add tracing for " lars.schneider
2018-01-22 19:37 ` Eric Sunshine
2018-01-23 10:17 ` [PATCH v2] " lars.schneider
2018-01-20 15:24 ` [PATCH v4 6/6] " lars.schneider
2018-01-23 20:19 ` [PATCH v4 0/6] convert: add support for different encodings Torsten Bögershausen
2018-01-23 20:53 ` Junio C Hamano
2018-01-29 20:18 ` [PATCH v5 0/7] " tboegi
2018-01-29 20:18 ` [PATCH v5 1/7] strbuf: remove unnecessary NUL assignment in xstrdup_tolower() tboegi
2018-01-29 20:19 ` [PATCH v5 2/7] strbuf: add xstrdup_toupper() tboegi
2018-01-29 20:19 ` [PATCH v5 3/7] utf8: add function to detect prohibited UTF-16/32 BOM tboegi
2018-01-29 20:19 ` [PATCH v5 4/7] utf8: add function to detect a missing " tboegi
2018-01-30 19:15 ` Junio C Hamano
2018-01-30 20:58 ` Lars Schneider
2018-01-30 21:54 ` Junio C Hamano
2018-01-29 20:19 ` [PATCH v5 5/7] convert: add 'working-tree-encoding' attribute tboegi
2018-01-30 20:05 ` Junio C Hamano
2018-01-30 20:31 ` Lars Schneider
2018-01-30 21:56 ` Junio C Hamano
2018-01-31 19:12 ` Lars Schneider
2018-01-31 22:45 ` Junio C Hamano
2018-01-29 20:19 ` [PATCH v5 6/7] convert: add tracing for " tboegi
2018-01-29 20:19 ` [PATCH/RFC v5 7/7] Careful with CRLF when using e.g. UTF-16 for working-tree-encoding tboegi
2018-01-30 11:23 ` Lars Schneider
2018-01-30 14:40 ` Torsten Bögershausen
2018-01-30 15:14 ` Lars Schneider
2018-01-31 17:28 ` Torsten Bögershausen
2018-01-31 19:37 ` Lars Schneider
2018-02-02 19:17 ` Junio C Hamano
2018-02-07 6:31 ` Torsten Bögershausen
2018-02-07 18:12 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180123092749.GA6308@ruderich.org \
--to=simon@ruderich.org \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
--cc=lars.schneider@autodesk.com \
--cc=larsxschneider@gmail.com \
--cc=peff@peff.net \
--cc=ramsay@ramsayjones.plus.com \
--cc=sunshine@sunshineco.com \
--cc=tboegi@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.