problem with notmuch-search-authors-width in JSON emacs implementation

problem with notmuch-search-authors-width in JSON emacs implementation

[Jameson Rollins]

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

Hey, folks.  I just build from Carl's HEAD which includes the new emacs
JSON interface and I'm noticing a problem with the
notmuch-search-authors-width variable.  If I set the variable as
follows:

(setq notmuch-search-authors-width 40)

author lists that overrun the specified width start spitting out into
the subject field.  I think bremner just confirmed this on irc.  I'll
try to look into this when I get a chance, but I don't have time right
now.  Maybe someone with more intimate knowledge of this piece of code
could figure it out more quickly.

jamie.

ps: By the way, do we have any official way to keep track of bugs?  Has
anyone looked into any of the distributed, git-based bug tracking
solutions?  We tried doing it with ikiwiki for a different project I was
working on, but it didn't really work that well.

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

Re: problem with notmuch-search-authors-width in JSON emacs implementation

[Dirk Hohndel]

On Wed, 21 Apr 2010 19:09:35 -0400, Jameson Rollins <jrollins@finestructure.net> wrote:
> Hey, folks.  I just build from Carl's HEAD which includes the new emacs
> JSON interface and I'm noticing a problem with the
> notmuch-search-authors-width variable.  If I set the variable as
> follows:
> 
> (setq notmuch-search-authors-width 40)
> 
> author lists that overrun the specified width start spitting out into
> the subject field.  I think bremner just confirmed this on irc.  I'll
> try to look into this when I get a chance, but I don't have time right
> now.  Maybe someone with more intimate knowledge of this piece of code
> could figure it out more quickly.

I think you are no longer supposed to do that. Here's what I have
instead in my .emacs file

 '(notmuch-search-result-format (quote (("date" . "%s ") ("count" . "%-7s ") ("authors" . "%-50s ") ("subject" . "%s ") ("tags" . "(%s)"))))

Actually, I did this from the customize-group -> notmuch settings inside
emacs. I think the overruns come from a conflict between the '20' that's
in the default notmuch-search-result-format and the explicit '40' that
you set here.

Maybe the code should be changed to deal with setting this variable - or
it should be made "internal" :-)

/D

-- 
Dirk Hohndel
Intel Open Source Technology Center

Re: problem with notmuch-search-authors-width in JSON emacs implementation

[Jameson Rollins]

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

On Wed, 21 Apr 2010 19:09:35 -0400, Jameson Rollins <jrollins@finestructure.net> wrote:
> Hey, folks.  I just build from Carl's HEAD which includes the new emacs
> JSON interface and I'm noticing a problem with the
> notmuch-search-authors-width variable.  If I set the variable as
> follows:
> 
> (setq notmuch-search-authors-width 40)
> 
> author lists that overrun the specified width start spitting out into
> the subject field.  I think bremner just confirmed this on irc.  I'll
> try to look into this when I get a chance, but I don't have time right
> now.  Maybe someone with more intimate knowledge of this piece of code
> could figure it out more quickly.

So I think I see what might be happening here, but I don't know elisp
well enough to fix it.  Maybe one of the elisp experts out there can
help me figure it out.

notmuch-search-authors-width is actually called three times in
notmuch.el:

0 servo:~/src/notmuch/git [master] $ grep -nH notmuch-search-authors-width emacs/*
emacs/notmuch.el:57:(defcustom notmuch-search-authors-width 20
emacs/notmuch.el:65:    ("authors" . ,(format "%%-%ds " notmuch-search-authors-width))
emacs/notmuch.el:626:		      (if (> authors-length notmuch-search-authors-width)
emacs/notmuch.el:627:			  (set 'authors (concat (substring authors 0 (- notmuch-search-authors-width 3)) "...")))
0 servo:~/src/notmuch/git [master] $ 

The first call is inside the defcustom definition for
notmuch-search-result-format.  The second two are in the
notmuch-search-process-filter function.  It's pretty clear from the
behavior that setting notmuch-search-authors-width, either with setq or
with "Customize Options", affects what is returned in
notmuch-search-process-filter, but *not* in the defcustom for
notmuch-search-result-format.  I guess what's happening is that the
value used in the defcustom definition of notmuch-search-result-format
is set when the defcustom is defined, and isn't changed later with the
new user-set value.

Do any elisp experts out there know any way out of this?  Is there a way
to escape the expansion of the variable used in
notmuch-search-result-format until it is actually called?

I guess we're using notmuch-search-authors-width to "nicely" truncate
the author list and add the "..." at the end.  We could just straight
truncate the authors field with something like '%-.40s', I think, which
would make the code a little simpler, but might not be as nice.

jamie.

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

Re: problem with notmuch-search-authors-width in JSON emacs implementation

[David Edmondson]

On Thu, 22 Apr 2010 00:37:25 -0400, Jameson Rollins <jrollins@finestructure.net> wrote:
> On Wed, 21 Apr 2010 19:09:35 -0400, Jameson Rollins <jrollins@finestructure.net> wrote:
> > Hey, folks.  I just build from Carl's HEAD which includes the new emacs
> > JSON interface and I'm noticing a problem with the
> > notmuch-search-authors-width variable.  If I set the variable as
> > follows:
> > 
> > (setq notmuch-search-authors-width 40)
> > 
> > author lists that overrun the specified width start spitting out into
> > the subject field.  I think bremner just confirmed this on irc.  I'll
> > try to look into this when I get a chance, but I don't have time right
> > now.  Maybe someone with more intimate knowledge of this piece of code
> > could figure it out more quickly.

I thought that I had this fixed up properly. Sorry for the breakage.

Can you show any explicit settings you have for
`notmuch-search-authors-width' and `notmuch-search-result-format'? Where
do you set them?

The intention was that if you just want a wider display of authors you
set `notmuch-search-authors-width' (which has existed for some time) and
`notmuch-search-result-format' adapts accordingly.

Admittedly, this doesn't cope well if you modify
`notmuch-search-result-format' without also updating
`notmuch-search-authors-width'. Some more thought required...

dme.
-- 
David Edmondson, http://dme.org

Re: problem with notmuch-search-authors-width in JSON emacs implementation

[Jameson Rollins]

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

On Thu, 22 Apr 2010 08:52:59 +0100, David Edmondson <dme@dme.org> wrote:
> On Thu, 22 Apr 2010 00:37:25 -0400, Jameson Rollins <jrollins@finestructure.net> wrote:
> > On Wed, 21 Apr 2010 19:09:35 -0400, Jameson Rollins <jrollins@finestructure.net> wrote:
> > > Hey, folks.  I just build from Carl's HEAD which includes the new emacs
> > > JSON interface and I'm noticing a problem with the
> > > notmuch-search-authors-width variable.  If I set the variable as
> > > follows:
> > > 
> > > (setq notmuch-search-authors-width 40)
> > > 
> > > author lists that overrun the specified width start spitting out into
> > > the subject field.  I think bremner just confirmed this on irc.  I'll
> > > try to look into this when I get a chance, but I don't have time right
> > > now.  Maybe someone with more intimate knowledge of this piece of code
> > > could figure it out more quickly.
> 
> I thought that I had this fixed up properly. Sorry for the breakage.

No worries.

> Can you show any explicit settings you have for
> `notmuch-search-authors-width' and `notmuch-search-result-format'? Where
> do you set them?

Changing notmuch-search-authors-width to anything longer than 20 (which
is the default set internally) causes the problem (I had it set to 40
when I first noticed).  It's very easy to test.  You can
notmuch-search-authors-width on the fly from the configuration manager
and see the result.  I have not modified notmuch-search-result-format.
See my previous email on tracking down the issue.

Thanks for the help.

jamie.

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

Re: problem with notmuch-search-authors-width in JSON emacs implementation

[David Edmondson]

On Thu, 22 Apr 2010 09:22:32 -0400, Jameson Rollins <jrollins@finestructure.net> wrote:
> > Can you show any explicit settings you have for
> > `notmuch-search-authors-width' and `notmuch-search-result-format'? Where
> > do you set them?
> 
> Changing notmuch-search-authors-width to anything longer than 20 (which
> is the default set internally) causes the problem (I had it set to 40
> when I first noticed).  It's very easy to test.  You can
> notmuch-search-authors-width on the fly from the configuration manager
> and see the result.  I have not modified notmuch-search-result-format.
> See my previous email on tracking down the issue.

Ah, when you set `notmuch-search-authors-width' the expression which
creates `notmuch-search-result-format' is not re-evaluated.

Do things get better if you restart emacs (sorry for suggesting that)?

If so then I have an idea how to fix it.

dme.
-- 
David Edmondson, http://dme.org

Re: problem with notmuch-search-authors-width in JSON emacs implementation

[Jameson Rollins]

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

On Thu, 22 Apr 2010 15:04:45 +0100, David Edmondson <dme@dme.org> wrote:
> Ah, when you set `notmuch-search-authors-width' the expression which
> creates `notmuch-search-result-format' is not re-evaluated.
> 
> Do things get better if you restart emacs (sorry for suggesting that)?

I can't see how that would help, because notmuch-search-result-format is
always going to be evaluated with the default value of
notmuch-search-authors-width, which is defined immediately previous to
it.

jamie.

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

Re: problem with notmuch-search-authors-width in JSON emacs implementation

[David Edmondson]

On Thu, 22 Apr 2010 10:12:59 -0400, Jameson Rollins <jrollins@finestructure.net> wrote:
> On Thu, 22 Apr 2010 15:04:45 +0100, David Edmondson <dme@dme.org> wrote:
> > Ah, when you set `notmuch-search-authors-width' the expression which
> > creates `notmuch-search-result-format' is not re-evaluated.
> > 
> > Do things get better if you restart emacs (sorry for suggesting that)?
> 
> I can't see how that would help, because notmuch-search-result-format is
> always going to be evaluated with the default value of
> notmuch-search-authors-width, which is defined immediately previous to
> it.

The `defcustom' does nothing if the variable already has a value.

dme.
-- 
David Edmondson, http://dme.org

Re: problem with notmuch-search-authors-width in JSON emacs implementation

[Jameson Rollins]

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

On Thu, 22 Apr 2010 15:38:55 +0100, David Edmondson <dme@dme.org> wrote:
> The `defcustom' does nothing if the variable already has a value.

But the defcustom is always going to be called before any customization
value, since my personal customizations are always loaded last, so I'm
don't see how this would ever apply.

jamie.

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

Re: problem with notmuch-search-authors-width in JSON emacs implementation

[David Edmondson]

On Thu, 22 Apr 2010 11:20:33 -0400, Jameson Rollins <jrollins@finestructure.net> wrote:
> On Thu, 22 Apr 2010 15:38:55 +0100, David Edmondson <dme@dme.org> wrote:
> > The `defcustom' does nothing if the variable already has a value.
> 
> But the defcustom is always going to be called before any customization
> value, since my personal customizations are always loaded last, so I'm
> don't see how this would ever apply.

I see. The original solution assumed that `notmuch.el' would be loaded
on demand rather than ahead of the call to `custom-set-variables'. I'll
find another solution.

dme.
-- 
David Edmondson, http://dme.org

[PATCH] emacs: Remove `notmuch-search-authors-width' and fix the use of `notmuch-search-result-format' accordingly

[David Edmondson]

The width of the authors field in search output was previously
specified in two places:
 - `notmuch-search-authors-width': the limit beyond which the authors
   names are truncated,
 - `notmuch-search-result-format': the layout of the search results.

Changing the configuration of one of these may have required the user
to know about and adapt the other accordingly. This led to confusion.

Instead, remove `notmuch-search-authors-width' and perform truncation
based on the relevant field in `notmuch-search-result-format'.
---

Jamie, could you test this patch please? My main concern is that it
makes a small assumption about the value of
`notmuch-search-result-format' - namely that the `authors' field ends
with a space.

 emacs/notmuch.el |   16 ++++++----------
 1 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index f96394a..d40c36e 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -55,15 +55,10 @@
 (require 'notmuch-show)
 (require 'notmuch-mua)
 
-(defcustom notmuch-search-authors-width 20
-  "Number of columns to use to display authors in a notmuch-search buffer."
-  :type 'integer
-  :group 'notmuch)
-
 (defcustom notmuch-search-result-format
   `(("date" . "%s ")
     ("count" . "%-7s ")
-    ("authors" . ,(format "%%-%ds " notmuch-search-authors-width))
+    ("authors" . "%-20s ")
     ("subject" . "%s ")
     ("tags" . "(%s)"))
   "Search result formating. Supported fields are:
@@ -585,7 +580,11 @@ matching will be applied."
    ((string-equal field "count")
     (insert (format (cdr (assoc field notmuch-search-result-format)) count)))
    ((string-equal field "authors")
-    (insert (format (cdr (assoc field notmuch-search-result-format)) authors)))
+    (insert (let ((sample (format (cdr (assoc field notmuch-search-result-format)) "")))
+	      (if (> (length authors)
+		     (length sample))
+		  (concat (substring authors 0 (- (length sample) 4)) "... ")
+		(format (cdr (assoc field notmuch-search-result-format)) authors)))))
    ((string-equal field "subject")
     (insert (format (cdr (assoc field notmuch-search-result-format)) subject)))
    ((string-equal field "tags")
@@ -614,12 +613,9 @@ matching will be applied."
 			   (date (match-string 2 string))
 			   (count (match-string 3 string))
 			   (authors (match-string 4 string))
-			   (authors-length (length authors))
 			   (subject (match-string 5 string))
 			   (tags (match-string 6 string))
 			   (tag-list (if tags (save-match-data (split-string tags)))))
-		      (if (> authors-length notmuch-search-authors-width)
-			  (set 'authors (concat (substring authors 0 (- notmuch-search-authors-width 3)) "...")))
 		      (goto-char (point-max))
 		      (let ((beg (point-marker)))
 			(notmuch-search-show-result date count authors subject tags)
-- 
1.7.0

Re: [PATCH] emacs: Remove `notmuch-search-authors-width' and fix the use of `notmuch-search-result-format' accordingly

[Jameson Rollins]

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

On Fri, 23 Apr 2010 11:24:09 +0100, David Edmondson <dme@dme.org> wrote:
> Jamie, could you test this patch please? My main concern is that it
> makes a small assumption about the value of
> `notmuch-search-result-format' - namely that the `authors' field ends
> with a space.

Tested, and it works.  Thanks so much for fixing this, David.  Very much
appreciated.

So this patch gets rid of the notmuch-search-authors-width variable
entirely.  That variable was somewhat convenient (for me at least, since
that was the only thing in the format i wanted to adjust), but I think
the new configuration is more straightforward.  Just set
notmuch-search-result-format how you want it to be, and it Just Works.
I think we should go with this solution...

Approved-By: Jameson Rollins <jrollins@finestructure.net>

(I was going to try to resend the patch with the "Approved-By" bit
added, but I was sure I was going to mess it up.  If someone can point
me to a message where that was done correctly, that'd be great.)

jamie.

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

Re: [PATCH] emacs: Remove `notmuch-search-authors-width' and fix the use of `notmuch-search-result-format' accordingly

[Carl Worth]

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

On Fri, 23 Apr 2010 13:22:24 -0400, Jameson Rollins <jrollins@finestructure.net> wrote:
> On Fri, 23 Apr 2010 11:24:09 +0100, David Edmondson <dme@dme.org> wrote:
> > Jamie, could you test this patch please? My main concern is that it
> > makes a small assumption about the value of
> > `notmuch-search-result-format' - namely that the `authors' field ends
> > with a space.
> 
> Tested, and it works.  Thanks so much for fixing this, David.  Very much
> appreciated.

Thanks to both of you! This is pushed now.

> Approved-By: Jameson Rollins <jrollins@finestructure.net>
> 
> (I was going to try to resend the patch with the "Approved-By" bit
> added, but I was sure I was going to mess it up.  If someone can point
> me to a message where that was done correctly, that'd be great.)

Just putting the above line in an email message is fine. In fact, it's
simpler in that I don't have to analyze two submissions of a patch to
see if there's anything different other than the commit message.

All I have to do for the above is a quick "git commit --amend".

-Carl

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

[PATCH] emacs: Fix `notmuch-search-insert-field'

[David Edmondson]

Compare the formatted version of the authors with the formatted sample
string rather than the un-formatted authors with the formatted sample
string.
---

Carl, please pull this in for 0.3. The previous fix didn't work
correctly when the authors string was one character shorter than the
length specified in the format string (presuming that the format
string is broadly unchanged from the default).

 emacs/notmuch.el |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index 378c004..eecff23 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -582,11 +582,13 @@ matching will be applied."
    ((string-equal field "count")
     (insert (format (cdr (assoc field notmuch-search-result-format)) count)))
    ((string-equal field "authors")
-    (insert (let ((sample (format (cdr (assoc field notmuch-search-result-format)) "")))
-	      (if (> (length authors)
-		     (length sample))
-		  (concat (substring authors 0 (- (length sample) 4)) "... ")
-		(format (cdr (assoc field notmuch-search-result-format)) authors)))))
+    (insert (let* ((format-string (cdr (assoc field notmuch-search-result-format)))
+		   (formatted-sample (format format-string ""))
+		   (formatted-authors (format format-string authors)))
+	      (if (> (length formatted-authors)
+		     (length formatted-sample))
+		  (concat (substring authors 0 (- (length formatted-sample) 4)) "... ")
+		formatted-authors))))
    ((string-equal field "subject")
     (insert (format (cdr (assoc field notmuch-search-result-format)) subject)))
    ((string-equal field "tags")
-- 
1.7.0

Re: [PATCH] emacs: Fix `notmuch-search-insert-field'

[Carl Worth]

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

On Mon, 26 Apr 2010 13:16:07 +0100, David Edmondson <dme@dme.org> wrote:
> Compare the formatted version of the authors with the formatted sample
> string rather than the un-formatted authors with the formatted sample
> string.
> ---
> 
> Carl, please pull this in for 0.3.

Pushed (without testing---just taking your word for it). :-)

-Carl

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