[PATCH v4 0/5] notmuch-emacs-mua updates

[PATCH v4 0/5] notmuch-emacs-mua updates

[Jani Nikula]

The main goal is to not force --create-frame on users who dislike
it. Split to some hopefully less controversial prep patches.

BR,
Jani.


Jani Nikula (5):
  man: clean up notmuch-emacs-mua man page
  notmuch-emacs-mua: support --no-window-system also for non-client
  notmuch-emacs-mua: move --client option handling around
  notmuch-emacs-mua: add --auto-daemon option to start Emacs daemon as
    needed
  notmuch-emacs-mua: do not create a frame by default with --client

 doc/man1/notmuch-emacs-mua.rst | 21 ++++++++++++++++-----
 notmuch-emacs-mua              | 30 +++++++++++++++++++++---------
 2 files changed, 37 insertions(+), 14 deletions(-)

-- 
2.1.4

[PATCH v4 1/5] man: clean up notmuch-emacs-mua man page

[Jani Nikula]

Move --client description to a more suitable location, stylistic
changes.
---
 doc/man1/notmuch-emacs-mua.rst | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/doc/man1/notmuch-emacs-mua.rst b/doc/man1/notmuch-emacs-mua.rst
index 36b51cdc3b18..7c2f5c24f1c0 100644
--- a/doc/man1/notmuch-emacs-mua.rst
+++ b/doc/man1/notmuch-emacs-mua.rst
@@ -18,10 +18,6 @@ Supported options for **notmuch-emacs-mua** include
     ``-h, --help``
         Display help.
 
-    ``--client``
-        Use emacsclient, rather than emacs. This will start
-        an emacs daemon process if necessary.
-
     ``-s, --subject=``\ <subject>
         Specify the subject of the message.
 
@@ -38,7 +34,11 @@ Supported options for **notmuch-emacs-mua** include
         Specify a file to include into the body of the message.
 
     ``--no-window-system``
-        Even if a window system is available, use the current terminal
+        Even if a window system is available, use the current terminal.
+
+    ``--client``
+        Use **emacsclient**, rather than **emacs**. This will start
+        an Emacs daemon process if necessary.
 
     ``--print``
         Output the resulting elisp to stdout instead of evaluating it.
-- 
2.1.4

[PATCH v4 2/5] notmuch-emacs-mua: support --no-window-system also for non-client

[Jani Nikula]

Clearly --no-window-system should be applicable without --client as
well. Prepare for future changes with the naming of the variables.
---
 notmuch-emacs-mua | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/notmuch-emacs-mua b/notmuch-emacs-mua
index 79714305b3e2..e33aa955d6b5 100755
--- a/notmuch-emacs-mua
+++ b/notmuch-emacs-mua
@@ -34,8 +34,9 @@ EMACS=${EMACS-emacs}
 EMACSCLIENT=${EMACSCLIENT-emacsclient}
 
 PRINT_ONLY=
+NO_WINDOW=
 USE_EMACSCLIENT=
-CLIENT_TYPE="-c"
+CREATE_FRAME="-c"
 
 # The crux of it all: construct an elisp progn and eval it.
 ELISP="(prog1 'done (require 'notmuch) (notmuch-mua-new-mail)"
@@ -103,7 +104,8 @@ while getopts :s:c:b:i:h opt; do
 	    PRINT_ONLY=1
 	    ;;
 	--no-window-system)
-	    CLIENT_TYPE="-t"
+	    NO_WINDOW="-nw"
+	    CREATE_FRAME=
 	    ;;
 	*)
 	    # We should never end up here.
@@ -132,7 +134,7 @@ fi
 
 if [ -n "$USE_EMACSCLIENT" ]; then
     # Evaluate the progn.
-    exec ${EMACSCLIENT} ${CLIENT_TYPE} -a '' --eval "${ELISP}"
+    exec ${EMACSCLIENT} ${NO_WINDOW} ${CREATE_FRAME} -a '' --eval "${ELISP}"
 else
-    exec ${EMACS} --eval "${ELISP}"
+    exec ${EMACS} ${NO_WINDOW} --eval "${ELISP}"
 fi
-- 
2.1.4

[PATCH v4 3/5] notmuch-emacs-mua: move --client option handling around

[Jani Nikula]

Move --client handling to a more suitable location. No functional
changes.
---
 notmuch-emacs-mua | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/notmuch-emacs-mua b/notmuch-emacs-mua
index e33aa955d6b5..1f088c42feb9 100755
--- a/notmuch-emacs-mua
+++ b/notmuch-emacs-mua
@@ -82,9 +82,6 @@ while getopts :s:c:b:i:h opt; do
 	--help|h)
 	    exec man notmuch-emacs-mua
 	    ;;
-	--client)
-	    USE_EMACSCLIENT="yes"
-	    ;;
 	--subject|s)
 	    ELISP="${ELISP} (message-goto-subject) (insert \"${OPTARG}\")"
 	    ;;
@@ -107,6 +104,9 @@ while getopts :s:c:b:i:h opt; do
 	    NO_WINDOW="-nw"
 	    CREATE_FRAME=
 	    ;;
+	--client)
+	    USE_EMACSCLIENT="yes"
+	    ;;
 	*)
 	    # We should never end up here.
 	    echo "$0: internal error (option ${opt})." >&2
-- 
2.1.4

[PATCH v4 4/5] notmuch-emacs-mua: add --auto-daemon option to start Emacs daemon as needed

[Jani Nikula]

Automatically starting Emacs in daemon mode if the Emacs server is not
running is a matter of preference better not hard coded in
notmuch-emacs-mua. Add an option to control the behaviour.
---
 doc/man1/notmuch-emacs-mua.rst | 9 +++++++--
 notmuch-emacs-mua              | 8 ++++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/doc/man1/notmuch-emacs-mua.rst b/doc/man1/notmuch-emacs-mua.rst
index 7c2f5c24f1c0..89a2fead17b1 100644
--- a/doc/man1/notmuch-emacs-mua.rst
+++ b/doc/man1/notmuch-emacs-mua.rst
@@ -37,8 +37,13 @@ Supported options for **notmuch-emacs-mua** include
         Even if a window system is available, use the current terminal.
 
     ``--client``
-        Use **emacsclient**, rather than **emacs**. This will start
-        an Emacs daemon process if necessary.
+        Use **emacsclient**, rather than **emacs**. For
+        **emacsclient** to work, you need an already running Emacs
+        with a server, or use ``--auto-daemon``.
+
+    ``--auto-daemon``
+        Automatically start Emacs in daemon mode, if the Emacs server
+        is not running. Applicable with ``--client``.
 
     ``--print``
         Output the resulting elisp to stdout instead of evaluating it.
diff --git a/notmuch-emacs-mua b/notmuch-emacs-mua
index 1f088c42feb9..ac03a4a60d56 100755
--- a/notmuch-emacs-mua
+++ b/notmuch-emacs-mua
@@ -36,6 +36,7 @@ EMACSCLIENT=${EMACSCLIENT-emacsclient}
 PRINT_ONLY=
 NO_WINDOW=
 USE_EMACSCLIENT=
+AUTO_DAEMON=
 CREATE_FRAME="-c"
 
 # The crux of it all: construct an elisp progn and eval it.
@@ -64,7 +65,7 @@ while getopts :s:c:b:i:h opt; do
 		    opt=${opt%%=*}
 		    ;;
 		# Long options without arguments.
-		--help|--print|--no-window-system|--client)
+		--help|--print|--no-window-system|--client|--auto-daemon)
 		    ;;
 		*)
 		    echo "$0: unknown long option ${opt}, or argument mismatch." >&2
@@ -107,6 +108,9 @@ while getopts :s:c:b:i:h opt; do
 	--client)
 	    USE_EMACSCLIENT="yes"
 	    ;;
+	--auto-daemon)
+	    AUTO_DAEMON="--alternate-editor="
+	    ;;
 	*)
 	    # We should never end up here.
 	    echo "$0: internal error (option ${opt})." >&2
@@ -134,7 +138,7 @@ fi
 
 if [ -n "$USE_EMACSCLIENT" ]; then
     # Evaluate the progn.
-    exec ${EMACSCLIENT} ${NO_WINDOW} ${CREATE_FRAME} -a '' --eval "${ELISP}"
+    exec ${EMACSCLIENT} ${NO_WINDOW} ${CREATE_FRAME} ${AUTO_DAEMON} --eval "${ELISP}"
 else
     exec ${EMACS} ${NO_WINDOW} --eval "${ELISP}"
 fi
-- 
2.1.4

[PATCH v4 5/5] notmuch-emacs-mua: do not create a frame by default with --client

[Jani Nikula]

Make the default behaviour for --client the same as emacsclient
default: do not create a new frame. Add a new option --create-frame,
passing the same option to emacsclient to create a frame.

---

v2: fix killing frame with --create-frame

v3: fix --create-frame w/o --client parameter check

v4: fix kill terminal logic, document behaviour with daemon mode
---
 doc/man1/notmuch-emacs-mua.rst |  6 ++++++
 notmuch-emacs-mua              | 14 ++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/doc/man1/notmuch-emacs-mua.rst b/doc/man1/notmuch-emacs-mua.rst
index 89a2fead17b1..c3689eb82c8a 100644
--- a/doc/man1/notmuch-emacs-mua.rst
+++ b/doc/man1/notmuch-emacs-mua.rst
@@ -45,6 +45,12 @@ Supported options for **notmuch-emacs-mua** include
         Automatically start Emacs in daemon mode, if the Emacs server
         is not running. Applicable with ``--client``.
 
+    ``--create-frame``
+        Create a new frame instead of trying to use the current Emacs
+        frame. Applicable with ``--client``. This will be required
+        when Emacs is running (or automatically started with
+        ``--auto-daemon``) in daemon mode.
+
     ``--print``
         Output the resulting elisp to stdout instead of evaluating it.
 
diff --git a/notmuch-emacs-mua b/notmuch-emacs-mua
index ac03a4a60d56..b51d8d0e2c5b 100755
--- a/notmuch-emacs-mua
+++ b/notmuch-emacs-mua
@@ -37,11 +37,10 @@ PRINT_ONLY=
 NO_WINDOW=
 USE_EMACSCLIENT=
 AUTO_DAEMON=
