pull request

pull request

[David Edmondson]

Carl, a couple of patches that I'd like you to consider. They are the
first two on the `dme' branch of git://github.com/dme/notmuch.git.

http://github.com/dme/notmuch/commit/f86254e4509ed02731aa3eaa8541df1f2d11e400
> notmuch-show: Add unix and pretty dates to the JSON output
> 
> Include a 'date_unix' and 'date_pretty' field in the JSON output for
> each message. 'date_pretty' can be used by a UI implementation,
> whereas 'date_unix' is useful when scripting.

http://github.com/dme/notmuch/commit/dfd99e186ffc6b759c4e09a990c43bb6b8743ef2
> notmuch: Add a 'part' subcommand
> 
> A new 'part' subcommand allows the user to extract a single part from a
> MIME message. Usage:
> 
>   notmuch part --part=<n> <search terms>
> 
> The search terms should match only a single message
> (e.g. id:foo@bar.com). The part number specified refers to the part
> identifiers output by `notmuch show'. The content of the part is written
> the stdout with no formatting or identification marks. It is not JSON
> formatted.

The second of these (part) has been the topic of some
discussion. There's a suggestion that a 'cat' subcommand or
'--format=raw' option to the 'show' subcommand would be better. I'm not
particular preference - I just wanted the functionality to use in the
Emacs UI.

(URLs included as the github frontend is quite nice for perusing the
changes.)

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

Re: pull request

[Carl Worth]

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

On Thu, 01 Apr 2010 15:41:03 +0100, David Edmondson <dme@dme.org> wrote:
> Carl, a couple of patches that I'd like you to consider. They are the
> first two on the `dme' branch of git://github.com/dme/notmuch.git.

Thanks, David!

I've got your branch fetched here and I plan to start playing with
it. The feature-set of the branch sounds extremely compelling, so I'm
looking forward to usability improvements from this. Thanks so much.

> http://github.com/dme/notmuch/commit/f86254e4509ed02731aa3eaa8541df1f2d11e400
> > notmuch-show: Add unix and pretty dates to the JSON output
> > 
> > Include a 'date_unix' and 'date_pretty' field in the JSON output for
> > each message. 'date_pretty' can be used by a UI implementation,
> > whereas 'date_unix' is useful when scripting.

With "date_unix" it's easy enough to guess what the value is. But
"date_pretty" is much more ambiguous. I assumed that this would be an
RFC 822 date string, (but then found that it's the relative date that
"notmuch show" is using currently).

I think I'd rather see that named "date_relative", (or dropped with the
expectation that callers can decide how to format dates on their own).

> > A new 'part' subcommand allows the user to extract a single part from a
> > MIME message. Usage:
> > 
> >   notmuch part --part=<n> <search terms>

This sounds like great functionality, and is something I had intended to
do very early after starting notmuch.el but never got around to.

> > The search terms should match only a single message
> > (e.g. id:foo@bar.com). The part number specified refers to the part
> > identifiers output by `notmuch show'. The content of the part is written
> > the stdout with no formatting or identification marks. It is not JSON
> > formatted.

The above documentation isn't quite complete to me. Is MIME decoding
handled by this or not? Also, the documentation says the search terms
"should match" only one message, but what does the implementation do if
more than one message is matched? It would be good to document that as
well.

More significantly, this level of documentation needs to be put into the
"notmuch help" output (instead of the placeholder that's there in the
current patch). It also needs to be added to the man page in notmuch.1.

> The second of these (part) has been the topic of some
> discussion. There's a suggestion that a 'cat' subcommand or
> '--format=raw' option to the 'show' subcommand would be better. I'm not
> particular preference - I just wanted the functionality to use in the
> Emacs UI.

One other approach that I imagined with the json output would be to
simply include all of the MIME parts of all messages directly in the
json-format output from "notmuch show". Does json have any particular
way of encodign a binary blob? If not, should we just have one single
encoding here? (Such as BASE64 within a json string?)

Looking forward to more,

-Carl

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

Re: pull request

[David Bremner]

On Thu, 01 Apr 2010 14:09:57 -0700, Carl Worth <cworth@cworth.org> wrote:

> More significantly, this level of documentation needs to be put into the
> "notmuch help" output (instead of the placeholder that's there in the
> current patch). It also needs to be added to the man page in notmuch.1.

Allow me to shamelessly plug my podification of notmuch docs :)

  id:1262281169-24909-1-git-send-email-david@tethera.net

This would allow only maintaining the docs in one place. 

David

Re: pull request

[David Edmondson]

On Thu, 01 Apr 2010 14:09:57 -0700, Carl Worth <cworth@cworth.org> wrote:
> On Thu, 01 Apr 2010 15:41:03 +0100, David Edmondson <dme@dme.org> wrote:
> > Carl, a couple of patches that I'd like you to consider. They are the
> > first two on the `dme' branch of git://github.com/dme/notmuch.git.

The updated changes are on the 'dme-for-cworth' branch now.

> > http://github.com/dme/notmuch/commit/f86254e4509ed02731aa3eaa8541df1f2d11e400
> > > notmuch-show: Add unix and pretty dates to the JSON output
> > > 
> > > Include a 'date_unix' and 'date_pretty' field in the JSON output for
> > > each message. 'date_pretty' can be used by a UI implementation,
> > > whereas 'date_unix' is useful when scripting.
> 
> With "date_unix" it's easy enough to guess what the value is. But
> "date_pretty" is much more ambiguous. I assumed that this would be an
> RFC 822 date string, (but then found that it's the relative date that
> "notmuch show" is using currently).
> 
> I think I'd rather see that named "date_relative", (or dropped with the
> expectation that callers can decide how to format dates on their own).

I renamed it to 'date_relative'.

Keeping the creation of the relative date strings in one place struck me
as a good idea - there will consistency between the two places it is
used (search mode and show mode) and if the `notmuch' command is
localised the Emacs UI will immediately benefit.

