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

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

[Tassilo Horn]

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.

Bye,
Tassilo

---
 notmuch.el |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/notmuch.el b/notmuch.el
index 0cabbe2..c2839c0 100644
--- a/notmuch.el
+++ b/notmuch.el
@@ -169,7 +169,7 @@ Unlike builtin `next-line' this version accepts no arguments."
     (if (not (looking-at notmuch-show-message-begin-regexp))
        (re-search-backward notmuch-show-message-begin-regexp))
     (re-search-forward notmuch-show-id-regexp)
-    (buffer-substring (match-beginning 1) (match-end 1))))
+    (buffer-substring-no-properties (match-beginning 1) (match-end 1))))
 
 (defun notmuch-show-get-filename ()
   (save-excursion
@@ -177,7 +177,7 @@ Unlike builtin `next-line' this version accepts no arguments."
     (if (not (looking-at notmuch-show-message-begin-regexp))
        (re-search-backward notmuch-show-message-begin-regexp))
     (re-search-forward notmuch-show-filename-regexp)
-    (buffer-substring (match-beginning 1) (match-end 1))))
+    (buffer-substring-no-properties (match-beginning 1) (match-end 1))))
 
 (defun notmuch-show-set-tags (tags)
   (save-excursion
-- 
1.6.5.3

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

[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

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

[Tassilo Horn]

Carl Worth <cworth@cworth.org> writes:

Hi Carl,

>> 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.

I guess it won't be the last one.  There are some byte-compiler warnings
with notmuch.el, that I'd like to remove.

> 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).

So an email looking like this would be correct?

--8<---------------cut here---------------start------------->8---
From: Tassilo Horn <tassilo@member.fsf.org>
To: Carl Worth <cworth@cworth.org>
Cc: notmuch@notmuchmail.org
Date: Fri, 27 Nov 2009 08:54:41 +0100
Subject: [PATCH] Remove preprocessor code

Don't define RUNNING_ON_VALGRIND, so that notmuch is probably broken.
---
 debugger.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/debugger.c b/debugger.c
index e8b9378..f32cdc9 100644
--- a/debugger.c
+++ b/debugger.c
@@ -24,8 +24,6 @@
 
 #if HAVE_VALGRIND
 #include <valgrind.h>
-#else
-#define RUNNING_ON_VALGRIND 0
 #endif
 
 notmuch_bool_t
-- 
1.6.5.3

Hi Carl,

this patch is completely wrong.  Please don't apply it. :-)

And thanks again for the great work.

Bye,
Tassilo
--8<---------------cut here---------------end--------------->8---

>   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".

Makes sense.

>      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.

Normally it causes almost never any problems, but IMO it's just bad
style.  When a user wants to get the Message-id, he most probably only
wants to do some calculations on that (e.g. jump to that message in Gnus
or rmail), or insert it somewhere else.  In the first case, text
properties aren't needed, and in the second case, it's most unlikely
that he wants to have exactly the same properties there, especially if
there are properties different from faces.

> 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. :-)

Yes, that all makes perfect sense, and because there are so many people
and patches for notmuch (which is great!), there have to be some strict
guidelines.

But instead of mailing each first-time committer a mail, you might
consider putting those guidelines on the notmuch homepage. :-)

Bye,
Tassilo

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

[Bart Trojanowski]

[[ First of all I am jazzed because this is the first email I am sending to
anyone other than myself from the vim interface to notmuch ]]

Getting on wit the show...

>      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.

You can actually put arbitrary text between the diffstat output and the first
diff --git line.  For example:

--- 8< ---
From e6628e78d9ce3f9383a4699df9063a648617b428 Mon Sep 17 00:00:00 2001
From: Bart Trojanowski <bart@jukie.net>
Date: Fri, 27 Nov 2009 18:02:05 -0500
Subject: [PATCH] this is the patch description

Text that goes here will end up in the git commit.
---
 vim/README             |    3 ++-
 1 files changed, 11 insertions(+), 12 deletions(-)

Anything that goes here will be ignored, so you can typ

diff --git a/vim/README b/vim/README
index 299c7f8..8cd3b1a 100644
--- a/vim/README
+++ b/vim/README
@@ -42,7 +42,8 @@ Buffer types:
         You are presented with the search results when you run :NotMuch.
 
         Keybindings:
-            <Enter> - show the selected message
+            <Space> - show the selected thread colapsing unmatched items
+            <Enter> - show the entire selected thread
             a       - archive message (remove inbox tag)
             f       - filter the current search terms
             o       - toggle search screen order
-- 
1.6.4.4.2.gc2f148

--- 8< ---



-- 
email sent from notmuch.vim plugin

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

[Bart Trojanowski]

* Bart Trojanowski <bart@jukie.net> [091127 18:32]:
> You can actually put arbitrary text between the diffstat output and the first
> diff --git line.  For example:

Oops, and that's exactly what you said...

> >      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).

Sorry.

-- 
				WebSig: http://www.jukie.net/~bart/sig/

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

[Carl Worth]

On Fri, 27 Nov 2009 09:13:12 +0100, Tassilo Horn <tassilo@member.fsf.org> wrote:
> I guess it won't be the last one.  There are some byte-compiler warnings
> with notmuch.el, that I'd like to remove.

Great! I'll look forward to more from you?

> So an email looking like this would be correct?

Almost. The place where you want to insert the explanatory text is
between the "---" marker and the line starting with "diff". See below.

> Normally it causes almost never any problems, but IMO it's just bad
> style.  When a user wants to get the Message-id, he most probably only
> wants to do some calculations on that (e.g. jump to that message in Gnus
> or rmail), or insert it somewhere else.  In the first case, text
> properties aren't needed, and in the second case, it's most unlikely
> that he wants to have exactly the same properties there, especially if
> there are properties different from faces.

Thanks for the explanation. The above paragraph would have been
*perfect* as the commit message.

> But instead of mailing each first-time committer a mail, you might
> consider putting those guidelines on the notmuch homepage. :-)

Yes, there's definitely a bunch of places that would be better to put
these things, (like in a HACKING file in the repository). But sometimes
it's just so *easy* to write email.

I will try to put things into better places before I repeat myself on
the list too much. But anyone can feel free to help by taking text I've
written in email and sending it back as patches to explanatory files
like HACKING.

And eventually I'll fix the website so that other people can contribute
to it as well.

Thanks again for all your help,

-Carl

--8<---------------cut here---------------start------------->8---
From: Tassilo Horn <tassilo@member.fsf.org>
To: Carl Worth <cworth@cworth.org>
Cc: notmuch@notmuchmail.org
Date: Fri, 27 Nov 2009 08:54:41 +0100
Subject: [PATCH] Remove preprocessor code

Don't define RUNNING_ON_VALGRIND, so that notmuch is probably broken.
---
Hi Carl,

this patch is completely wrong.  Please don't apply it. :-)

And thanks again for the great work.

Bye,
Tassilo

 debugger.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/debugger.c b/debugger.c
index e8b9378..f32cdc9 100644
--- a/debugger.c
+++ b/debugger.c
@@ -24,8 +24,6 @@
 
 #if HAVE_VALGRIND
 #include <valgrind.h>
-#else
-#define RUNNING_ON_VALGRIND 0
 #endif
 
 notmuch_bool_t
--8<---------------cut here---------------end--------------->8---

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

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 289 bytes --]

On Fri, 27 Nov 2009 18:31:23 -0500 (EST), Bart Trojanowski <bart@jukie.net> wrote:
> [[ First of all I am jazzed because this is the first email I am
> sending to anyone other than myself from the vim interface to
> notmuch ]]

Congratulations Bart on a fine milestone!

Well done.

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]