-CREATE_FRAME="-c"
+CREATE_FRAME=
 
 # The crux of it all: construct an elisp progn and eval it.
 ELISP="(prog1 'done (require 'notmuch) (notmuch-mua-new-mail)"
-ELISP="${ELISP} (setq message-exit-actions (list #'save-buffers-kill-terminal))"
 
 # Short options compatible with mutt(1).
 while getopts :s:c:b:i:h opt; do
@@ -65,7 +64,7 @@ while getopts :s:c:b:i:h opt; do
 		    opt=${opt%%=*}
 		    ;;
 		# Long options without arguments.
-		--help|--print|--no-window-system|--client|--auto-daemon)
+		--help|--print|--no-window-system|--client|--auto-daemon|--create-frame)
 		    ;;
 		*)
 		    echo "$0: unknown long option ${opt}, or argument mismatch." >&2
@@ -103,7 +102,6 @@ while getopts :s:c:b:i:h opt; do
 	    ;;
 	--no-window-system)
 	    NO_WINDOW="-nw"
-	    CREATE_FRAME=
 	    ;;
 	--client)
 	    USE_EMACSCLIENT="yes"
@@ -111,6 +109,9 @@ while getopts :s:c:b:i:h opt; do
 	--auto-daemon)
 	    AUTO_DAEMON="--alternate-editor="
 	    ;;
+	--create-frame)
+	    CREATE_FRAME="-c"
+	    ;;
 	*)
 	    # We should never end up here.
 	    echo "$0: internal error (option ${opt})." >&2
@@ -128,6 +129,11 @@ for arg; do
     ELISP="${ELISP} (message-goto-to) (insert \"${arg}, \")"
 done
 
+# Kill the terminal/frame if we're creating one.
+if [ -z "$USE_EMACSCLIENT" -o -n "$CREATE_FRAME" -o -n "$NO_WINDOW" ]; then
+    ELISP="${ELISP} (setq message-exit-actions (list #'save-buffers-kill-terminal))"
+fi
+
 # End progn.
 ELISP="${ELISP})"
 
-- 
2.1.4

Re: [PATCH v4 5/5] notmuch-emacs-mua: do not create a frame by default with --client

[Tomi Ollila]

On Sat, Aug 15 2015, Jani Nikula <jani@nikula.org> wrote:

> Make the default behaviour for --client the same as emacsclient
> default: do not create a new frame. Add a new option --create-frame,
> passing the same option to emacsclient to create a frame.
>
> ---
>
> v2: fix killing frame with --create-frame
>
> v3: fix --create-frame w/o --client parameter check
>
> v4: fix kill terminal logic, document behaviour with daemon mode
> ---
>  doc/man1/notmuch-emacs-mua.rst |  6 ++++++
>  notmuch-emacs-mua              | 14 ++++++++++----
>  2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/doc/man1/notmuch-emacs-mua.rst b/doc/man1/notmuch-emacs-mua.rst
> index 89a2fead17b1..c3689eb82c8a 100644
> --- a/doc/man1/notmuch-emacs-mua.rst
> +++ b/doc/man1/notmuch-emacs-mua.rst
> @@ -45,6 +45,12 @@ Supported options for **notmuch-emacs-mua** include
>          Automatically start Emacs in daemon mode, if the Emacs server
>          is not running. Applicable with ``--client``.
>  
> +    ``--create-frame``
> +        Create a new frame instead of trying to use the current Emacs
> +        frame. Applicable with ``--client``. This will be required
> +        when Emacs is running (or automatically started with
> +        ``--auto-daemon``) in daemon mode.
> +
>      ``--print``
>          Output the resulting elisp to stdout instead of evaluating it.
>  
> diff --git a/notmuch-emacs-mua b/notmuch-emacs-mua
> index ac03a4a60d56..b51d8d0e2c5b 100755
> --- a/notmuch-emacs-mua
> +++ b/notmuch-emacs-mua
> @@ -37,11 +37,10 @@ PRINT_ONLY=
>  NO_WINDOW=
>  USE_EMACSCLIENT=
>  AUTO_DAEMON=
> -CREATE_FRAME="-c"
> +CREATE_FRAME=
>  
>  # The crux of it all: construct an elisp progn and eval it.
>  ELISP="(prog1 'done (require 'notmuch) (notmuch-mua-new-mail)"
> -ELISP="${ELISP} (setq message-exit-actions (list #'save-buffers-kill-terminal))"
>  
>  # Short options compatible with mutt(1).
>  while getopts :s:c:b:i:h opt; do
> @@ -65,7 +64,7 @@ while getopts :s:c:b:i:h opt; do
>  		    opt=${opt%%=*}
>  		    ;;
>  		# Long options without arguments.
> -		--help|--print|--no-window-system|--client|--auto-daemon)
> +		--help|--print|--no-window-system|--client|--auto-daemon|--create-frame)
>  		    ;;
>  		*)
>  		    echo "$0: unknown long option ${opt}, or argument mismatch." >&2
> @@ -103,7 +102,6 @@ while getopts :s:c:b:i:h opt; do
>  	    ;;
>  	--no-window-system)
>  	    NO_WINDOW="-nw"
> -	    CREATE_FRAME=
>  	    ;;
>  	--client)
>  	    USE_EMACSCLIENT="yes"
> @@ -111,6 +109,9 @@ while getopts :s:c:b:i:h opt; do
>  	--auto-daemon)
>  	    AUTO_DAEMON="--alternate-editor="
>  	    ;;
> +	--create-frame)
> +	    CREATE_FRAME="-c"
> +	    ;;
>  	*)
>  	    # We should never end up here.
>  	    echo "$0: internal error (option ${opt})." >&2

This looks pretty good now; I player quite a bit with --client,
--create-frame and --auto-daemon 
(with and without setting EMACSCLIENT='echo emacsclient')

Based on that I think this check should be added here (after option
parsing):

if [ -n "$AUTO_DAEMON" -a -z "$CREATE_FRAME" ]; then
    echo "$0: --auto-daemon is only applicable with --create-frame." >&2
    exit 1       
fi

without this one may execute ./notmuch-emacs-mua --client --auto-daemon
which yields starting emacs in daemon mode (in this example it is expected
emacs is not running; otherwise --auto-daemon has no use in this example)
-- but no ui to that newly-running emacs is provided. Similar behaviour
can be observed by the following

$ emacsclient --eval '(kill emacs) ;; but beware! ' 
$ emacsclient --alternate-editor= --eval '(find-file "/etc/passwd")'

(side note: using this --alternate-editor= is pretty nifty way to get it done)

emacs --daemon is executed which loaded /etc/passwd but w/o ui

Note the difference in:

emacsclient --alternate-editor= /etc/passwd

now emacs --daemon is executed *and* ui where /etc/passwd is available is
provided!

otoh,
emacsclient --alternate-editor= -c [-nw] --eval '(find-file "/etc/passwd")'

works fine, albeit a bit different than the one where /etc/passwd was given
as a simple filename argument.

comparative to ./notmuch-emacs-mua --client --create-frame --auto-daemon 

> @@ -128,6 +129,11 @@ for arg; do
>      ELISP="${ELISP} (message-goto-to) (insert \"${arg}, \")"
>  done
>  
> +# Kill the terminal/frame if we're creating one.
> +if [ -z "$USE_EMACSCLIENT" -o -n "$CREATE_FRAME" -o -n "$NO_WINDOW" ]; then
> +    ELISP="${ELISP} (setq message-exit-actions (list #'save-buffers-kill-terminal))"
> +fi

I am not very happy that message-exit-actions was added to $ELISP when
not using emacsclient; when emacs is started its sole (initial) purpose is
to serve mail sending (and not lending a frame in some other emacs) -- in
this case it would be nice to be able to retrieve the sent mail buffer.

perhaps the above could be changed to just 

if [ -n "$CREATE_FRAME" ]; then
    ELISP="${ELISP} (setq message-exit-actions (list #'save-buffers-kill-terminal))"
fi

another option is --yet-another-option ;/

No other issues (relevant to this series).

Tomi


> +
>  # End progn.
>  ELISP="${ELISP})"
>  
> -- 
> 2.1.4

Re: [PATCH v4 5/5] notmuch-emacs-mua: do not create a frame by default with --client

[Jani Nikula]

On Wed, 26 Aug 2015, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Sat, Aug 15 2015, Jani Nikula <jani@nikula.org> wrote:
>
>> Make the default behaviour for --client the same as emacsclient
>> default: do not create a new frame. Add a new option --create-frame,
>> passing the same option to emacsclient to create a frame.
>>
>> ---
>>
>> v2: fix killing frame with --create-frame
>>
>> v3: fix --create-frame w/o --client parameter check
>>
>> v4: fix kill terminal logic, document behaviour with daemon mode
>> ---
>>  doc/man1/notmuch-emacs-mua.rst |  6 ++++++
>>  notmuch-emacs-mua              | 14 ++++++++++----
>>  2 files changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/doc/man1/notmuch-emacs-mua.rst b/doc/man1/notmuch-emacs-mua.rst
>> index 89a2fead17b1..c3689eb82c8a 100644
>> --- a/doc/man1/notmuch-emacs-mua.rst
>> +++ b/doc/man1/notmuch-emacs-mua.rst
>> @@ -45,6 +45,12 @@ Supported options for **notmuch-emacs-mua** include
>>          Automatically start Emacs in daemon mode, if the Emacs server
>>          is not running. Applicable with ``--client``.
>>  
>> +    ``--create-frame``
>> +        Create a new frame instead of trying to use the current Emacs
>> +        frame. Applicable with ``--client``. This will be required
>> +        when Emacs is running (or automatically started with
>> +        ``--auto-daemon``) in daemon mode.
>> +
>>      ``--print``
>>          Output the resulting elisp to stdout instead of evaluating it.
>>  
>> diff --git a/notmuch-emacs-mua b/notmuch-emacs-mua
>> index ac03a4a60d56..b51d8d0e2c5b 100755
>> --- a/notmuch-emacs-mua
>> +++ b/notmuch-emacs-mua
>> @@ -37,11 +37,10 @@ PRINT_ONLY=
>>  NO_WINDOW=
>>  USE_EMACSCLIENT=
>>  AUTO_DAEMON=
>> -CREATE_FRAME="-c"
>> +CREATE_FRAME=
>>  
>>  # The crux of it all: construct an elisp progn and eval it.
>>  ELISP="(prog1 'done (require 'notmuch) (notmuch-mua-new-mail)"
>> -ELISP="${ELISP} (setq message-exit-actions (list #'save-buffers-kill-terminal))"
>>  
>>  # Short options compatible with mutt(1).
>>  while getopts :s:c:b:i:h opt; do
>> @@ -65,7 +64,7 @@ while getopts :s:c:b:i:h opt; do
>>  		    opt=${opt%%=*}
>>  		    ;;
>>  		# Long options without arguments.
>> -		--help|--print|--no-window-system|--client|--auto-daemon)
>> +		--help|--print|--no-window-system|--client|--auto-daemon|--create-frame)
>>  		    ;;
>>  		*)
>>  		    echo "$0: unknown long option ${opt}, or argument mismatch." >&2
>> @@ -103,7 +102,6 @@ while getopts :s:c:b:i:h opt; do
>>  	    ;;
>>  	--no-window-system)
>>  	    NO_WINDOW="-nw"
>> -	    CREATE_FRAME=
>>  	    ;;
>>  	--client)
>>  	    USE_EMACSCLIENT="yes"
>> @@ -111,6 +109,9 @@ while getopts :s:c:b:i:h opt; do
>>  	--auto-daemon)
>>  	    AUTO_DAEMON="--alternate-editor="
>>  	    ;;
>> +	--create-frame)
>> +	    CREATE_FRAME="-c"
>> +	    ;;
>>  	*)
>>  	    # We should never end up here.
>>  	    echo "$0: internal error (option ${opt})." >&2
>
> This looks pretty good now; I player quite a bit with --client,
> --create-frame and --auto-daemon 
> (with and without setting EMACSCLIENT='echo emacsclient')
>
> Based on that I think this check should be added here (after option
> parsing):
>
> if [ -n "$AUTO_DAEMON" -a -z "$CREATE_FRAME" ]; then
>     echo "$0: --auto-daemon is only applicable with --create-frame." >&2
>     exit 1       
> fi
>
> without this one may execute ./notmuch-emacs-mua --client --auto-daemon
> which yields starting emacs in daemon mode (in this example it is expected
> emacs is not running; otherwise --auto-daemon has no use in this example)
> -- but no ui to that newly-running emacs is provided. Similar behaviour
> can be observed by the following

This problem is present with the current code already, right now. I
don't see the need to fix everything in my series.

> $ emacsclient --eval '(kill emacs) ;; but beware! ' 
> $ emacsclient --alternate-editor= --eval '(find-file "/etc/passwd")'
>
> (side note: using this --alternate-editor= is pretty nifty way to get it done)
>
> emacs --daemon is executed which loaded /etc/passwd but w/o ui
>
> Note the difference in:
>
> emacsclient --alternate-editor= /etc/passwd
>
> now emacs --daemon is executed *and* ui where /etc/passwd is available is
> provided!
>
> otoh,
> emacsclient --alternate-editor= -c [-nw] --eval '(find-file "/etc/passwd")'
>
> works fine, albeit a bit different than the one where /etc/passwd was given
> as a simple filename argument.
>
> comparative to ./notmuch-emacs-mua --client --create-frame --auto-daemon 
>
>> @@ -128,6 +129,11 @@ for arg; do
>>      ELISP="${ELISP} (message-goto-to) (insert \"${arg}, \")"
>>  done
>>  
>> +# Kill the terminal/frame if we're creating one.
>> +if [ -z "$USE_EMACSCLIENT" -o -n "$CREATE_FRAME" -o -n "$NO_WINDOW" ]; then
>> +    ELISP="${ELISP} (setq message-exit-actions (list #'save-buffers-kill-terminal))"
>> +fi
>
> I am not very happy that message-exit-actions was added to $ELISP when
> not using emacsclient; when emacs is started its sole (initial) purpose is
> to serve mail sending (and not lending a frame in some other emacs) -- in
> this case it would be nice to be able to retrieve the sent mail buffer.

And I am not very happy with being expected to modify these patches ad
infinitum while trying to get back the *one* use case I wrote this
script for in the first place, and that got broken somewhere down the
line.

Sincerely,
Jani.


>
> perhaps the above could be changed to just 
>
> if [ -n "$CREATE_FRAME" ]; then
>     ELISP="${ELISP} (setq message-exit-actions (list #'save-buffers-kill-terminal))"
> fi
>
> another option is --yet-another-option ;/
>
> No other issues (relevant to this series).
>
> Tomi
>
>
>> +
>>  # End progn.
>>  ELISP="${ELISP})"
>>  
>> -- 
>> 2.1.4

Re: [PATCH v4 5/5] notmuch-emacs-mua: do not create a frame by default with --client

[Tomi Ollila]

On Fri, Aug 28 2015, Jani Nikula <jani@nikula.org> wrote:

> On Wed, 26 Aug 2015, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>> On Sat, Aug 15 2015, Jani Nikula <jani@nikula.org> wrote:
>>
>>> Make the default behaviour for --client the same as emacsclient
>>> default: do not create a new frame. Add a new option --create-frame,
>>> passing the same option to emacsclient to create a frame.
>>>
>>> ---
>>>
>>> v2: fix killing frame with --create-frame
>>>
>>> v3: fix --create-frame w/o --client parameter check
>>>
>>> v4: fix kill terminal logic, document behaviour with daemon mode
>>> ---
>>>  doc/man1/notmuch-emacs-mua.rst |  6 ++++++
>>>  notmuch-emacs-mua              | 14 ++++++++++----
>>>  2 files changed, 16 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/doc/man1/notmuch-emacs-mua.rst b/doc/man1/notmuch-emacs-mua.rst
>>> index 89a2fead17b1..c3689eb82c8a 100644
>>> --- a/doc/man1/notmuch-emacs-mua.rst
>>> +++ b/doc/man1/notmuch-emacs-mua.rst
>>> @@ -45,6 +45,12 @@ Supported options for **notmuch-emacs-mua** include
>>>          Automatically start Emacs in daemon mode, if the Emacs server
>>>          is not running. Applicable with ``--client``.
>>>  
>>> +    ``--create-frame``
>>> +        Create a new frame instead of trying to use the current Emacs
>>> +        frame. Applicable with ``--client``. This will be required
>>> +        when Emacs is running (or automatically started with
>>> +        ``--auto-daemon``) in daemon mode.
>>> +
>>>      ``--print``
>>>          Output the resulting elisp to stdout instead of evaluating it.
>>>  
>>> diff --git a/notmuch-emacs-mua b/notmuch-emacs-mua
>>> index ac03a4a60d56..b51d8d0e2c5b 100755
>>> --- a/notmuch-emacs-mua
>>> +++ b/notmuch-emacs-mua
>>> @@ -37,11 +37,10 @@ PRINT_ONLY=
>>>  NO_WINDOW=
>>>  USE_EMACSCLIENT=
>>>  AUTO_DAEMON=
>>> -CREATE_FRAME="-c"
>>> +CREATE_FRAME=
>>>  
>>>  # The crux of it all: construct an elisp progn and eval it.
>>>  ELISP="(prog1 'done (require 'notmuch) (notmuch-mua-new-mail)"
>>> -ELISP="${ELISP} (setq message-exit-actions (list #'save-buffers-kill-terminal))"
>>>  
>>>  # Short options compatible with mutt(1).
>>>  while getopts :s:c:b:i:h opt; do
>>> @@ -65,7 +64,7 @@ while getopts :s:c:b:i:h opt; do
>>>  		    opt=${opt%%=*}
>>>  		    ;;
>>>  		# Long options without arguments.
>>> -		--help|--print|--no-window-system|--client|--auto-daemon)
>>> +		--help|--print|--no-window-system|--client|--auto-daemon|--create-frame)
>>>  		    ;;
>>>  		*)
>>>  		    echo "$0: unknown long option ${opt}, or argument mismatch." >&2
>>> @@ -103,7 +102,6 @@ while getopts :s:c:b:i:h opt; do
>>>  	    ;;
>>>  	--no-window-system)
>>>  	    NO_WINDOW="-nw"
>>> -	    CREATE_FRAME=
>>>  	    ;;
>>>  	--client)
>>>  	    USE_EMACSCLIENT="yes"
>>> @@ -111,6 +109,9 @@ while getopts :s:c:b:i:h opt; do
>>>  	--auto-daemon)
>>>  	    AUTO_DAEMON="--alternate-editor="
>>>  	    ;;
>>> +	--create-frame)
>>> +	    CREATE_FRAME="-c"
>>> +	    ;;
>>>  	*)
>>>  	    # We should never end up here.
>>>  	    echo "$0: internal error (option ${opt})." >&2
>>
>> This looks pretty good now; I player quite a bit with --client,
>> --create-frame and --auto-daemon 
>> (with and without setting EMACSCLIENT='echo emacsclient')
>>
>> Based on that I think this check should be added here (after option
>> parsing):
>>
>> if [ -n "$AUTO_DAEMON" -a -z "$CREATE_FRAME" ]; then
>>     echo "$0: --auto-daemon is only applicable with --create-frame." >&2
>>     exit 1       
>> fi
>>
>> without this one may execute ./notmuch-emacs-mua --client --auto-daemon
>> which yields starting emacs in daemon mode (in this example it is expected
>> emacs is not running; otherwise --auto-daemon has no use in this example)
>> -- but no ui to that newly-running emacs is provided. Similar behaviour
>> can be observed by the following
>
> This problem is present with the current code already, right now. I
> don't see the need to fix everything in my series.

Yes, the problem was already present in patch 4/5 (and probably earlier as
you mentioned that there was problem already) -- I just did not want
to pick that also in this discussion... ;/

>> $ emacsclient --eval '(kill emacs) ;; but beware! ' 
>> $ emacsclient --alternate-editor= --eval '(find-file "/etc/passwd")'
>>
>> (side note: using this --alternate-editor= is pretty nifty way to get it done)
>>
>> emacs --daemon is executed which loaded /etc/passwd but w/o ui
>>
>> Note the difference in:
>>
>> emacsclient --alternate-editor= /etc/passwd
>>
>> now emacs --daemon is executed *and* ui where /etc/passwd is available is
>> provided!
>>
>> otoh,
>> emacsclient --alternate-editor= -c [-nw] --eval '(find-file "/etc/passwd")'
>>
>> works fine, albeit a bit different than the one where /etc/passwd was given
>> as a simple filename argument.
>>
>> comparative to ./notmuch-emacs-mua --client --create-frame --auto-daemon 
>>
>>> @@ -128,6 +129,11 @@ for arg; do
>>>      ELISP="${ELISP} (message-goto-to) (insert \"${arg}, \")"
>>>  done
>>>  
>>> +# Kill the terminal/frame if we're creating one.
>>> +if [ -z "$USE_EMACSCLIENT" -o -n "$CREATE_FRAME" -o -n "$NO_WINDOW" ]; then
>>> +    ELISP="${ELISP} (setq message-exit-actions (list #'save-buffers-kill-terminal))"
>>> +fi
>>
>> I am not very happy that message-exit-actions was added to $ELISP when
>> not using emacsclient; when emacs is started its sole (initial) purpose is
>> to serve mail sending (and not lending a frame in some other emacs) -- in
>> this case it would be nice to be able to retrieve the sent mail buffer.
>
> And I am not very happy with being expected to modify these patches ad
> infinitum while trying to get back the *one* use case I wrote this
> script for in the first place, and that got broken somewhere down the
> line.

I can agree that these patches could be applied as is and I can do the
followup patch that changes the things later ... but I'd like to get these
changes I presented discussed and agreed (whatever the agreement is) before
doing these so that the result will be good and safe for all potential
users...

>
> Sincerely,
> Jani.
>

ystävällisin terveisin :D,
Tomi

>
>>
>> perhaps the above could be changed to just 
>>
>> if [ -n "$CREATE_FRAME" ]; then
>>     ELISP="${ELISP} (setq message-exit-actions (list #'save-buffers-kill-terminal))"
>> fi
>>
>> another option is --yet-another-option ;/
>>
>> No other issues (relevant to this series).
>>
>> Tomi
>>
>>
>>> +
>>>  # End progn.
>>>  ELISP="${ELISP})"
>>>  
>>> -- 
>>> 2.1.4

Re: [PATCH v4 5/5] notmuch-emacs-mua: do not create a frame by default with --client

[David Bremner]

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


> if [ -n "$AUTO_DAEMON" -a -z "$CREATE_FRAME" ]; then
>     echo "$0: --auto-daemon is only applicable with --create-frame." >&2
>     exit 1       
> fi
>
> without this one may execute ./notmuch-emacs-mua --client --auto-daemon
> which yields starting emacs in daemon mode (in this example it is expected
> emacs is not running; otherwise --auto-daemon has no use in this example)
> -- but no ui to that newly-running emacs is provided. Similar behaviour
> can be observed by the following
>

I think what you propose is fine for a followup patch; note that the
scenario you worry about also needs --client to be a problem. Apparently
nothing is uncontroversial here, but if auto-daemon only works with
create frame, then perhaps the followup would be to have auto-daemon
imply create-frame

>>  
>> +# Kill the terminal/frame if we're creating one.
>> +if [ -z "$USE_EMACSCLIENT" -o -n "$CREATE_FRAME" -o -n "$NO_WINDOW" ]; then
>> +    ELISP="${ELISP} (setq message-exit-actions (list #'save-buffers-kill-terminal))"
>> +fi
>
> I am not very happy that message-exit-actions was added to $ELISP when
> not using emacsclient; when emacs is started its sole (initial) purpose is
> to serve mail sending (and not lending a frame in some other emacs) -- in
> this case it would be nice to be able to retrieve the sent mail buffer.

I'm somewhat less sympathetic here. AIUI, the goal of notmuch-emacs-mua
is to provide a drop in tool to replace mutt for sending mail in the
shell or from other programs. In this case, I think the most common
expectation is to terminate after sending mail (e.g. to return use of
the shell to the user, or allow the spawning program to continue).
However, it seems like this would be relatively easy to set up some
customization in notmuch-message-mode so that it could check if running
from the cli, and then either kill-or-not emacs depending on some
variable. So I think I'd like to keep the current behaviour as default,
and make it customizable at the emacs level.

Re: [PATCH v4 0/5] notmuch-emacs-mua updates

[David Bremner]

Jani Nikula <jani@nikula.org> writes:

> The main goal is to not force --create-frame on users who dislike
> it. Split to some hopefully less controversial prep patches.

series pushed.

d

Re: [PATCH v4 5/5] notmuch-emacs-mua: do not create a frame by default with --client

[Tomi Ollila]

On Mon, Sep 28 2015, David Bremner <david@tethera.net> wrote:

> Tomi Ollila <tomi.ollila@iki.fi> writes:
>
>
>> if [ -n "$AUTO_DAEMON" -a -z "$CREATE_FRAME" ]; then
>>     echo "$0: --auto-daemon is only applicable with --create-frame." >&2
>>     exit 1       
>> fi
>>
>> without this one may execute ./notmuch-emacs-mua --client --auto-daemon
>> which yields starting emacs in daemon mode (in this example it is expected
>> emacs is not running; otherwise --auto-daemon has no use in this example)
>> -- but no ui to that newly-running emacs is provided. Similar behaviour
>> can be observed by the following
>>
>
> I think what you propose is fine for a followup patch; note that the
> scenario you worry about also needs --client to be a problem. Apparently
> nothing is uncontroversial here, but if auto-daemon only works with
> create frame, then perhaps the followup would be to have auto-daemon
> imply create-frame

