[PATCH 0 of 6] Improve readability of non-ascii hg emails (issue814)

Christian Ebert blacktrash at gmx.net
Sat Jul 12 15:28:35 CDT 2008


* Matt Mackall on Friday, July 11, 2008 at 11:48:34 -0500
> On Wed, 2008-07-09 at 17:33 +0100, Christian Ebert wrote:
>> I resubmit a patch series to improve handling of non-ascii mail.
> 
> Hey folks: when you submit a patch that tries tackle a tricky technical
> issue, you should describe exactly what your approach is and why it's
> better than the alternatives up front. This patch series may very well
> be the best possible answer to the problem,

I guarantee that it is not ;)

> and you may have already
> described exactly what your approach is to me last week, but a) I've
> completely forgotten already and b) it needs to be in the changelog
> because I'll have forgotten it again next week. 

Hm, I tried hard in the description you snipped.

> If you do that, I might say "that all seems perfectly sane"

In the interest of your sanity you'd better not.

> and queue it
> right up. Otherwise it may sit in a big pile in my inbox until I have
> time to review it in detail which may never come.
> 
> Some initial comments:
>
> Mailcharset.py can be rolled into mail.py.

Done.

> Using a class for this is inappropriate. The result is not a "natural
> object" - it's not a natural metaphor complex data that substantially
> simplifies thinking and coding. Instead, it's simply there to wrap up
> some vaguely related functions and a -single- variable. It makes things
> more complex and confusing.

Why do it simple, when it can be done complex and confusing?

> Just have one function called charsets(ui) that the various functions
> call and pass ui through them. Then the callers can skip allocating 
> an extra object that's basically a proxy for the ui variable.  

Done.

> Also, don't do the **opts thing. Passing arbitrary sets of options
> around is best avoided. Yes, I know there are a bunch of places we do it
> in hg, and those are all pretty unfortunate. Here you're checking a
> grand total of one option (test), and doing the wrong thing: we should
> actually output the encoding that will be sent! How else can you
> actually "test" mail encoding?

I beg to defer. Something like the following, while correct, is
not helpful to a user:

|Displaying [PATCH] ümläut ...
|Content-Type: text/plain; charset="utf-8"
|MIME-Version: 1.0
|Content-Transfer-Encoding: base64
|Subject: =?iso-8859-1?q?=5BPATCH=5D_=FCml=E4ut?=
|X-Mercurial-Node: 2ed78d3c883382187adf93fdbba3fc66405afe8b
|Message-Id: <2ed78d3c883382187adf.1215873670 at krille.blacktrash.org>
|Date: Sat, 12 Jul 2008 15:41:10 +0100
|From: Christian Ebert <blacktrash at gmx.net>
|To: chris
|
|IyBIRyBjaGFuZ2VzZXQgcGF0Y2gKIyBVc2VyIENocmlzdGlhbiBFYmVydCA8YmxhY2t0cmFzaEBn
|bXgubmV0PgojIERhdGUgMTIwMzE2ODk3MSAtMzYwMAojIE5vZGUgSUQgMmVkNzhkM2M4ODMzODIx
|ODdhZGY5M2ZkYmJhM2ZjNjY0MDVhZmU4YgojIFBhcmVudCAgNTc3MDI3OGZmNzQzMDk3OTVmMGY3
|MjMyYjRlNDA4ZTkwODNkMzllYwrDvG1sw6R1dAoKZGlmZiAtciA1NzcwMjc4ZmY3NDMgLXIgMmVk
|NzhkM2M4ODMzIGEKLS0tIGEvYQlUaHUgRGVjIDIwIDE0OjQ3OjQ3IDIwMDcgKzAxMDAKKysrIGIv
|YQlTYXQgRmViIDE2IDE0OjM2OjExIDIwMDggKzAxMDAKQEAgLTEsMiArMSwzIEBACiAkSWQkCiBo
|ZWxsbworaMO2cm1hbAo=

I think proper testing of encoding should be done in
test-mail-encoding or some such -- which I haven't done yet
because I have to find out how to do this in a portable way
independent of locale setting etc.

I resent with above behaviour for the time being.

c
-- 
  Was heißt hier Dogma, ich bin Underdogma!
[ What the hell do you mean dogma, I am underdogma. ]

_F R E E_  _V I D E O S_  -->>  http://www.blacktrash.org/underdogma/


More information about the Mercurial-devel mailing list