> > > The search terms should match only a single message
> > > (e.g. id:foo@bar.com). The part number specified refers to the part
> > > identifiers output by `notmuch show'. The content of the part is written
> > > the stdout with no formatting or identification marks. It is not JSON
> > > formatted.
> 
> The above documentation isn't quite complete to me. Is MIME decoding
> handled by this or not? Also, the documentation says the search terms
> "should match" only one message, but what does the implementation do if
> more than one message is matched? It would be good to document that as
> well.
> 
> More significantly, this level of documentation needs to be put into the
> "notmuch help" output (instead of the placeholder that's there in the
> current patch). It also needs to be added to the man page in
> notmuch.1.

The documentation is updated. Sorry for being lazy.

> > The second of these (part) has been the topic of some
> > discussion. There's a suggestion that a 'cat' subcommand or
> > '--format=raw' option to the 'show' subcommand would be better. I'm not
> > particular preference - I just wanted the functionality to use in the
> > Emacs UI.
> 
> One other approach that I imagined with the json output would be to
> simply include all of the MIME parts of all messages directly in the
> json-format output from "notmuch show". Does json have any particular
> way of encodign a binary blob? If not, should we just have one single
> encoding here? (Such as BASE64 within a json string?)

I'm sure that JSON could express the blob. There were two reasons that I
decided not to include the full message in the JSON output:

        - some of the parts can be very large, causing Emacs to spend
          considerably time loading the part (and consume a bunch of
          memory). This would be particularly noticeable in a thread
          where many of the messages include large parts - the UI will
          load all of the parts, even if you've already seen the
          message.

        - there are some parts that the UI will probably never display
          inline usefully (OpenOffice?). For those parts it's quite
          wasteful to have the UI pull them in.

There's obviously a compromise to be had. If we agreed to include
text/html parts in the JSON output it would make sense to me - maybe all
text/* parts should be there. There are probably others that would be
useful.

The later 'display all parts' version of notmuch-show is able to use
either inline or external content to display parts (it uses that
included in the JSON output if present).

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

Re: pull request

[Carl Worth]

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

On Fri, 02 Apr 2010 09:53:04 +0100, David Edmondson <dme@dme.org> wrote:
> The updated changes are on the 'dme-for-cworth' branch now.

Thanks for the quick update, David!

I've pushed this now.

Having played with it a tiny bit, I do think that since the part command
always requires a --part option that we might as well just make it part
of notmuch show. That is:

	notmuch show --part=2 id:foo

instead of:

	notmuch part --part=2 id:foo

It's no more to type and it keeps our top-level command-set simpler.

(Along that same line, I think I'd like to fold "notmuch search-tags"
and perhaps "notmuch count" into sub-options of "notmuch search" as
well.)

But I didn't make the patch block on that. I'm asserting that the
notmuch command-line interface is not yet set in stone and that user
should expect changes.

-Carl

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

Re: pull request

[David Edmondson]

On Fri, 02 Apr 2010 15:53:34 -0700, Carl Worth <cworth@cworth.org> wrote:
> Having played with it a tiny bit, I do think that since the part command
> always requires a --part option that we might as well just make it part
> of notmuch show. That is:
> 
> 	notmuch show --part=2 id:foo
> 
> instead of:
> 
> 	notmuch part --part=2 id:foo
> 
> It's no more to type and it keeps our top-level command-set simpler.

As part of having more information about MIME structure in the JSON
output (multipart/alternative, for example), I think that we (I, if I
get to it first) will need to re-work the `show' implementation
somewhat. We should wait until doing that work to merge the `--part'
support across.

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

Re: pull request

[Carl Worth]

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

On Sat, 03 Apr 2010 07:34:57 +0100, David Edmondson <dme@dme.org> wrote:
> As part of having more information about MIME structure in the JSON
> output (multipart/alternative, for example), I think that we (I, if I
> get to it first) will need to re-work the `show' implementation
> somewhat. We should wait until doing that work to merge the `--part'
> support across.

OK. I've made a note about that in TODO now at least.

-Carl

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

pull request

[David Edmondson]

Carl, please consider the following (from the 'for-cworth' branch of
git://github.com/dme/notmuch.git) for 0.2. I hope to have some more UI
changes merged next week.

commit 651f8ca16beadd658c412075a585e4ec90e31456
Author: David Edmondson <dme@dme.org>
Date:   Mon Mar 22 14:50:20 2010 +0000

    emacs/notmuch-show.el: Avoid passing unintended format strings to `message'
    
    If the text being stashed included %, `message' was unhappy and
    complained.

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

commit 744c8274e56e38940644d31cc6a1588f989daa43
Author: David Edmondson <dme@dme.org>
Date:   Wed Mar 24 15:50:11 2010 +0000

    emacs/notmuch.el: Enable `hl-line-mode' in `notmuch-search-mode'

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

commit 42949084a6dae6997f5872ee9d74da4fc0a89369
Author: David Edmondson <dme@dme.org>
Date:   Tue Apr 6 08:24:00 2010 +0100

    json: Avoid calling strlen(NULL)
    
    MIME parts may have no filename, which previously resulted in calling
    strlen(NULL).

 json.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

commit 94ba1fa0de79902c94612dad84e32fdcdc8a34dc
Author: David Edmondson <dme@dme.org>
Date:   Sun Apr 11 08:58:43 2010 +0100

    emacs: JSON based implementation
    
    Re-implement notmuch-show.el using the JSON output format of the
    notmuch command. Most functionality is retained - HTML display is
    noticeably missing.

 emacs/notmuch-lib.el  |   11 -
 emacs/notmuch-show.el | 1620 +++++++++++++++++++++++++------------------------
 emacs/notmuch.el      |   69 +--
 3 files changed, 837 insertions(+), 863 deletions(-)

commit a586736deaf934ac348125499601b00e71d7c841
Author: David Edmondson <dme@dme.org>
Date:   Mon Mar 22 16:49:16 2010 +0000

    emacs: Move body markup to a separate file
    
    Move the citation and signature markup for text/plain parts to a new
    file (notmuch-wash.el) and call it using a hook mechanism rather than
    directly.

 emacs/Makefile.local  |    3 +-
 emacs/notmuch-show.el |  131 ++-----------------------------------------
 emacs/notmuch-wash.el |  150 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 157 insertions(+), 127 deletions(-)