Without --client --auto-daemon is no-op (as it is no-op in case emacs
server is already running). I am (only) concerned about user experience
when one runs --client --auto-daemon and user gets nothing (i.e. emacs
server is running in the background w/o any clients connected to it.

We could make --auto-daemon imply --create-frame, but then 

./notmuch-emacs-mua --auto-daemon (i.e. w/o --client) starts new mail
compose window to separate frame (even though user did not request
it w/ --create-frame)

(actually I already did the 'imply' option (easy, one line in script,
another in namual), just that testing it gave this thought...

... therefore I'd rather make ./notmuch-emacs-mua --auto-daemon
spit an error and exit -- but I can be convinced otherwise :)

Tomi

Re: [PATCH v4 5/5] notmuch-emacs-mua: do not create a frame by default with --client

[Tomi Ollila]

On Tue, Sep 29 2015, Tomi Ollila <tomi.ollila@iki.fi> wrote:

> On Mon, Sep 28 2015, David Bremner <david@tethera.net> wrote:
>
>> Tomi Ollila <tomi.ollila@iki.fi> writes:
>>
>>
>>> if [ -n "$AUTO_DAEMON" -a -z "$CREATE_FRAME" ]; then
>>>     echo "$0: --auto-daemon is only applicable with --create-frame." >&2
>>>     exit 1       
>>> fi
>>>
>>> without this one may execute ./notmuch-emacs-mua --client --auto-daemon
>>> which yields starting emacs in daemon mode (in this example it is expected
>>> emacs is not running; otherwise --auto-daemon has no use in this example)
>>> -- but no ui to that newly-running emacs is provided. Similar behaviour
>>> can be observed by the following
>>>
>>
>> I think what you propose is fine for a followup patch; note that the
>> scenario you worry about also needs --client to be a problem. Apparently
>> nothing is uncontroversial here, but if auto-daemon only works with
>> create frame, then perhaps the followup would be to have auto-daemon
>> imply create-frame
>
> Without --client --auto-daemon is no-op (as it is no-op in case emacs
> server is already running). I am (only) concerned about user experience
> when one runs --client --auto-daemon and user gets nothing (i.e. emacs
> server is running in the background w/o any clients connected to it.
>
> We could make --auto-daemon imply --create-frame, but then 
>
> ./notmuch-emacs-mua --auto-daemon (i.e. w/o --client) starts new mail
> compose window to separate frame (even though user did not request
> it w/ --create-frame)

Hmm, a few more tests. it now looks like in case the command line is

./notmuch-emacs-mua --client --auto-daemon

Then the --create-frame should be implied. w/o --client --create-frame
would ... well, create that (only) emacs frame anyways... oh, the
combinations; for reference I just paste from my history...


    1  07:57  0:01  ccd notmuch
    2  07:57  0:06  git pull --rebase
    3  07:57  0:01  git log
    4  07:57  0:04  tig
    5  07:58  0:10  ./notmuch-emacs-mua --auto-daemon
    6  07:58  0:00  ps ax
    8  07:58  0:00  emacs &
    9  07:58  0:07  ./notmuch-emacs-mua --auto-daemon
   10  07:59  0:01  ./notmuch-emacs-mua --client --auto-daemon
   12  08:01  0:03  ./notmuch-emacs-mua --auto-daemon
   13  08:02  0:00  ps ax
   14  08:02  0:00  ls /tmp/emacs1001
   15  08:02  0:00  ./notmuch-emacs-mua -nw --auto-daemon
   16  08:02  0:04  DISPLAY= ./notmuch-emacs-mua  --auto-daemon
   17  08:02  0:05  tig
   18  08:03  0:02  DISPLAY= ./notmuch-emacs-mua --auto-daemon
   19  08:04  0:01  DISPLAY= ./notmuch-emacs-mua --client --auto-daemon
   20  08:04  0:00  ps ax
   21  08:04  0:00  kill 1860
   22  08:04  0:00  ps ax
   23  08:06  0:00   ./notmuch-emacs-mua --client --auto-daemon
   24  08:06  0:00  ps ax
   25  08:06  0:00  kill 1875
   26  08:06  0:00   ./notmuch-emacs-mua --client --auto-daemon
   27  08:06  0:00  ps ax
   28  08:06  0:00  kill 1926
   29  08:06  0:06  ./notmuch-emacs-mua --client --auto-daemon --create-frame
   30  08:07  0:28  ./notmuch-emacs-mua --auto-daemon --create-frame
   31  08:08  0:00  ps x
   32  08:08  0:00  h

So. I'm going w/ the suggestion making --auto-daemon imply --create-frame


>
> (actually I already did the 'imply' option (easy, one line in script,
> another in namual), just that testing it gave this thought...
>
> ... therefore I'd rather make ./notmuch-emacs-mua --auto-daemon
> spit an error and exit -- but I can be convinced otherwise :)
>
> Tomi