From: Lars Schneider Date: Wed, 07 Mar 2018 22:49:08 GMT Subject: Re: [PATCH v10 7/9] convert: check for detectable errors in UTF encodings Message-ID: <766B302C-446B-4730-A5D9-909C6FCF6A84@gmail.com> In-Reply-To: > On 07 Mar 2018, at 23:32, Junio C Hamano wrote: > > Lars Schneider writes: > >> I also would have liked to advise "UTF-16" instead of "UTF16" as >> you suggested. However, that required a few more lines and I wanted >> to keep the change to a minimum. I feel this could be added in a >> follow up patch. > > I'd say the whole upcase thing belongs to such a follow-up patch if > that is the case. > >>> On the other hand, if we are not enforcing such a policy decision >>> but merely explaining a way to work around this check, then it may >>> be better to give a variant with the smaller difference from the >>> original (i.e. without up-casing). >> >> See example mentioned above: "Utf-16". How would you handle that? > > Dropping LE suffix from "Utf-16LE" or "Utf16LE" would yield "Utf-16" > or "Utf16" if the advise message does not force policy, or "UTF-16" > in the canoical form if it does. Is there a problem? In the case of has_prohibited_utf_bom() you are right as we are dropping the BE/LE suffix in the advise. However, look at the is_missing_required_utf_bom() advise. Here we *add* BE/LE. At this point I thought it would make sense to make the advised encoding name uppercase in both situations. OK with you? - Lars