commit e4fe7e4274f9669aa8040d34b2df5f6571ff9b61
Author: David Edmondson <dme@dme.org>
Date:   Thu Apr 1 16:25:20 2010 +0100

    emacs: Add more functions to clean up text/plain parts
    
    Add:
    - notmuch-wash-wrap-long-lines: Wrap lines longer than the width of
      the current window whilst maintaining any citation prefix.
    - notmuch-wash-tidy-citations: Tidy up citations by:
      - compress repeated otherwise blank citation lines,
      - remove otherwise blank citation lines at the head and tail of a
        citation and remove blank lines between attribution statements and
        the citation,
    - notmuch-wash-compress-blanks: Compress repeated blank lines and
      remove leading and trailing blank lines.
    
    Enable all of the functions by default by adding them to
    `notmuch-show-insert-text/plain-hook'.
    
    With the wrapping features for text/plain parts enabled, word wrapping
    of the buffer leads to an unappealing display of text, so disable it.

 emacs/Makefile.local  |    3 +-
 emacs/coolj.el        |  145 +++++++++++++++++++++++++++++++++++++++++++++++++
 emacs/notmuch-show.el |   14 ++++-
 emacs/notmuch-wash.el |   72 ++++++++++++++++++++++++-
 4 files changed, 230 insertions(+), 4 deletions(-)

commit 53544b7907b3945b06c10113e7900b59113e405f
Author: David Edmondson <dme@dme.org>
Date:   Tue Mar 23 10:06:00 2010 +0000

    emacs/notmuch-show.el: Improved part labelling
    
    If a text/plain part is not the first part in a message, add a label
    in order that a user can see that multiple parts are present.
    
    If a part has a 'filename' attribute, include it in any label
    describing the part.

 emacs/notmuch-show.el |   26 +++++++++++++++++---------
 1 files changed, 17 insertions(+), 9 deletions(-)

commit 8144f6553b443a916c599d7fbb347c557923e71f
Author: David Edmondson <dme@dme.org>
Date:   Tue Mar 23 11:54:05 2010 +0000

    emacs: Use `mm-display-part' when possible
    
    For parts that the mm-decode/mm-view functions can inline and we have
    the content, use `mm-display-part' to insert the part in the
    buffer.

 emacs/notmuch-show.el |   30 ++++++++++++++++++++++--------
 1 files changed, 22 insertions(+), 8 deletions(-)

commit bf3da2f2422539f9c099158457b84482b22fbd41
Author: David Edmondson <dme@dme.org>
Date:   Tue Mar 23 11:54:05 2010 +0000

    emacs: Use mailcap.el to guess the type of application/octet-stream parts
    
    Use the mailcap functionality to guess a MIME type for attachments of
    type application/octet-stream and, presuming successful, feed the
    attachment back into the display code with the determine type.
    
    This is mostly useless at the moment, as the JSON output from notmuch
    does not include the content of application/octet-stream parts, so
    they cannot be displayed even if the guess is a good one.

 emacs/notmuch-show.el |   54 +++++++++++++++++++++++++++++++++++-------------
 1 files changed, 39 insertions(+), 15 deletions(-)

commit c8946454e1f39845db5e9dd90fdfcff55b2a0c86
Author: David Edmondson <dme@dme.org>
Date:   Thu Apr 1 18:33:46 2010 +0100

    emacs: Display all body parts using `notmuch part --part=<n>'
    
    Use the `notmuch part' command to access body parts not currently
    included in the JSON output and display those body parts
    appropriately.

 emacs/notmuch-show.el |   82 ++++++++++++++++++++++++++++--------------------
 1 files changed, 48 insertions(+), 34 deletions(-)

commit 42b98a1402347551b19fa912b574c2c1e45d0ede
Author: David Edmondson <dme@dme.org>
Date:   Thu Mar 25 12:26:49 2010 +0000

    emacs/notmuch-wash.el: Add `notmuch-wash-inline-patch'
    
    `notmuch-wash-inline-patch' attempts to convert inline patches to fake
    attachments, in order that diff-mode highlighting can be applied to
    the patch. It should be added to
    `notmuch-show-insert-text/plain-hook', usually before
    `notmuch-wash-markup-citations'.
    
    Due to the scope for error in detecting inline patches (and their
    extent), this function is not enabled by default.

 emacs/notmuch-wash.el |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

commit ad8deaa17f1753cedd8e0a593376f3bc32e2d000
Author: David Edmondson <dme@dme.org>
Date:   Sun Mar 28 14:50:46 2010 +0100

    emacs/notmuch-show.el: Part headers are real buttons that save the part
    
    Convert the part headers into buttons that save the part when
    activated. Don't attempt to save 'fake' parts generated by
    `notmuch-wash-inline-patch'.

 emacs/notmuch-show.el |   56 ++++++++++++++++++++++++++++++++++++------------
 emacs/notmuch-wash.el |    2 +-
 2 files changed, 43 insertions(+), 15 deletions(-)

commit 912c215bf3636945abf50cf35c96691b0e244bf4 (HEAD, github/for-cworth, for-cworth)
Author: David Edmondson <dme@dme.org>
Date:   Mon Mar 29 10:31:58 2010 +0100

    emacs/notmuch-show.el: Add `notmuch-show-toggle-all' bound to M-RET
    
    `notmuch-show-toggle-all' changes the visibility all of the messages
    in the current thread. By default it makes all of the messages not
    visible. With a prefix argument, it makes them all visible.

 emacs/notmuch-show.el |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

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

Re: pull request

[David Edmondson]

On Sun, 11 Apr 2010 11:29:29 +0100, David Edmondson <dme@dme.org> wrote:
> Carl, please consider the following (from the 'for-cworth' branch of
> git://github.com/dme/notmuch.git) for 0.2. I hope to have some more UI
> changes merged next week.

This is the same set rebased onto 0.2.

commit 8586a86b9dd4ed2406a2fbda6c08bdc6a598cfd8
Author: David Edmondson <dme@dme.org>
Date:   Sat Apr 10 09:02:32 2010 +0100

    debian: git should ignore packaging intermediate files

commit f5de1bb5b4216a1950f6aa5f471c9964e4d0e521
Author: David Edmondson <dme@dme.org>
Date:   Mon Mar 22 14:50:20 2010 +0000

    emacs/notmuch-show.el: Avoid passing unintended format strings to `message'
    
    If the text being stashed included %, `message' was unhappy and
    complained.

commit 4159baa2166a3410663adc200f91145edf8e0e13
Author: David Edmondson <dme@dme.org>
Date:   Wed Mar 24 15:50:11 2010 +0000

    emacs/notmuch.el: Enable `hl-line-mode' in `notmuch-search-mode'

commit 53c4e64943d09b07e75c9258fc9f954c87a490d6
Author: David Edmondson <dme@dme.org>
Date:   Tue Apr 6 08:24:00 2010 +0100

    json: Avoid calling strlen(NULL)
    
    MIME parts may have no filename, which previously resulted in calling
    strlen(NULL).

commit 7dedc95af671173a57bafd973604614c03121ce6
Author: David Edmondson <dme@dme.org>
Date:   Sun Apr 11 08:58:43 2010 +0100

    emacs: JSON based implementation
    
    Re-implement notmuch-show.el using the JSON output format of the
    notmuch command. Most functionality is retained - HTML display is
    noticeably missing.

commit 514e14c42e214718768a1ec94cb869cd3eb47114
Author: David Edmondson <dme@dme.org>
Date:   Mon Mar 22 16:49:16 2010 +0000

    emacs: Move body markup to a separate file
    
    Move the citation and signature markup for text/plain parts to a new
    file (notmuch-wash.el) and call it using a hook mechanism rather than
    directly.

commit 2b6201fbf9209a875f216d48c30b95a6f583c575
Author: David Edmondson <dme@dme.org>
Date:   Thu Apr 1 16:25:20 2010 +0100

    emacs: Add more functions to clean up text/plain parts
    
    Add:
    - notmuch-wash-wrap-long-lines: Wrap lines longer than the width of
      the current window whilst maintaining any citation prefix.
    - notmuch-wash-tidy-citations: Tidy up citations by:
      - compress repeated otherwise blank citation lines,
      - remove otherwise blank citation lines at the head and tail of a
        citation and remove blank lines between attribution statements and
        the citation,
    - notmuch-wash-compress-blanks: Compress repeated blank lines and
      remove leading and trailing blank lines.
    
    Enable all of the functions by default by adding them to
    `notmuch-show-insert-text/plain-hook'.
    
    With the wrapping features for text/plain parts enabled, word wrapping
    of the buffer leads to an unappealing display of text, so disable it.

commit c7872f5e1f11cfa10d93cb818c5f6f6c0835b918
Author: David Edmondson <dme@dme.org>
Date:   Tue Mar 23 10:06:00 2010 +0000

    emacs/notmuch-show.el: Improved part labelling
    
    If a text/plain part is not the first part in a message, add a label
    in order that a user can see that multiple parts are present.
    
    If a part has a 'filename' attribute, include it in any label
    describing the part.

commit 951db85a55a1893e766b26de1377dda5b4573366
Author: David Edmondson <dme@dme.org>
Date:   Tue Mar 23 11:54:05 2010 +0000

    emacs: Use `mm-display-part' when possible
    
    For parts that the mm-decode/mm-view functions can inline and we have
    the content, use `mm-display-part' to insert the part in the
    buffer.

commit 5c060ded87ec4dc479625348708ef73852d60b36
Author: David Edmondson <dme@dme.org>
Date:   Tue Mar 23 11:54:05 2010 +0000

    emacs: Use mailcap.el to guess the type of application/octet-stream parts
    
    Use the mailcap functionality to guess a MIME type for attachments of
    type application/octet-stream and, presuming successful, feed the
    attachment back into the display code with the determine type.
    
    This is mostly useless at the moment, as the JSON output from notmuch
    does not include the content of application/octet-stream parts, so
    they cannot be displayed even if the guess is a good one.

commit 5fcbb528384b7bda838f7c77434def15f85c7382
Author: David Edmondson <dme@dme.org>
Date:   Thu Apr 1 18:33:46 2010 +0100

    emacs: Display all body parts using `notmuch part --part=<n>'
    
    Use the `notmuch part' command to access body parts not currently
    included in the JSON output and display those body parts
    appropriately.

commit 9e193a3998b7503e35d21013c71cc4ecaf6c9d50
Author: David Edmondson <dme@dme.org>
Date:   Thu Mar 25 12:26:49 2010 +0000

    emacs/notmuch-wash.el: Add `notmuch-wash-inline-patch'
    
    `notmuch-wash-inline-patch' attempts to convert inline patches to fake
    attachments, in order that diff-mode highlighting can be applied to
    the patch. It should be added to
    `notmuch-show-insert-text/plain-hook', usually before
    `notmuch-wash-markup-citations'.
    
    Due to the scope for error in detecting inline patches (and their
    extent), this function is not enabled by default.

commit 444de7e73d988cab9b8d1fef851c8ad26174a996
Author: David Edmondson <dme@dme.org>
Date:   Sun Mar 28 14:50:46 2010 +0100

    emacs/notmuch-show.el: Part headers are real buttons that save the part
    
    Convert the part headers into buttons that save the part when
    activated. Don't attempt to save 'fake' parts generated by
    `notmuch-wash-inline-patch'.

commit e9d737feb5a49fd59e1f27bccd24cac2fd1ef749
Author: David Edmondson <dme@dme.org>
Date:   Mon Mar 29 10:31:58 2010 +0100

    emacs/notmuch-show.el: Add `notmuch-show-toggle-all' bound to M-RET
    
    `notmuch-show-toggle-all' changes the visibility all of the messages
    in the current thread. By default it makes all of the messages not
    visible. With a prefix argument, it makes them all visible.

commit b3be927b54956a7258f203159e0bdb954e686c80
Author: David Edmondson <dme@dme.org>
Date:   Mon Apr 12 08:51:30 2010 +0100

    emacs: Support for customizing search result display
    
    This patch helps in customizing search result display similar to
    mutt's index_format. The customization is done by defining an alist as
    below:
    
    (setq notmuch-search-result-format '(("date" . "%s ")
    				     ("authors" . "%-40s ")
    				     ("subject" . "%s ")))
    
    The supported keywords are date, count, authors, subject and tags.
    
    Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@gmail.com>
    Signed-off-by: David Edmondson <dme@dme.org>

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

Re: pull request

[Carl Worth]

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

On Mon, 19 Apr 2010 09:27:39 +0100, David Edmondson <dme@dme.org> wrote:
> This is the same set rebased onto 0.2.

Thanks for these, David!

>  commit 8586a86b9dd4ed2406a2fbda6c08bdc6a598cfd8
>     debian: git should ignore packaging intermediate files

I committed an alternate version of this, (with a new debian/.gitignore
file). I used more wildcarding too. And I couldn't actually find how to
make all these files appear. If you need more ignores, just let me know
(and let me know what commands trigger those, since I'm curious).

>  commit f5de1bb5b4216a1950f6aa5f471c9964e4d0e521
>     emacs/notmuch-show.el: Avoid passing unintended format strings to
>     `message'

Thanks. I've committed this.

> commit 4159baa2166a3410663adc200f91145edf8e0e13
> 
>     emacs/notmuch.el: Enable `hl-line-mode' in `notmuch-search-mode'

I've got some misgivings about this one. First, notmuch-search-hook is
a hook for the user to manipulate, while the hl-line-mode functionality
is something that should be on by default. That is, if the user happens
to set the search-hook then the hl-line-mode shouldn't magically
disappear.

Meanwhile, I *am* getting hl-line-mode in notmuch-search-mode
already. I'm not sure where that's coming from. (I also don't know
what's up with the current "options '(hl-line-mode)" which doesn't seem
right.)

> commit 53c4e64943d09b07e75c9258fc9f954c87a490d6
>     json: Avoid calling strlen(NULL)

That commit message describes this hunk of the patch:

> +    if (str == NULL)
> +       str = "";
> +
>     return (json_quote_chararray (ctx, str, strlen (str)));

But this other hunk looks independent. What's going on here?

> -    if (len == 0)
> -       return (char *)"\"\"";
> -

So I haven't committed this piece yet.

That brings me up to the big JSON rewrite, which I'll start testing and
review in a separate reply.

-Carl

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

Re: pull request

[Carl Worth]

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

On Mon, 19 Apr 2010 11:07:40 -0700, Carl Worth <cworth@cworth.org> wrote:
> On Mon, 19 Apr 2010 09:27:39 +0100, David Edmondson <dme@dme.org> wrote:
> 
> I've got some misgivings about this one. First, notmuch-search-hook is
> a hook for the user to manipulate, while the hl-line-mode functionality
> is something that should be on by default. That is, if the user happens
> to set the search-hook then the hl-line-mode shouldn't magically
> disappear.

I forgot. The user is expected to use add-hook which avoids this very
problem. So this patch is fine.

-Carl

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

Re: pull request

[David Edmondson]

On Mon, 19 Apr 2010 11:07:40 -0700, Carl Worth <cworth@cworth.org> wrote:
> >  commit 8586a86b9dd4ed2406a2fbda6c08bdc6a598cfd8
> >     debian: git should ignore packaging intermediate files
> 
> I committed an alternate version of this, (with a new debian/.gitignore
> file). I used more wildcarding too. And I couldn't actually find how to
> make all these files appear. If you need more ignores, just let me know
> (and let me know what commands trigger those, since I'm curious).

From memory I ran 'debuild'.

> > commit 53c4e64943d09b07e75c9258fc9f954c87a490d6
> >     json: Avoid calling strlen(NULL)
> 
> That commit message describes this hunk of the patch:
> 
> > +    if (str == NULL)
> > +       str = "";
> > +
> >     return (json_quote_chararray (ctx, str, strlen (str)));
> 
> But this other hunk looks independent. What's going on here?
> 
> > -    if (len == 0)
> > -       return (char *)"\"\"";
> > -
> 
> So I haven't committed this piece yet.

The second chunk was intended to cover a similar case (len == 0), but
becomes unnecessary after the first chunk. At least, that's what I
convinced myself after the conversation with Anthony Towns
(id:h2y87b3a4191004060117v5421db8ejbe3030d0626e7440@mail.gmail.com).

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

Re: pull request

[Carl Worth]

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

On Tue, 20 Apr 2010 06:27:02 +0100, David Edmondson <dme@dme.org> wrote:
> The second chunk was intended to cover a similar case (len == 0), but
> becomes unnecessary after the first chunk. At least, that's what I
> convinced myself after the conversation with Anthony Towns
> (id:h2y87b3a4191004060117v5421db8ejbe3030d0626e7440@mail.gmail.com).

Thanks for the clarification.

And I really appreciated seeing a reference to the original discussion
that led to this patch. You'll notice there that Anthony's proposed
commit had the same patch content that you had in your tree, but with a
more detailed commit message, ("and always return a newly talloced
array").

That was exactly the kind of explanation I was looking for but couldn't
find in the commit I first reviewed. It wasn't really a question of
whether the code was correct. The problem was that there was a code
change that wasn't described in the commit message. I don't want that
even if the change is correct.

Anyway, thanks AJ and David.

I've now pushed my version of these changes up through this point. I'm
currently working on the make-emacs-use-JSON patch, (it's got some
confusion about "body visible" vs. "message visible" that I want to fix
before pushing).

-Carl

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

Re: pull request

[David Edmondson]

On Tue, 20 Apr 2010 09:25:36 -0700, Carl Worth <cworth@cworth.org> wrote:
> I'm currently working on the make-emacs-use-JSON patch, (it's got some
> confusion about "body visible" vs. "message visible" that I want to
> fix before pushing).

Could you describe the confusion?

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

Re: pull request

[Carl Worth]

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

On Wed, 21 Apr 2010 13:39:50 +0100, David Edmondson <dme@dme.org> wrote:
> On Tue, 20 Apr 2010 09:25:36 -0700, Carl Worth <cworth@cworth.org> wrote:
> > I'm currently working on the make-emacs-use-JSON patch, (it's got some
> > confusion about "body visible" vs. "message visible" that I want to
> > fix before pushing).
> 
> Could you describe the confusion?

Sure. Hiding a message with 'b' is visually identical to hiding it with
RET. Except that the internal mechanism is distinct, so that afterwards
one can't make it visible again with RET.

I think we just need to drop 'b'---it's really not needed as a separate
keybinding now that RET works on any part of a message. And after that,
there's some simplification to be made in the code.[*]

I'll have some patches pushed for this soon, (on top of the switch to
JSON for the emacs code).

-Carl

[*] Even more simplification is possible if we stop trying to hide
header components. Several people have requested that To and Cc be
visible all the time.

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

Re: pull request

[Jameson Rollins]

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

On Wed, 21 Apr 2010 12:15:48 -0700, Carl Worth <cworth@cworth.org> wrote:
> [*] Even more simplification is possible if we stop trying to hide
> header components. Several people have requested that To and Cc be
> visible all the time.

Hey, Carl.  I had actually been meaning to bring this up.  I actually
don't like having the headers collapsed at all, so I am definitely in
favor of the idea of doing away with this "feature".  At the very least,
I would like to see header hiding/collapsing be an option, so that I
could turn it off.  As is, I'm having to hit 'h' for every message I
view, so I can just see who the message was sent to (not to mention to
recover the unfortunately missing blank line between the header and the
body, which is visually useful).

jamie.

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

Re: pull request

[Carl Worth]

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

On Mon, 19 Apr 2010 09:27:39 +0100, David Edmondson <dme@dme.org> wrote:
> commit 7dedc95af671173a57bafd973604614c03121ce6
> 
>     emacs: JSON based implementation

I merged this and followed it up immediately with a commit to remove the
ability to toggle the body visibility separate from the message
visibility. That was just confusing and none too useful.

> commit 514e14c42e214718768a1ec94cb869cd3eb47114
>
>     emacs: Move body markup to a separate file

This is pushed.

> commit 2b6201fbf9209a875f216d48c30b95a6f583c575
> 
>     emacs: Add more functions to clean up text/plain parts
>     
>     Add:
>     - notmuch-wash-wrap-long-lines: Wrap lines longer than the width of
>       the current window whilst maintaining any citation prefix.
>     - notmuch-wash-tidy-citations: Tidy up citations by:
>       - compress repeated otherwise blank citation lines,
>       - remove otherwise blank citation lines at the head and tail of a
>         citation and remove blank lines between attribution statements and
>         the citation,
>     - notmuch-wash-compress-blanks: Compress repeated blank lines and
>       remove leading and trailing blank lines.

I did not push this one. I think this munges messages too much to be on
by default.

Consider a window of 80 columns and a thread with messages all wrapped
at a reasonable boundary, (75 columns or whatever). And consider that
the thread causes indentation such that messages start running into the
right edge of the window.

Before the above patch, this scenario will cause the message to be
displayed in an obviously incorrect fashion, (some lines will only have
one or two words displayed on them). But since the content is unchanged
From the original message (other than the extra indentation whitespace)
I can just widen my emacs window and view the message correctly.

After the patch, the broken case looks a little nicer, (lines that have
only one or two words are now indented and citation markers are added),
but if I widen the window nothing gets fixed. So that's annoying.

With sup, the approach for this scenarios was to effectively make the
buffer much wider, and to automatically scroll the buffer to the left
when necessary so that the current message was made visible with no
wrapping. I liked that.

Dealing with broken messages that have entire paragraphs unwrapped is a
separate issue from displaying correct messages nicely. I'd like to make
correct messages work well first, and then worry about the broken
messages. (For those, I'm much more willing to accept munged display of
the message.)

> commit c7872f5e1f11cfa10d93cb818c5f6f6c0835b918
>
>     emacs/notmuch-show.el: Improved part labelling
... 
> commit 951db85a55a1893e766b26de1377dda5b4573366
> 
>     emacs: Use `mm-display-part' when possible
...
> commit 5c060ded87ec4dc479625348708ef73852d60b36
> 
>     emacs: Use mailcap.el to guess the type of application/octet-stream parts
...
> commit 5fcbb528384b7bda838f7c77434def15f85c7382
> 
>     emacs: Display all body parts using `notmuch part --part=<n>'

All four of these are pushed now. I like the inline display of
images. (Though I wish that emacs would allow the scrollbar to display
partial images---the sudden disappearance of the message once it hits
the top of the window is annoying.)

> commit 9e193a3998b7503e35d21013c71cc4ecaf6c9d50
> 
>     emacs/notmuch-wash.el: Add `notmuch-wash-inline-patch'

The commit message for this one was enough to make me skip it for now:

>     Due to the scope for error in detecting inline patches (and their
>     extent), this function is not enabled by default.

For this entire series it would be very nice to have some emacs-based
testing in the test suite. And for a patch like this with heuristics
that are known to not be perfect, I think it's essential to have that
testing in place first.

> commit 444de7e73d988cab9b8d1fef851c8ad26174a996
> 
>     emacs/notmuch-show.el: Part headers are real buttons that save the part

Pushed. This is a delightful improvement to our handling of attachments.

I'd like the text on the button to give some more instruction, such as:

	"Click or press Enter to save."

And if we could pull it off, (I don't know how flexible the button
widget is in emacs), it would be nice to have something like this as
well:

	"Or press V to view."

so that one could easily use an external viewer for a single attachment.

Meanwhile, another issue with the result of this series is that I now
seem to get rendering for both the text/plain and the text/html
alternatives when a message has both. For now, the paragraphs are
wrapped much more nicely in the rendering of the html portion, but links
are apparently entirely missing. The link URLs at least appear in the
text/plain rendering, (which is pretty ugly, but at least not impossible
to use).

If we could get one version or the other working completely, then it
would be nice to display only one.

>     Don't attempt to save 'fake' parts generated by
>     `notmuch-wash-inline-patch'.

There was one portion of the patch that did not apply since I hadn't
applied an earlier patch. It looked to me like it was the portion
described by the above sentence, so I dropped that sentence from the
commit message.

> commit e9d737feb5a49fd59e1f27bccd24cac2fd1ef749
> 
>     emacs/notmuch-show.el: Add `notmuch-show-toggle-all' bound to M-RET
>     
>     `notmuch-show-toggle-all' changes the visibility all of the messages
>     in the current thread. By default it makes all of the messages not
>     visible. With a prefix argument, it makes them all visible.

I didn't push this one yet.

The feature is *almost* what I want. It's just that I often want to open
all messages in a thread, (and I've never found myself wanting to close
all messages). So I'd like to switch which behavior requires the prefix
argument here.

Also, care should be taken in the documentation of this function so that
the first line reads as a complete sentence. This is because our help
screen (with the "?" key) displays only the first line of help for each
function.

[Aside: We could improve our help screen by making RET on any line then
bring up the full help message for that function.]

> commit b3be927b54956a7258f203159e0bdb954e686c80
> 
>     emacs: Support for customizing search result display

Finally, this is pushed. It's a quite lovely feature that's quite easy
to take advantage of from "M-x customize-group" "notmuch".

Anyone, if you haven't tried that mode yet for customizing notmuch, you
should give it a try. (And we should add some hint somewhere to make it
easier to find. Perhaps a keybinding to get to the customization
buffer.)

Thanks again, David!

-Carl

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

Re: pull request

[David Edmondson]

On Wed, 21 Apr 2010 12:15:48 -0700, Carl Worth <cworth@cworth.org> wrote:
> Sure. Hiding a message with 'b' is visually identical to hiding it with
> RET.

That's not quite true. `b' will (would) hide only the body. If the
header is visible `b' will not hide it.

> Except that the internal mechanism is distinct, so that afterwards one
> can't make it visible again with RET.

The internal mechanism is distinct because it does a different
thing. The original code drew a distinction between header visibility,
body visibility and message-as-a-whole visibility. I agree that visually
there is no difference between 'header and body hidden' and 'message
hidden'. It's possible to have only the header visible, which I've yet
to find a use for. The code works as I intended, though I'd agree that
may not be the desired behaviour :-)

Given that I've never used `b' other than in testing, I'm not worried
about it going away.

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

Re: pull request

[David Edmondson]

On Wed, 21 Apr 2010 14:03:03 -0700, Carl Worth <cworth@cworth.org> wrote:
> > commit 2b6201fbf9209a875f216d48c30b95a6f583c575
> > 
> >     emacs: Add more functions to clean up text/plain parts
> >     
> >     Add:
> >     - notmuch-wash-wrap-long-lines: Wrap lines longer than the width of
> >       the current window whilst maintaining any citation prefix.
> >     - notmuch-wash-tidy-citations: Tidy up citations by:
> >       - compress repeated otherwise blank citation lines,
> >       - remove otherwise blank citation lines at the head and tail of a
> >         citation and remove blank lines between attribution statements and
> >         the citation,
> >     - notmuch-wash-compress-blanks: Compress repeated blank lines and
> >       remove leading and trailing blank lines.
> 
> I did not push this one. I think this munges messages too much to be on
> by default.

Your concerns appear to be about `notmuch-wash-wrap-long-lines'. I agree
that it can generate unfortunate results in the "deep thread, narrow
terminal" case. Are the other two functions okay?

One of the reasons that I chose a hook-based approach is to allow the
user to decide which body cleaning should be applied. If you don't want
the line-wrapping to be enabled by default we can leave it out but
include the definition so that a user can enable it. The inline-patch
formatting is similar (I'll say more about that in response to your
comments on it).

> Dealing with broken messages that have entire paragraphs unwrapped is a
> separate issue from displaying correct messages nicely. I'd like to make
> correct messages work well first, and then worry about the broken
> messages. (For those, I'm much more willing to accept munged display of
> the message.)

Enabling `notmuch-wash-tidy-citations' and
`notmuch-wash-compress-blanks' by default would fit with this policy, I
think.

Then again, including the functions in the code but enabling none of
them by default would also be acceptable (I already enable them all and
have another function that is not shared).

> > commit 9e193a3998b7503e35d21013c71cc4ecaf6c9d50
> > 
> >     emacs/notmuch-wash.el: Add `notmuch-wash-inline-patch'
> 
> The commit message for this one was enough to make me skip it for now:
> 
> >     Due to the scope for error in detecting inline patches (and their
> >     extent), this function is not enabled by default.
> 
> For this entire series it would be very nice to have some emacs-based
> testing in the test suite. And for a patch like this with heuristics
> that are known to not be perfect, I think it's essential to have that
> testing in place first.

Testing for Emacs is something that I'm looking into. There doesn't seem
to be a de-facto best framework.

Aside from the need to test, this particular patch will benefit only
minimally from testing - there will always be false positive cases
however good the analysis used. Are you saying that we can not tolerate
any false-positives, or that we just need to demonstrate that it is down
to some acceptable level? (Though I've no idea how I would achieve the
latter.)

> > commit 444de7e73d988cab9b8d1fef851c8ad26174a996
> > 
> >     emacs/notmuch-show.el: Part headers are real buttons that save the part
> 
> Pushed. This is a delightful improvement to our handling of attachments.
> 
> I'd like the text on the button to give some more instruction, such as:
> 
> 	"Click or press Enter to save."
> 
> And if we could pull it off, (I don't know how flexible the button
> widget is in emacs), it would be nice to have something like this as
> well:
> 
> 	"Or press V to view."
> 
> so that one could easily use an external viewer for a single attachment.

Agreed. There are a bunch of enhancements that I'd like to make here,
including toggling the visibility of part, viewing in another buffer,
view externally, force the type, etc.

> Meanwhile, another issue with the result of this series is that I now
> seem to get rendering for both the text/plain and the text/html
> alternatives when a message has both.

This is a consequence of `notmuch show' squashing the MIME details in
the message before output. In particular most messages which include
both text/plain and text/html parts are declared as
multipart/alternative, but the UI never gets to see that detail. Work on
improving that is happening (slowly) at:
	http://github.com/dme/notmuch/tree/multipart
(please don't start using this - the JSON output is mostly-okay, but
text output is broken and the Emacs code is incomplete!)

multipart/alternative suppor would fix this. multipart/related is
necessary for displaying HTML with inline images that are part of the
message ('cid:' images). multipart/signed and multipart/encrypted are
necessary for PGP (and others). If `notmuch' can be persuaded to include
the structure in the JSON output then I think that the Emacs code is in
reasonable shape to be extended to display things well.

> For now, the paragraphs are wrapped much more nicely in the rendering
> of the html portion, but links are apparently entirely missing. The
> link URLs at least appear in the text/plain rendering, (which is
> pretty ugly, but at least not impossible to use).

The HTML formatting depends somewhat on the tools available on your
system. Do you have w3m and w3m-el[1] installed? I'd be curious to see a
screenshot of a message that you think is poorly displayed.

