From: Michal Sojka <sojkam1@fel.cvut.cz>
To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org
Subject: Re: [PATCH] emacs: prefer notmuch-emacs-version in User-Agent: header
In-Reply-To: <1407496781-17458-1-git-send-email-tomi.ollila@iki.fi>
References: <1407496781-17458-1-git-send-email-tomi.ollila@iki.fi>
User-Agent: Notmuch/0.18.1+101~g56b0ff0 (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-pc-linux-gnu)
Date: Sat, 27 Sep 2014 16:56:03 +0200
Message-ID: <87oau1un7g.fsf@steelpick.2x.cz>
MIME-Version: 1.0
Content-Type: text/plain
Cc: tomi.ollila@iki.fi
Precedence: list

Hi Tomi,

On Fri, Aug 08 2014, Tomi Ollila wrote:
> Now that we have `notmuch-emacs-version' defined in notmuch emacs MUA
> use that as a part of User-Agent: header to provide more accurate
> version information when sending emails.
>
> In case some incomplete installation of notmuch emacs MUA is used and
> `notmuch-emacs-version' is defined as "unknown" then fall back to ask
> version info from cli (as it used to be) -- the function to do that was
> removed from `notmuch-version' to `notmuch-cli-version' to make things
> clearer and more consistent.
> ---
>  NEWS                   | 14 ++++++++++++++
>  emacs/notmuch-hello.el |  2 +-
>  emacs/notmuch-lib.el   |  4 ++--
>  emacs/notmuch-mua.el   | 11 ++++++++---
>  4 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index f7aaedf91d07..9664146768f3 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -7,6 +7,20 @@ Library changes
>  Add return status to notmuch_database_close and
>  notmuch_database_destroy
>  
> +Emacs Interface
> +---------------
> +
> +`notmuch-emacs-version` is used in `User-Agent` header
> +
> +  The value of recently introduced variable `notmuch-emacs-version` is
> +  now used as a part of `User-Agent` header when sending emails.
> +
> +Removed `notmuch-version` function by renaming it to `notmuch-cli-version`
> +
> +  With existing variable `notmuch-emacs-version` the accompanied
> +  function which retrieves the version of `notmuch-command` is
> +  better named as `notmuch-cli-version`.
> +
>  nmbug-status
>  ------------
>  
> diff --git a/emacs/notmuch-hello.el b/emacs/notmuch-hello.el
> index 65d062760a71..7bfa752d2a04 100644
> --- a/emacs/notmuch-hello.el
> +++ b/emacs/notmuch-hello.el
> @@ -628,7 +628,7 @@ (defvar notmuch-emacs-version)
>  (defun notmuch-hello-versions ()
>    "Display the notmuch version(s)"
>    (interactive)
> -  (let ((notmuch-cli-version (notmuch-version)))
> +  (let ((notmuch-cli-version (notmuch-cli-version)))
>      (message "notmuch version %s"
>  	     (if (string= notmuch-emacs-version notmuch-cli-version)
>  		 notmuch-cli-version
> diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
> index 19269e3c469b..ca18ff9d5487 100644
> --- a/emacs/notmuch-lib.el
> +++ b/emacs/notmuch-lib.el
> @@ -192,8 +192,8 @@ (defun notmuch-assert-cli-sane ()
>  "Perhaps you haven't run \"notmuch setup\" yet? Try running this
>  on the command line, and then retry your notmuch command")))
>  
> -(defun notmuch-version ()
> -  "Return a string with the notmuch version number."
> +(defun notmuch-cli-version ()
> +  "Return a string with the notmuch-command version number."
>    (let ((long-string
>  	 ;; Trim off the trailing newline.
>  	 (substring (notmuch-command-to-string "--version") 0 -1)))
> diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
> index 2c5888600b6c..dbf5df28669d 100644
> --- a/emacs/notmuch-mua.el
> +++ b/emacs/notmuch-mua.el
> @@ -100,12 +100,17 @@ (defun notmuch-mua-user-agent-full ()
>  	  " "
>  	  (notmuch-mua-user-agent-emacs)))
>  
> +;; the following variable is defined as being defconst in notmuch-version.el
> +(defvar notmuch-emacs-version)

I don't like this part. It took me quite some time to figure out what's
happening with notmuch-emacs-version so that we need defvar here. It
seems that the same piece of code is also in notmuch-hello.el. What
about introducing a function in notmuch-lib that could be used the get
the best guess about emacs version? I think that something like the
following diff would make things clearer.

-Michal

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index 19269e3..b883375 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -192,7 +192,7 @@ Otherwise the output will be returned"
 "Perhaps you haven't run \"notmuch setup\" yet? Try running this
 on the command line, and then retry your notmuch command")))
 
-(defun notmuch-version ()
+(defun notmuch-cli-version ()
   "Return a string with the notmuch version number."
   (let ((long-string
         ;; Trim off the trailing newline.
@@ -202,6 +202,15 @@ on the command line, and then retry your notmuch command")))
        (match-string 2 long-string)
       "unknown")))
 
+(unless (require 'notmuch-version nil t)
+  (defconst notmuch-emacs-version "unknown"
+    "Placeholder variable when notmuch-version.el[c] is not available."))
+
+(defun notmuch-guess-emacs-version ()
+  (if (string= notmuch-emacs-version "unknown")
+      (notmuch-cli-version)
+    notmuch-emacs-version))
+
 (defun notmuch-config-get (item)
   "Return a value from the notmuch configuration."
   (let* ((val (notmuch-command-to-string "config" "get" item))
diff --git a/emacs/notmuch.el b/emacs/notmuch.el
index b44a907..7245b2d 100644
--- a/emacs/notmuch.el
+++ b/emacs/notmuch.el
@@ -61,10 +61,6 @@
 (require 'notmuch-message)
 (require 'notmuch-parser)
 
-(unless (require 'notmuch-version nil t)
-  (defconst notmuch-emacs-version "unknown"
-    "Placeholder variable when notmuch-version.el[c] is not available."))
-
 (defcustom notmuch-search-result-format
   `(("date" . "%12s ")
     ("count" . "%-7s ")


> +
>  (defun notmuch-mua-user-agent-notmuch ()
> -  "Generate a `User-Agent:' string suitable for notmuch."
> -  (concat "Notmuch/" (notmuch-version) " (http://notmuchmail.org)"))
> +  "Generate notmuch part of `User-Agent:' string suitable for notmuch."
> +  (concat "Notmuch/" (if (string= notmuch-emacs-version "unknown")
> +			 (notmuch-cli-version)
> +		       notmuch-emacs-version) " (http://notmuchmail.org)"))
>  
>  (defun notmuch-mua-user-agent-emacs ()
> -  "Generate a `User-Agent:' string suitable for notmuch."
> +  "Generate emacs part of `User-Agent:' string suitable for notmuch."
>    (concat "Emacs/" emacs-version " (" system-configuration ")"))
>  
>  (defun notmuch-mua-add-more-hidden-headers ()
> -- 
> 2.0.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch