git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Max Kirillov <max@max630.net>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	Jeff King <peff@peff.net>,
	git@vger.kernel.org
Subject: Re: [PATCH] utf8.c: print warning about disabled iconv
Date: Mon, 08 Jun 2015 14:14:48 -0700	[thread overview]
Message-ID: <xmqqoakp9a2f.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20150608210705.GA22850@wheezy.local> (Max Kirillov's message of "Tue, 9 Jun 2015 00:07:05 +0300")

Max Kirillov <max@max630.net> writes:

> User, in theory, can be not the same person who builds, or
> can be not aware that the case needs recoding.

Because you can pretty much say the same for build with iconv
enabled, I think that line of argument is futile.  The users do not
have control over platform's iconv (and sysadmin's choice of locale
packages) what charset/encoding can be converted to what other ones.

>> I actually am OK if the user gets exactly the same warning between
>> the two cases:
>> 
>>  - iconv failed to convert in the real reencode_string_len()
>> 
>>  - we compiled out iconv() and real conversion was asked.
>
> Does 'exactly the same' mean the same text?

No, I was trying to point out the total lack of corresponding
warnings in the iconv-enabled build.

After all, if you had to convert between UTF-8 and ISO-2022-JP, the
latter of which your system does not support, whether you use
iconv-disabled build of Git or iconv-enabled build of Git, we pass
the bytestream through, right?  Your patch gives warning for the
former (which is a good starting point if we want to warn "user
expected them to be converted, we didn't" case) but does not do
anything to the latter, even though users of the iconv-disabled
build is more likely to be aware of the potential issue (and are
likely to be willing to accept that) than the ones with
iconv-enabled build that runs on a sysmet that cannot convert the
specific encoding.

  reply	other threads:[~2015-06-08 21:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-06 21:02 [PATCH] utf8.c: print warning about disabled iconv Max Kirillov
2015-06-08 16:16 ` Junio C Hamano
2015-06-08 21:07   ` Max Kirillov
2015-06-08 21:14     ` Junio C Hamano [this message]
2015-08-14 21:55 ` [PATCH v2] utf8.c: print warning about iconv errors Max Kirillov
2015-08-14 22:35   ` Junio C Hamano
2015-08-17 19:02     ` Jeff King
2015-08-17 19:49       ` 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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqoakp9a2f.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=max@max630.net \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).