> If we could get one version or the other working completely, then it
> would be nice to display only one.

See above - multipart/alternative support is required.

> > commit e9d737feb5a49fd59e1f27bccd24cac2fd1ef749
> > 
> >     emacs/notmuch-show.el: Add `notmuch-show-toggle-all' bound to M-RET
> >     
> >     `notmuch-show-toggle-all' changes the visibility all of the messages
> >     in the current thread. By default it makes all of the messages not
> >     visible. With a prefix argument, it makes them all visible.
> 
> I didn't push this one yet.
> 
> The feature is *almost* what I want. It's just that I often want to open
> all messages in a thread, (and I've never found myself wanting to close
> all messages). So I'd like to switch which behavior requires the prefix
> argument here.

Should I just re-submit with the sense reversed? After using it for a
few days the opposite approach seems more useful.

> Anyone, if you haven't tried that mode yet for customizing notmuch, you
> should give it a try. (And we should add some hint somewhere to make it
> easier to find. Perhaps a keybinding to get to the customization
> buffer.)

A button in `notmuch-hello' ?

Footnotes: 
[1]  Debian names - presumably similar on other systems.


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

[PATCH] emacs/notmuch-show.el: Add `notmuch-show-toggle-all' bound to M-RET

[David Edmondson]

`notmuch-show-toggle-all' changes the visibility all of the messages
in the current thread. By default it makes all of the messages
visible. With a prefix argument, it makes them all not visible.
---
 emacs/notmuch-show.el |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 916b39e..9775fb4 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -526,6 +526,7 @@ function is used. "
 	(define-key map "p" 'notmuch-show-previous-open-message)
 	(define-key map (kbd "DEL") 'notmuch-show-rewind)
 	(define-key map " " 'notmuch-show-advance-and-archive)
+	(define-key map (kbd "M-RET") 'notmuch-show-toggle-all)
 	(define-key map (kbd "RET") 'notmuch-show-toggle-message)
 	map)
       "Keymap for \"notmuch show\" buffers.")
@@ -900,6 +901,18 @@ to stdout or stderr will appear in the *Messages* buffer."
      (not (plist-get props :message-visible))))
   (force-window-update))
 
+(defun notmuch-show-toggle-all ()
+  "Change the visibility all of the messages in the current
+thread. By default make all of the messages visible. With a
+prefix argument, make them all not visible."
+  (interactive)
+  (save-excursion
+    (goto-char (point-min))
+    (loop do (notmuch-show-message-visible (notmuch-show-get-message-properties)
+					   (not current-prefix-arg))
+	  until (not (notmuch-show-goto-message-next))))
+  (force-window-update))
+
 (defun notmuch-show-next-button ()
   "Advance point to the next button in the buffer."
   (interactive)
-- 
1.7.0

Re: pull request

[Servilio Afre Puentes]

On 21 April 2010 17:03, Carl Worth <cworth@cworth.org> wrote:
> On Mon, 19 Apr 2010 09:27:39 +0100, David Edmondson <dme@dme.org> wrote:
[...]
> Meanwhile, another issue with the result of this series is that I now
> seem to get rendering for both the text/plain and the text/html
> alternatives when a message has both. For now, the paragraphs are
> wrapped much more nicely in the rendering of the html portion, but links
> are apparently entirely missing. The link URLs at least appear in the
> text/plain rendering, (which is pretty ugly, but at least not impossible
> to use).
>
> If we could get one version or the other working completely, then it
> would be nice to display only one.

I think that a better approach here would be to list them as parts if
they are present, then have a [configurable] way to show only one by
default, and the other would be available to show in-line.

[...]
>> commit e9d737feb5a49fd59e1f27bccd24cac2fd1ef749
>>
>>     emacs/notmuch-show.el: Add `notmuch-show-toggle-all' bound to M-RET
>>
>>     `notmuch-show-toggle-all' changes the visibility all of the messages
>>     in the current thread. By default it makes all of the messages not
>>     visible. With a prefix argument, it makes them all visible.
>
> I didn't push this one yet.
>
> The feature is *almost* what I want. It's just that I often want to open
> all messages in a thread, (and I've never found myself wanting to close
> all messages). So I'd like to switch which behavior requires the prefix
> argument here.

What I miss in this view sometimes is the possibility of being able to
see the structure of the thread. A way to toggle the expanded state of
the messages originally expanded when I first opened the view would do
this very nicely.

[...]
> [Aside: We could improve our help screen by making RET on any line then
> bring up the full help message for that function.]

+1

>> commit b3be927b54956a7258f203159e0bdb954e686c80
>>
>>     emacs: Support for customizing search result display
>
> Finally, this is pushed. It's a quite lovely feature that's quite easy
> to take advantage of from "M-x customize-group" "notmuch".
>
> Anyone, if you haven't tried that mode yet for customizing notmuch, you
> should give it a try. (And we should add some hint somewhere to make it
> easier to find. Perhaps a keybinding to get to the customization
> buffer.)

The help screen is a very good place to place a link to the
customization screen.

Servilio

Re: pull request

[David Edmondson]

On Thu, 22 Apr 2010 04:58:16 -0400, Servilio Afre Puentes <servilio@gmail.com> wrote:
> On 21 April 2010 17:03, Carl Worth <cworth@cworth.org> wrote:
> > On Mon, 19 Apr 2010 09:27:39 +0100, David Edmondson <dme@dme.org> wrote:
> [...]
> > Meanwhile, another issue with the result of this series is that I now
> > seem to get rendering for both the text/plain and the text/html
> > alternatives when a message has both. For now, the paragraphs are
> > wrapped much more nicely in the rendering of the html portion, but links
> > are apparently entirely missing. The link URLs at least appear in the
> > text/plain rendering, (which is pretty ugly, but at least not impossible
> > to use).
> >
> > If we could get one version or the other working completely, then it
> > would be nice to display only one.
> 
> I think that a better approach here would be to list them as parts if
> they are present, then have a [configurable] way to show only one by
> default, and the other would be available to show in-line.

Showing only one (with a variable allowing you to express preference) is
my intention. Any non-shown parts will appear as attachments - you can
save them using the button (and perhaps later view them).

This can make quite a big difference in a 200 message thread with lots
of 'text/plain or text/html ?' choices - using the text/plain part will
improve the performance of building the show buffer significantly.

> What I miss in this view sometimes is the possibility of being able to
> see the structure of the thread. A way to toggle the expanded state of
> the messages originally expanded when I first opened the view would do
> this very nicely.

Ah, so you want a "go back to how it was initially" command?

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

Re: pull request

[Servilio Afre Puentes]

On 22 April 2010 08:30, David Edmondson <dme@dme.org> wrote:
> On Thu, 22 Apr 2010 04:58:16 -0400, Servilio Afre Puentes <servilio@gmail.com> wrote:
>> On 21 April 2010 17:03, Carl Worth <cworth@cworth.org> wrote:
>> > On Mon, 19 Apr 2010 09:27:39 +0100, David Edmondson <dme@dme.org> wrote:
>> [...]
>> > Meanwhile, another issue with the result of this series is that I now
>> > seem to get rendering for both the text/plain and the text/html
>> > alternatives when a message has both. For now, the paragraphs are
>> > wrapped much more nicely in the rendering of the html portion, but links
>> > are apparently entirely missing. The link URLs at least appear in the
>> > text/plain rendering, (which is pretty ugly, but at least not impossible
>> > to use).
>> >
>> > If we could get one version or the other working completely, then it
>> > would be nice to display only one.
>>
>> I think that a better approach here would be to list them as parts if
>> they are present, then have a [configurable] way to show only one by
>> default, and the other would be available to show in-line.
>
> Showing only one (with a variable allowing you to express preference) is
> my intention. Any non-shown parts will appear as attachments - you can
> save them using the button (and perhaps later view them).

Why not see them in-line if possible?

> This can make quite a big difference in a 200 message thread with lots
> of 'text/plain or text/html ?' choices - using the text/plain part will
> improve the performance of building the show buffer significantly.

Yes, it can make a big difference.

>> What I miss in this view sometimes is the possibility of being able to
>> see the structure of the thread. A way to toggle the expanded state of
>> the messages originally expanded when I first opened the view would do
>> this very nicely.
>
> Ah, so you want a "go back to how it was initially" command?

Yep, then bind it to a key that would alternate between collapsing all
messages and expanding the ones that were originally expanded.

Servilio

Re: pull request

[David Edmondson]

On Thu, 22 Apr 2010 10:10:15 -0400, Servilio Afre Puentes <servilio@gmail.com> wrote:
> > Showing only one (with a variable allowing you to express preference) is
> > my intention. Any non-shown parts will appear as attachments - you can
> > save them using the button (and perhaps later view them).
> 
> Why not see them in-line if possible?

This will probably come later, sure. At the moment the entire buffer is
built in one pass before you see it. If we want to support the insertion
of parts 'on demand' then a little more work is required.

> >> What I miss in this view sometimes is the possibility of being able to
> >> see the structure of the thread. A way to toggle the expanded state of
> >> the messages originally expanded when I first opened the view would do
> >> this very nicely.
> >
> > Ah, so you want a "go back to how it was initially" command?
> 
> Yep, then bind it to a key that would alternate between collapsing all
> messages and expanding the ones that were originally expanded.

I'll give this some thought.

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

Re: pull request

[David Edmondson]

On Wed, 21 Apr 2010 15:47:12 -0400, Jameson Rollins <jrollins@finestructure.net> wrote:
> On Wed, 21 Apr 2010 12:15:48 -0700, Carl Worth <cworth@cworth.org> wrote:
> > [*] Even more simplification is possible if we stop trying to hide
> > header components. Several people have requested that To and Cc be
> > visible all the time.
> 
> Hey, Carl.  I had actually been meaning to bring this up.  I actually
> don't like having the headers collapsed at all, so I am definitely in
> favor of the idea of doing away with this "feature".  At the very least,
> I would like to see header hiding/collapsing be an option, so that I
> could turn it off.  As is, I'm having to hit 'h' for every message I
> view, so I can just see who the message was sent to (not to mention to
> recover the unfortunately missing blank line between the header and the
> body, which is visually useful).

See id:1272023661-14038-1-git-send-email-dme@dme.org.

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

Re: pull request

[Jameson Rollins]

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

On Fri, 23 Apr 2010 12:57:42 +0100, David Edmondson <dme@dme.org> wrote:
> > Hey, Carl.  I had actually been meaning to bring this up.  I actually
> > don't like having the headers collapsed at all, so I am definitely in
> > favor of the idea of doing away with this "feature".  At the very least,
> > I would like to see header hiding/collapsing be an option, so that I
> > could turn it off.  As is, I'm having to hit 'h' for every message I
> > view, so I can just see who the message was sent to (not to mention to
> > recover the unfortunately missing blank line between the header and the
> > body, which is visually useful).
> 
> See id:1272023661-14038-1-git-send-email-dme@dme.org.

Yes!  +1!  Thank you very much, David.

jamie.

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

Re: [PATCH] emacs/notmuch-show.el: Add `notmuch-show-toggle-all' bound to M-RET

[Carl Worth]

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

On Thu, 22 Apr 2010 09:24:03 +0100, David Edmondson <dme@dme.org> wrote:
> `notmuch-show-toggle-all' changes the visibility all of the messages
> in the current thread. By default it makes all of the messages
> visible. With a prefix argument, it makes them all not visible.

This is a better default, definitely.

The only complaint I have left is that the function name is wrong. It's
not toggling the visibility of all messages, (which would be a totally
insane thing to do of course).

The positive-sense operation would be something like
notmuch-show-open-all. So maybe a function that does both would be
something like notmuch-show-open-or-close-all ?

-Carl

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

Re: pull request

[Carl Worth]

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

On Thu, 22 Apr 2010 07:59:36 +0100, David Edmondson <dme@dme.org> wrote:
> Your concerns appear to be about `notmuch-wash-wrap-long-lines'. I agree
> that it can generate unfortunate results in the "deep thread, narrow
> terminal" case. Are the other two functions okay?

If they had come in as separate patches, I would have reviewed them
separately and would have applied any I was OK with.

I'm not sure about the other two, as I really haven't played with
them. I don't think I'd get a lot of benefit from them as I don't recall
seeing a lot of accidental duplicate lines anywhere.

I still do have some concerns about munging the original message,
though. I can easily imagine that suppressing duplicate blank lines
could destroy some ASCII (or UTF-8!) art for example.

So I think things that do this kind of munging really should be on an
opt-in basis.

> One of the reasons that I chose a hook-based approach is to allow the
> user to decide which body cleaning should be applied. If you don't want
> the line-wrapping to be enabled by default we can leave it out but
> include the definition so that a user can enable it. The inline-patch
> formatting is similar (I'll say more about that in response to your
> comments on it).

I do appreciate the way the hook works such that users can select which
ones they want. Let's make all of these available but any that could
possibly "corrupt" a message be off by default. And let's be sure to
find a way to easily advertise the list of available washing functions
to the user, (perhaps in the documentation of the customize option for
this).

> Aside from the need to test, this particular patch will benefit only
> minimally from testing - there will always be false positive cases
> however good the analysis used. Are you saying that we can not tolerate
> any false-positives, or that we just need to demonstrate that it is down
> to some acceptable level? (Though I've no idea how I would achieve the
> latter.)

I may have overreacted to a misreading of the commit message. I read it
almost like "Here's some code that is known to be buggy" which is
something I don't ever want to accept. If the meaning is instead, "some
users might find this handy while others find it unacceptable", then
that would be more acceptable.

I think it would be useful to define exactly what the implemented
conditions are so that users can easily decide whether they trust the
heuristic.

(Personally, even when correctly identified, the coloring of patch
output is distracting to me.)

The remainder of the discussion about future improvements to mime part
handling and, multipart support, and HTML rendering is all very
encouraging. I look forward to the future!

-Carl

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

Re: [PATCH] emacs/notmuch-show.el: Add `notmuch-show-toggle-all' bound to M-RET

[Carl Worth]

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

On Thu, 22 Apr 2010 09:24:03 +0100, David Edmondson <dme@dme.org> wrote:
> `notmuch-show-toggle-all' changes the visibility all of the messages
> in the current thread. By default it makes all of the messages
> visible. With a prefix argument, it makes them all not visible.

I pushed this now, (with a rename of the function as I mentioned
before).

-Carl

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