[PATCH] New function notmuch-show-kill-ring-save-message-id.

[PATCH] New function notmuch-show-kill-ring-save-message-id.

[Jed Brown]

Puts current message ID in the kill ring.  This is useful any time you
want to explicitly refer to the message, such as in the body of another
message, through git format-patch, or on IRC.

It is bound to "C-c i".

Corrected spelling of function name in commit message, and updated to
apply against HEAD after c1e16435cfe4471c3415d9f625f7230d59c8afb4

Signed-off-by: Jed Brown <jed@59A2.org>
---
 notmuch.el |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/notmuch.el b/notmuch.el
index 907df2c..e3e0e06 100644
--- a/notmuch.el
+++ b/notmuch.el
@@ -80,6 +80,7 @@
     (define-key map "?" 'describe-mode)
     (define-key map (kbd "TAB") 'notmuch-show-next-button)
     (define-key map (kbd "M-TAB") 'notmuch-show-previous-button)
+    (define-key map (kbd "C-c i") 'notmuch-show-kill-ring-save-message-id)
     map)
   "Keymap for \"notmuch show\" buffers.")
 (fset 'notmuch-show-mode-map notmuch-show-mode-map)
@@ -663,6 +664,17 @@ which this thread was originally shown."
       (notmuch-show-markup-message)))
   (notmuch-show-hide-markers))
 
+(defun notmuch-show-kill-ring-save-message-id ()
+  "Put the current message id in the kill ring.
+
+This is useful for referencing messages or running external
+queries."
+  (interactive)
+  (let ((message-id (notmuch-show-get-message-id)))
+    (kill-new message-id)
+    (when (called-interactively-p 'interactive)
+      (message "Saved message ID: \"%s\"" message-id))))
+
 ;;;###autoload
 (defun notmuch-show-mode ()
   "Major mode for viewing a thread with notmuch.
-- 
1.6.5.3

[PATCH] New function notmuch-show-kill-message-id, puts current message ID in the kill ring.

[Jed Brown]

This is useful any time you want to explicitly refer to the message,
such as in the body of another message, through git format-patch, or on
IRC.

It is bound to "C-c i".

Signed-off-by: Jed Brown <jed@59A2.org>
---
 notmuch.el |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/notmuch.el b/notmuch.el
index fa6e7de..071ee5a 100644
--- a/notmuch.el
+++ b/notmuch.el
@@ -84,6 +84,7 @@
     (define-key map "?" 'describe-mode)
     (define-key map (kbd "TAB") 'notmuch-show-next-button)
     (define-key map (kbd "M-TAB") 'notmuch-show-previous-button)
+    (define-key map (kbd "C-c i") 'notmuch-show-kill-ring-save-message-id)
     map)
   "Keymap for \"notmuch show\" buffers.")
 (fset 'notmuch-show-mode-map notmuch-show-mode-map)
@@ -698,6 +699,17 @@ which this thread was originally shown."
   (force-window-update)
   (redisplay t))
 
+(defun notmuch-show-kill-ring-save-message-id ()
+  "Put the current message id in the kill ring.
+
+This is useful for referencing messages or running external
+queries."
+  (interactive)
+  (let ((message-id (notmuch-show-get-message-id)))
+    (kill-new message-id)
+    (when (called-interactively-p 'interactive)
+      (message "Saved message ID: \"%s\"" message-id))))
+
 ;;;###autoload
 (defun notmuch-show-mode ()
   "Major mode for viewing a thread with notmuch.
-- 
1.6.5.3

Re: [PATCH] New function notmuch-show-kill-ring-save-message-id.

[Jed Brown]

It turns out that this ID has id: prefixed (which I thought was fine
because I'm frequently doing another query with it).  But git send-email
doesn't strip that, so this was not threaded correctly.  Would this be
better with the id: prefix stripped?

Jed

Re: [PATCH] New function notmuch-show-kill-ring-save-message-id.

[Carl Worth]

Thanks for the patch, Jed, I almost pushed it, but noticed that it's
calling `called-interactively-p' with an argument even though that
function does not accept an argument. Meanwhile, the documentation of
called-interactively-p suggests that using (interactive "p") is an
easier way to check whether the function is called interactively.

Then, you also had one question:

On Tue, 24 Nov 2009 21:29:56 +0100, Jed Brown <jed@59A2.org> wrote:
> It turns out that this ID has id: prefixed (which I thought was fine
> because I'm frequently doing another query with it).  But git send-email
> doesn't strip that, so this was not threaded correctly.  Would this be
> better with the id: prefix stripped?

No, "git send-email" wouldn't strip that since it's a custom thing for
notmuch.

So for passing the thread ID to notmuch users, the "id:" prefix is
convenient. For passing it not non-notmuch-based consumers it won't be
desired. And that's hard to fix.

I think long-term, a good solution would be to switch from "id:foo" to
"<foo>" as the syntax for message-ID-based search strings. That's then a
syntax that almost any consumer of a message ID should be prepared to
accept.

And that's something that will only really be possible when we bite the
bullet and write our own query parser rather than using the default
parser that Xapian provides.

-Carl

Re: [PATCH] New function notmuch-show-kill-ring-save-message-id.

[Jed Brown]

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

On Fri, 27 Nov 2009 05:41:44 -0800, Carl Worth <cworth@cworth.org> wrote:
> Thanks for the patch, Jed, I almost pushed it, but noticed that it's
> calling `called-interactively-p' with an argument even though that
> function does not accept an argument.

My docs say it does take an argument:

  called-interactively-p is a built-in function in `C source code'.
  
  (called-interactively-p KIND)
  
  Return t if the containing function was called by `call-interactively'.
  If KIND is `interactive', then only return t if the call was made
  interactively by the user, i.e. not in `noninteractive' mode nor
  when `executing-kbd-macro'.
  If KIND is `any', on the other hand, it will return t for any kind of
  interactive call, including being called as the binding of a key, or
  from a keyboard macro, or in `noninteractive' mode.
  
  The only known proper use of `interactive' for KIND is in deciding
  whether to display a helpful message, or how to display it.  If you're
  thinking of using it for any other purpose, it is quite likely that
  you're making a mistake.  Think: what do you want to do when the
  command is called from a keyboard macro?
  
  This function is meant for implementing advice and other
  function-modifying features.  Instead of using this, it is sometimes
  cleaner to give your function an extra optional argument whose
  `interactive' spec specifies non-nil unconditionally ("p" is a good
  way to do this), or via (not (or executing-kbd-macro noninteractive)).

> Meanwhile, the documentation of called-interactively-p suggests that
> using (interactive "p") is an easier way to check whether the function
> is called interactively.

I thought my usage fell precisely under "in deciding whether to display
a helpful message, or how to display it".  This message is just noise if
executed inside a macro.  As further evidence, see copy-region-as-kill
(simple.el):

  ;; This use of called-interactively-p is correct
  ;; because the code it controls just gives the user visual feedback.
  (if (called-interactively-p 'interactive)

Let me know if you still want me to change it.

> No, "git send-email" wouldn't strip that since it's a custom thing for
> notmuch.

Of course, I was just sloppy when pasting it into my shell, which rubbed
this issue in my face:

> So for passing the thread ID to notmuch users, the "id:" prefix is
> convenient. For passing it not non-notmuch-based consumers it won't be
> desired. And that's hard to fix.

I'm thinking of having a prefix determine whether it is stripped or not.
Do you have a preference about which is the non-prefix behavior?

> I think long-term, a good solution would be to switch from "id:foo" to
> "<foo>" as the syntax for message-ID-based search strings. That's then a
> syntax that almost any consumer of a message ID should be prepared to
> accept.

Downside is that it requires shell escapes when pasting into a terminal.

Jed

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

Re: [PATCH] New function notmuch-show-kill-ring-save-message-id.

[Carl Worth]

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

On Fri, 27 Nov 2009 15:18:31 +0100, Jed Brown <jed@59A2.org> wrote:
> On Fri, 27 Nov 2009 05:41:44 -0800, Carl Worth <cworth@cworth.org> wrote:
> > Thanks for the patch, Jed, I almost pushed it, but noticed that it's
> > calling `called-interactively-p' with an argument even though that
> > function does not accept an argument.
> 
> My docs say it does take an argument:

Ah. So we have our first case of emacs-lisp portability issues. This is
what I get:

    called-interactively-p is a built-in function in `C source code'.

    (called-interactively-p)

    Return t if the function using this was called with
    `call-interactively'.  This is used for implementing advice and
    other function-modifying features of Emacs.

    The cleanest way to test whether your function was called with
    `call-interactively' is by adding an extra optional argument, and
    making the `interactive' spec specify non-nil unconditionally for
    that argument.  (`p' is a good way to do this.)

> I thought my usage fell precisely under "in deciding whether to display
> a helpful message, or how to display it".  This message is just noise if
> executed inside a macro.  As further evidence, see copy-region-as-kill
> (simple.el):

Here's what I see for that:

(defun copy-region-as-kill (beg end)
  "Save the region as if killed, but don't kill it.
In Transient Mark mode, deactivate the mark.
If `interprogram-cut-function' is non-nil, also save the text for a window
system cut and paste.

This command's old key binding has been given to `kill-ring-save'."
  (interactive "r")
  (if (eq last-command 'kill-region)
      (kill-append (filter-buffer-substring beg end) (< end beg))
    (kill-new (filter-buffer-substring beg end)))
  (setq deactivate-mark t)
  nil)

No called-interactively anywhere.

> Let me know if you still want me to change it.

I can't apply the patch as it since it just results in an error.

I'm using "GNU emacs 23.1.1" currently, for what it's worth.

> > So for passing the thread ID to notmuch users, the "id:" prefix is
> > convenient. For passing it not non-notmuch-based consumers it won't be
> > desired. And that's hard to fix.
> 
> I'm thinking of having a prefix determine whether it is stripped or not.
> Do you have a preference about which is the non-prefix behavior?

Not a strong preference either way. It's just a few characters after
all.

> > I think long-term, a good solution would be to switch from "id:foo" to
> > "<foo>" as the syntax for message-ID-based search strings. That's then a
> > syntax that almost any consumer of a message ID should be prepared to
> > accept.
> 
> Downside is that it requires shell escapes when pasting into a terminal.

Yeah, there is that.

-Carl

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

Re: [PATCH] New function notmuch-show-kill-ring-save-message-id.

[Jed Brown]

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

On Fri, 27 Nov 2009 21:54:11 -0800, Carl Worth <cworth@cworth.org> wrote:
> Ah. So we have our first case of emacs-lisp portability issues.

Bummer, my docs say nothing about this changing, let alone in a way
that's going to break anything that used it.

> I'm using "GNU emacs 23.1.1" currently, for what it's worth.

GNU Emacs 23.1.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.16.5) of 2009-10-08 on home.sergej.pp.ru

I'll fix it up and add the prefix argument.

Jed

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