Re: [PATCH v2] cli/show: add --format=pretty

2021-07-04 Thread Hannu Hartikainen
Thanks for the review!

On Sun, 04 Jul 2021 12:40:59 +0300, Tomi Ollila  wrote:
> The code looks good to me, just that these "hardcoded" color values gives
> me a bit of suspicion...

That's true and I do think the colors could eventually be configurable,
but right now I don't know how the configuration should work. Should it
be a palette, should each header have a configurable color (and should
the displayed headers be configurable too) etc. It's easy to make this
hugely bloated so I'll avoid going there until I have a clear idea of a
good, minimal approach.

> the colors look OK when background is dark (black). on light background
> (white) color 33 (yellow), color 36 (cyan) and color 32 (green) are
> somewhat hard to read (in decreasing hardness)...

Most terminals have user-configurable colors so I don't think this is a
showstopper. But as I said, yes, it would be best to make the colors
configurable.

> I did not try to apply this and see how those looks like when displaying
> real emails (no setup on this machine for now...)

Here's a screenshot: https://i.imgur.com/7dlP7lt.png

I'm happy to receive any suggestions but I think it's best to stay with
the original 3-bit spec (SGR codes 30-37, or in practice 31-36: red,
green, yellow, blue, magenta, cyan) if the values are hardcoded. Those
are most widely supported.

> One option would be to first send this without color support and then
> add *configurable* color support -- I don't know which way is better
> but as a rewiever that would be easier to accept...

That's an excellent idea! I'll post PATCH v3 later. I don't really want
to try real feature detection because some terminals have weird corner
cases with their ANSI sequence support. `isatty` is what other CLI tools
tend to use as default AFAIK.

Hannu
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


Re: [PATCH v2] cli/show: add --format=pretty

2021-07-04 Thread Tomi Ollila
On Fri, Jul 02 2021, Hannu Hartikainen wrote:

> The rationale for this feature is twofold:
>
> 1. It is useful to be able to view messages in as human-friendly format
>as possible.
> 2. The same format should still be machine-readable, too.
>
> The email format is mostly human-readable as is. The things difficult
> for a human eye are the huge amount of headers that are common these
> days and telling different messages apart when there are many.
>
> This commit adds the display format `pretty`. It recognizes if the
> output is a TTY and if so, applies coloring to headers. This turns out
> to be a good visual message separator. Additionally it only shows the
> most important headers, similarly to `--format=text`, and only displays
> plaintext parts (ie. text/* but not text/html).
>
> While human readability is the main goal, another design goal was that
> piping the output to `git am` works, at least for individual messages
> sent with `git send-email`.
> ---
>
> I wrote a v2 of this patch. I've been dogfooding for a while now and
> wanted a couple of enhancements, and also had learned about the notmuch
> test harness. The differences to the first version are:
>
> - add a unit test
> - show Message-ID, making replying etc. much easier
> - print a newline after each part, which helps a lot with messages that
>   do not end in a newline
>
> I'm using this as a daily driver and am happy with it.
>
>  NEWS   |  7 
>  completion/notmuch-completion.bash |  2 +-
>  completion/zsh/_notmuch|  2 +-
>  doc/man1/notmuch-show.rst  |  8 +++-
>  notmuch-show.c | 67 ++
>  test/T520-show.sh  | 32 ++
>  6 files changed, 115 insertions(+), 3 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 3e776009..f5142ff1 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,6 +1,13 @@
>  Notmuch 0.33 (UNRELEASED)
>  =
>  
> +CLI
> +---
> +
> +`notmuch show` now has `--format=pretty`, optimized for reading plain
> +text emails on the command line. It only shows the most important
> +headers and plain text parts and uses colors for headers.
> +
>  Emacs
>  -
>  
> diff --git a/completion/notmuch-completion.bash 
> b/completion/notmuch-completion.bash
> index 15425697..86cbbcdc 100644
> --- a/completion/notmuch-completion.bash
> +++ b/completion/notmuch-completion.bash
> @@ -514,7 +514,7 @@ _notmuch_show()
>   return
>   ;;
>   --format)
> - COMPREPLY=( $( compgen -W "text json sexp mbox raw" -- "${cur}" ) )
> + COMPREPLY=( $( compgen -W "text pretty json sexp mbox raw" -- 
> "${cur}" ) )
>   return
>   ;;
>   --exclude|--body)
> diff --git a/completion/zsh/_notmuch b/completion/zsh/_notmuch
> index e920f10b..5cc386e2 100644
> --- a/completion/zsh/_notmuch
> +++ b/completion/zsh/_notmuch
> @@ -237,7 +237,7 @@ _notmuch_search() {
>  _notmuch_show() {
>_arguments -S \
>  '--entire-thread=[output entire threads]:show thread:(true false)' \
> -'--format=[set output format]:output format:(text json sexp mbox raw)' \
> +'--format=[set output format]:output format:(text pretty json sexp mbox 
> raw)' \
>  '--format-version=[set output format version]:format version: ' \
>  '--part=[output a single decoded mime part]:part number: ' \
>  '--verify[verify signed MIME parts]' \
> diff --git a/doc/man1/notmuch-show.rst b/doc/man1/notmuch-show.rst
> index fc6bec62..1fe4dcc7 100644
> --- a/doc/man1/notmuch-show.rst
> +++ b/doc/man1/notmuch-show.rst
> @@ -34,7 +34,7 @@ Supported options for **show** include
> the matching messages. For ``--format=json`` and ``--format=sexp``
> this defaults to true. For other formats, this defaults to false.
>  
> -.. option:: --format=(text|json|sexp|mbox|raw)
> +.. option:: --format=(text|pretty|json|sexp|mbox|raw)
>  
> **text** (default for messages)
>   The default plain-text format has all text-content MIME parts
> @@ -46,6 +46,12 @@ Supported options for **show** include
>   '}'), to either open or close the component. For a multipart
>   MIME message, these parts will be nested.
>  
> +   **pretty**
> + The plain-text parts of all matching messages are printed in a
> + format optimized for readability. Only the most important
> + headers are displayed. If the output is to a TTY, the headers
> + are colored.
> +
> **json**
>   The output is formatted with Javascript Object Notation
>   (JSON). This format is more robust than the text format for
> diff --git a/notmuch-show.c b/notmuch-show.c
> index 232557d5..c417ec00 100644
> --- a/notmuch-show.c
> +++ b/notmuch-show.c
> @@ -606,6 +606,65 @@ format_part_text (const void *ctx, sprinter_t *sp, 
> mime_node_t *node,
>  return NOTMUCH_STATUS_SUCCESS;
>  }
>  
> +static notmuch_status_t
> +format_part_pretty (const void *ctx, sprinter_t *sp, mime_node_t *node,
> + int