Re: bug#49380: 27.1; is mm-inline-message supported outside Gnus?

2021-07-04 Thread David Bremner
Lars Ingebrigtsen  writes:

>
> Yeah, it's confusing.  `mm-inline-message' is really an "internal" Gnus
> function, while most of the other mm functions are more general.
>
> Disentangling that mess is daunting, but for that function in
> particular, I guess it wouldn't be that difficult to rewrite it to avoid
> doing anything Gnus-ish unless we're in Gnus...  On the other hand, it's
> a function to display a complete message, which leaves you with a buffer
> that's pretty pointless unless you have a mail reader to interact with
> it afterwards.

I should have been more precise. I'm calling it indirectly via
mm-display-part, which is useful in the case where someone attaches a
message/rfc822 part to their message. In principle notmuch can (and in
the general case does) render the part directly, but in certain odd
fall-back cases it is handy to give a visual representation of the part
in the buffer without parsing etc.. ourselves.  So the user does have a
mail reader, namely notmuch. In fact I can almost make it work by
forcing the buffer back into notmuch-show-mode after calling
mm-display-part, but that has some side-effects I'd prefer to avoid.

Thanks for thinking about my (weird?) use-cases,

David





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


Re: bug#49380: 27.1; is mm-inline-message supported outside Gnus?

2021-07-04 Thread Lars Ingebrigtsen
David Bremner  writes:

> notmuch-show-mode relies on various mm-* functions to display message
> parts. This is problematic for "message/rfc822" parts, since
> mm-inline-message ends up calling (gnus-article-prepare-display) which
> resets the buffer major-mode to gnus-article-mode since
> notmuch-show-mode is not a derived mode of gnus-article-mode. This
> breaks a few things from a notmuch internals / user point of view.  I
> can forcibly reset the major-mode, but that has it's own problems. I
> would welcome suggestions for workarounds, or clarification of the
> extent to which the functions in mm-view.el are expected to be usable
> outside gnus.

Yeah, it's confusing.  `mm-inline-message' is really an "internal" Gnus
function, while most of the other mm functions are more general.

Disentangling that mess is daunting, but for that function in
particular, I guess it wouldn't be that difficult to rewrite it to avoid
doing anything Gnus-ish unless we're in Gnus...  On the other hand, it's
a function to display a complete message, which leaves you with a buffer
that's pretty pointless unless you have a mail reader to interact with
it afterwards.

So perhaps it should just be renamed and moved to Gnus?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


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

2021-07-04 Thread David Bremner
Hannu Hartikainen  writes:

> 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.

This is not really directed at Hannu, but at the notmuch community. As
you can imagine I'm not super enthusiastic an every growing number of
output formats to maintain.

The usual argument for keeping --format=text is that it useful for
scripting. There is also the vim UI, but afaict that is no longer using
--format=text.  It would be nice if a new text like format could
(eventually) replace the old one. So what would the new format need to
do so that we could at least deprecate the old one?

One thing the old format did not do, but a generically useful on the
command-line format probably should is deal with signature verification
and decryption. There is obviously potential for visual spoofing, but
maybe color can help.

