Re: [notmuch] [PATCH] Return unpropertized strings for filename and message-id

Subject: Re: [notmuch] [PATCH] Return unpropertized strings for filename and message-id

Date: Thu, 26 Nov 2009 13:42:03 -0800

To: Tassilo Horn, notmuch@notmuchmail.org

Cc:

From: Carl Worth

On Mon, 23 Nov 2009 17:57:31 +0100, Tassilo Horn <tassilo@member.fsf.org> wrote:
> Hi!
> 
> Here's my first patch.  It changes that notmuch-show-get-filename and
> notmuch-show-get-message-id return simple strings and not propertited
> strings.

Thanks, Tassilo!

It's great to have a contribution from you in notmuch. I've pushed this
out now.

Two things with regards to your patch:

  1. It's most convenient (for me) to apply emailed patches by sending
     directly to "git am". And "git am" just happens to want to see the
     complete commit message as the first thing in the mail message,
     (continuing the summary of the commit which comes from the
     subject).

     So to satisfy "git am", introductory and explanatory portions of
     the email, ("Hi!" and "Here's my first patch"), have to be
     relegated to past the "---" divider).

     I actually don't love this about "git am", since I think those
     introductory parts are essential to having cordial and friendly
     exchanges on the mailing list, (rather than just dryly shooting
     code back and forth). And it feels natural to have them first. One
     thing that might be interesting is to teach "git am" about an
     additional divider so that other text can came *before* the commit
     message.

     Alternately, one can put introductory text in one message, and the
     dry commit-only stuff as a reply.

  2. Maybe I'll undermine my point above, but the commit here really
     *does* need a commit message beyond the first line.

     I've described this before as the one-line summary giving the
     "what" and the rest of the commit message giving the "why".

     And this is a perfect case of that. I can see exactly what the
     patch does, and it makes sense. But I tried to write the rest of
     the commit message and found I couldn't. In what cases did the
     presence of text properties cause a problem? I don't know, and
     that's what the commit message should have said.

I'd said before that I would bounce patches with only a one-line
summary. I guess I'm still too soft, but do expect me to be more strict
on this in the future. :-)

-Carl

Thread: