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

2009-11-28 Thread Jed Brown
On Fri, 27 Nov 2009 21:54:11 -0800, Carl Worth  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
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 



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

2009-11-27 Thread Carl Worth
On Fri, 27 Nov 2009 15:18:31 +0100, Jed Brown  wrote:
> On Fri, 27 Nov 2009 05:41:44 -0800, Carl Worth  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
> > "" 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
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 



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

2009-11-27 Thread Jed Brown
On Fri, 27 Nov 2009 05:41:44 -0800, Carl Worth  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
> "" 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
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 



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

2009-11-27 Thread 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  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
"" 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: [notmuch] [PATCH] New function notmuch-show-kill-ring-save-message-id.

2009-11-27 Thread Jed Brown
On Fri, 27 Nov 2009 05:41:44 -0800, Carl Worth cwo...@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


pgpgUNOoP8HQk.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


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

2009-11-24 Thread 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


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

2009-11-24 Thread 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 
---
 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