>
> 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`.
> ---

In my experience, notmuch show --format=raw works pretty well
for this. There was an issue with encoded line endings but that is fixed
in git 2.32. What advantage does this new format bring for patches?

d


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


Re: [PATCH 2/2] test: known broken test for emacs display of message/rfc822 parts

2021-07-04 Thread David Bremner
Tomi Ollila  writes:

> In this case one could just do 
>
> test_expect_code 1 "fgrep -q '!!!' OUTPUT" 
>
> (these file comparisons w/ /dev/null (currently 5) annoys me ;/
> so if I am biased the current suggestion is fine)

Sure, I've adopted your suggestion in git

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


[PATCH 1/3] emacs: forcibly set notmuch-show-mode after delegating inlining

2021-07-04 Thread David Bremner
Some mm-* functions end up calling (gnus-article-mode), which breaks
notmuch.
---
 emacs/notmuch-lib.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index c7bb2091..8ca416d4 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -694,6 +694,8 @@ current buffer, if possible."
  (when (mm-inlinable-p handle)
(set-buffer display-buffer)
(mm-display-part handle)
+   ;; restore major mode if changed by gnus
+   (notmuch-show-mode)
t))
 
 ;;; Generic Utilities
-- 
2.30.2
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH 2/3] emacs: don't inline message/rfc822 parts without content

2021-07-04 Thread David Bremner
This avoids some ugly error messages and exceptions, and hopes that
some gnus method will display message/rfc822 parts that have only a
file, no :content part.
---
 emacs/notmuch-show.el | 32 +---
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 9a95eb34..7695e869 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -718,21 +718,23 @@ will return nil if the CID is unknown or cannot be 
retrieved."
   t)
 
 (defun notmuch-show-insert-part-message/rfc822 (msg part _content-type _nth 
depth _button)
-  (let* ((message (car (plist-get part :content)))
-(body (car (plist-get message :body)))
-(start (point)))
-;; Override `notmuch-message-headers' to force `From' to be
-;; displayed.
-(let ((notmuch-message-headers '("From" "Subject" "To" "Cc" "Date")))
-  (notmuch-show-insert-headers (plist-get message :headers)))
-;; Blank line after headers to be compatible with the normal
-;; message display.
-(insert "\n")
-;; Show the body
-(notmuch-show-insert-bodypart msg body depth)
-(when notmuch-show-indent-multipart
-  (indent-rigidly start (point) 1)))
-  t)
+  (let ((message (car (plist-get part :content
+(and
+ message
+ (let ((body (car (plist-get message :body)))
+  (start (point)))
+   ;; Override `notmuch-message-headers' to force `From' to be
+   ;; displayed.
+   (let ((notmuch-message-headers '("From" "Subject" "To" "Cc" "Date")))
+(notmuch-show-insert-headers (plist-get message :headers)))
+   ;; Blank line after headers to be compatible with the normal
+   ;; message display.
+   (insert "\n")
+   ;; Show the body
+   (notmuch-show-insert-bodypart msg body depth)
+   (when notmuch-show-indent-multipart
+(indent-rigidly start (point) 1))
+   t
 
 (defun notmuch-show-insert-part-text/plain (msg part _content-type _nth depth 
button)
   ;; For backward compatibility we want to apply the text/plain hook
-- 
2.30.2
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


WIP fixes for rfc822/attachement display

2021-07-04 Thread David Bremner
This is not finished/correct yet, but maybe it will save someone else
some debugging.

[PATCH 1/3] emacs: forcibly set notmuch-show-mode after delegating

- In particular this resets some variables to default values which
  breaks the indentation test.

[PATCH 2/3] emacs: don't inline message/rfc822 parts without content

- of the three patches, this seems like the most sane. The content
  isn't there, so either the function needs to be rewritten to handle
  re-parsing the blob, or just fail gracefully

[PATCH 3/3] emacs: dynamically bind gnus-newsgroup-charset

- this is very much papering over the symptoms
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org


[PATCH 3/3] emacs: dynamically bind gnus-newsgroup-charset

2021-07-04 Thread David Bremner
This avoids requiring gnus-sum, which is currently where this variable
is defined.
---
 emacs/notmuch-lib.el| 3 ++-
 test/T450-emacs-show.sh | 1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 8ca416d4..37fb4018 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -693,7 +693,8 @@ current buffer, if possible."
(insert (notmuch-get-bodypart-binary msg part process-crypto)))
  (when (mm-inlinable-p handle)
(set-buffer display-buffer)
-   (mm-display-part handle)
+   (notmuch-dlet ((gnus-newsgroup-charset nil))
+ (mm-display-part handle))
;; restore major mode if changed by gnus
(notmuch-show-mode)
t))
diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
index 75a52640..7a731ac7 100755
--- a/test/T450-emacs-show.sh
+++ b/test/T450-emacs-show.sh
@@ -220,7 +220,6 @@ test_emacs '(notmuch-show 
"id:basic-encryp...@crypto.notmuchmail.org")
 test_expect_equal_file $EXPECTED/notmuch-show-decrypted-message OUTPUT
 
 test_begin_subtest "show encrypted rfc822 message"
-test_subtest_known_broken
 test_emacs '(notmuch-show 
"id:encrypted-rfc822-attachm...@crypto.notmuchmail.org")
 (test-visible-output)'
 count=$(fgrep -c '!!!' OUTPUT)
-- 
2.30.2
___
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 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 

Re: [PATCH 2/2] test: known broken test for emacs display of message/rfc822 parts

2021-07-04 Thread Tomi Ollila
On Sat, Jul 03 2021, David Bremner wrote:

> There are at least 3 bugs present.
>
> 1) notmuch-show-insert-part-message/rfc822 assumes that message/rfc822
> parts will have a ":content" property, but that turns out not to be
> the case.
>
> 2) something deep in gnus wants gnus-newsgroup-charset, but that is
> defined in gnus-sum, which is not loaded by default.
>
> 3) If gnus-sum is loaded, then the display of the message/rfc822 part
> succeeds, but the buffer gets put into gnus-article-mode, which means
> that, inter alia, notmuch text properties and keybindings get wiped.
> ---
>  test/T450-emacs-show.sh | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
> index 9d08d2cc..bf7c1fe0 100755
> --- a/test/T450-emacs-show.sh
> +++ b/test/T450-emacs-show.sh
> @@ -219,6 +219,13 @@ test_emacs '(notmuch-show 
> "id:basic-encryp...@crypto.notmuchmail.org")
>  (test-visible-output)'
>  test_expect_equal_file $EXPECTED/notmuch-show-decrypted-message OUTPUT
>  
> +test_begin_subtest "show encrypted rfc822 message"
> +test_subtest_known_broken
> +test_emacs '(notmuch-show 
> "id:encrypted-rfc822-attachm...@crypto.notmuchmail.org")
> +(test-visible-output)'
> +fgrep '!!!' OUTPUT > OUTPUT.err
> +test_expect_equal_file /dev/null OUTPUT.err

In this case one could just do 

test_expect_code 1 "fgrep -q '!!!' OUTPUT" 

(these file comparisons w/ /dev/null (currently 5) annoys me ;/
so if I am biased the current suggestion is fine)

Tomi

> +
>  test_begin_subtest "show undecryptable message"
>  test_emacs '(notmuch-show "id:simple-encryp...@crypto.notmuchmail.org")
>  (test-visible-output)'
> -- 
> 2.30.2
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org