emacs: Split wide window vertically for tree mode message pane

emacs: Split wide window vertically for tree mode message pane

[Prof Jayanth R Varma]

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

This patch modifies the function notmuch-tree-show-message-in in 
notmuch-tree.el to split the window vertically while creating a 
message pane in tree-mode if the window is wider than 160 (so that 
after splitting, the message pane can show the standard 76 
character plain text email lines without wrapping).

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Split-wide-window-vertically-for-tree-mode-message-p.patch --]
[-- Type: text/x-patch, Size: 1807 bytes --]

From e4829e484ffe3aab564f112d677fb72e9a400144 Mon Sep 17 00:00:00 2001
From: Jayanth Varma <jrvarma@gmail.com>
Date: Wed, 3 Jan 2018 14:44:31 +0530
Subject: [PATCH] Split wide window vertically for tree-mode message pane in
 emacs

---
 NEWS                  | 4 ++++
 emacs/notmuch-tree.el | 9 ++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 47b62d73..d2808df1 100644
--- a/NEWS
+++ b/NEWS
@@ -151,6 +151,10 @@ Extend mitigation (disabling handling x-display in text/enriched) for
 Emacs bug #28350 to Emacs versions before 24.4 (i.e. without
 `advice-add`).
 
+When opening message pane in tree-mode, split window vertically if
+window is wider than 160 (message pane after splitting is wider than
+80).
+
 Command Line Interface
 ----------------------
 
diff --git a/emacs/notmuch-tree.el b/emacs/notmuch-tree.el
index c00315e8..082bcfa5 100644
--- a/emacs/notmuch-tree.el
+++ b/emacs/notmuch-tree.el
@@ -469,7 +469,14 @@ NOT change the database."
       ;; this might cause flickering but seems ok.
       (notmuch-tree-close-message-window)
       (setq notmuch-tree-message-window
-	    (split-window-vertically (/ (window-height) 4)))
+	    ;; Split message pane vertically in wide windows (width > 160)
+	    ;; Under RFC 2045 line length is 76 characters so plain text emails
+	    ;; will be displayed properly in 80 character wide pane
+	    ;; HTML mail will anyway reformat to pane width
+	    ;; In narrow windows, split pane horizontally 1:3
+	    (if (> (window-total-width) 160)
+		(split-window-horizontally)
+	      (split-window-vertically (/ (window-height) 4))))
       (with-selected-window notmuch-tree-message-window
 	;; Since we are only displaying one message do not indent.
 	(let ((notmuch-show-indent-messages-width 0)
-- 
2.15.1

Re: emacs: Split wide window vertically for tree mode message pane

[David Bremner]

Prof Jayanth R Varma <jrvarma@gmail.com> writes:

> This patch modifies the function notmuch-tree-show-message-in in 
> notmuch-tree.el to split the window vertically while creating a 
> message pane in tree-mode if the window is wider than 160 (so that 
> after splitting, the message pane can show the standard 76 
> character plain text email lines without wrapping).

First, I'm sorry to take so long to review this.  A few comments.

1) I'm reluctant to add new features to notmuch without corresponding
   tests. Currently all of our emacs tests use a standard terminal width
   (I think 80). Perhaps Tomi knows how hard it would be to override for
   a single test.

2) I noticed that the horizontal splitting leads to truncation of tag
   display. So I think not everyone will want this (it would be pretty
   frustrating to widen the emacs frame to see a full tag list, just to
   have it truncated). One option would be to control the splitting by a
   variable specifying the minimum width at which to switch to horizonal
   splitting.

3) The commit message / NEWS talks about splitting the window
   vertically, but that seems contrary to the emacs terminology (as
   evinced by the source code in the patch)

Re: emacs: Split wide window vertically for tree mode message pane

[Tomi Ollila]

On Wed, May 09 2018, David Bremner wrote:

> Prof Jayanth R Varma <jrvarma@gmail.com> writes:
>
>> This patch modifies the function notmuch-tree-show-message-in in 
>> notmuch-tree.el to split the window vertically while creating a 
>> message pane in tree-mode if the window is wider than 160 (so that 
>> after splitting, the message pane can show the standard 76 
>> character plain text email lines without wrapping).
>
> First, I'm sorry to take so long to review this.  A few comments.
>
> 1) I'm reluctant to add new features to notmuch without corresponding
>    tests. Currently all of our emacs tests use a standard terminal width
>    (I think 80). Perhaps Tomi knows how hard it would be to override for
>    a single test.

That would be pretty simple; we'd add variable for colums to be set before
test... buf for this particular case I don't know how there could be any
sensible test done 

> 2) I noticed that the horizontal splitting leads to truncation of tag
>    display. So I think not everyone will want this (it would be pretty
>    frustrating to widen the emacs frame to see a full tag list, just to
>    have it truncated). One option would be to control the splitting by a
>    variable specifying the minimum width at which to switch to horizonal
>    splitting.

I did some (outside of notmuch) experimentation

in emacs 25.3 

truncate-partial-width-windows is a variable defined in ‘C source code’.
Its value is 50

when I split 80-char emacs window horizontally, lines were truncated, but
when widening it enough (presumably over 101) the lines (started to) wrap
(again). Did this truncation of tag display happened (anyway?)

> 3) The commit message / NEWS talks about splitting the window
>    vertically, but that seems contrary to the emacs terminology (as
>    evinced by the source code in the patch)

That's bad ;/

Tomi

Re: emacs: Split wide window vertically for tree mode message pane

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Wed, May 09 2018, David Bremner wrote:
>
>> Prof Jayanth R Varma <jrvarma@gmail.com> writes:
>>
>>> This patch modifies the function notmuch-tree-show-message-in in 
>>> notmuch-tree.el to split the window vertically while creating a 
>>> message pane in tree-mode if the window is wider than 160 (so that 
>>> after splitting, the message pane can show the standard 76 
>>> character plain text email lines without wrapping).
>>
>> First, I'm sorry to take so long to review this.  A few comments.
>>
>> 1) I'm reluctant to add new features to notmuch without corresponding
>>    tests. Currently all of our emacs tests use a standard terminal width
>>    (I think 80). Perhaps Tomi knows how hard it would be to override for
>>    a single test.

>
> That would be pretty simple; we'd add variable for colums to be set before
> test... buf for this particular case I don't know how there could be any
> sensible test done 

I was thinking basically the tests with output in
emacs-tree.expected-output, with the other split.

d

Re: emacs: Split wide window vertically for tree mode message pane

[Prof Jayanth R Varma]

On Wed, May 09 2018, David Bremner wrote:

> Prof Jayanth R Varma <jrvarma@gmail.com> writes:

>> This patch modifies the function notmuch-tree-show-message-in 
>> in notmuch-tree.el to split the window vertically while 
>> creating a message pane in tree-mode if the window is wider 
>> than 160 (so that after splitting, the message pane can show 
>> the standard 76 character plain text email lines without 
>> wrapping).

> 2) I noticed that the horizontal splitting leads to truncation 
> of tag display. So I think not everyone will want this (it would 
> be pretty frustrating to widen the emacs frame to see a full tag 
> list, just to have it truncated). One option would be to control 
> the splitting by a variable specifying the minimum width at 
> which to switch to horizonal splitting.

I agree that the customization option is superior to my original 
patch. The default value of the customization variable could be 
set equal to the emacs variable "most-positive-fixnum" (the 
largest value that is representable in a Lisp integer) so that the 
patch would not have an impact for anybody who does not explicitly 
customize it.

> 3) The commit message / NEWS talks about splitting the window 
> vertically, but that seems contrary to the emacs terminology (as 
> evinced by the source code in the patch)

My mistake: vertically should be changed to horizontally in the 
commit message, NEWS and source code comments.