Ludovic Courtès (2015-10-18 19:50 +0300) wrote:

> Alex Kost <[email protected]> skribis:
>
>> Ludovic Courtès (2015-10-17 16:34 +0300) wrote:
>
> [...]
>
>>> The docstring should mention that it can be the empty string.
>>> Alternately, ‘configure’ could set DOT_USER_PROGRAM to “dot” when it’s
>>> not found, which I think is ever preferable.  WDYT?
>>
>> IMO an empty string is preferable (explained below), so I would like to
>> leave it like this (with an improved docstring as you pointed), if you
>> don't mind.
>>
>>>> +(defcustom guix-dot-program
>>>> +  (if (string= "" guix-config-dot-program)
>>>> +      (executable-find "dot")
>>>> +    guix-config-dot-program)
>>>
>>> Thus here we’d be checking whether ‘guix-config-dot-program’ is an
>>> absolute file name.
>>
>> I think (executable-find "dot") is more reliable than just "dot" which
>> may not exist.
>
> Agreed!  I was just saying that an empty string is odd, and that setting
> it to ‘dot’ would be aesthetically more pleasing.

Not for me, but OK.

> So the only different is that the above would become:
>
>   (defcustom guix-dot-program
>     (if (absolute-file-name-p guix-config-dot-program)
>          guix-config-dot-program
>       (executable-find "dot")))
>
> Does it make sense?

Yes, now I see what you mean.  I still prefer an empty string, but I
don't mind the variant you suggest.  The updated patch is attached,
thanks!

>From 0cc7c8d44b88e7df9a0f443d8939c6b8ac14232e Mon Sep 17 00:00:00 2001
From: Alex Kost <[email protected]>
Date: Fri, 16 Oct 2015 20:34:32 +0300
Subject: [PATCH] build: Set DOT_USER_PROGRAM for Emacs interface.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Suggested by Ludovic Courtès <[email protected]>.

* configure.ac: Set DOT_USER_PROGRAM variable.
* emacs/guix-config.el.in (guix-config-dot-program): New constant.
* emacs/guix-external.el (guix-dot-program): Use it.
---
 configure.ac            | 1 +
 emacs/guix-config.el.in | 4 ++++
 emacs/guix-external.el  | 7 ++++++-
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index bb3d947..00e7c7b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -206,6 +206,7 @@ AC_CONFIG_FILES([pre-inst-env], [chmod +x pre-inst-env])
 AC_CONFIG_FILES([test-env],     [chmod +x test-env])
 
 dnl Emacs interface.
+AC_PATH_PROG([DOT_USER_PROGRAM], [dot], [dot])
 AM_PATH_LISPDIR
 AM_CONDITIONAL([HAVE_EMACS], [test "x$EMACS" != "xno"])
 
diff --git a/emacs/guix-config.el.in b/emacs/guix-config.el.in
index 16434ce..c7affb8 100644
--- a/emacs/guix-config.el.in
+++ b/emacs/guix-config.el.in
@@ -1,6 +1,7 @@
 ;;; guix-config.el --- Compile-time configuration of Guix.
 
 ;; Copyright © 2015 Mathieu Lirzin <[email protected]>
+;; Copyright © 2015 Alex Kost <[email protected]>
 
 ;; This file is part of GNU Guix.
 
@@ -35,6 +36,9 @@ strings of the form:
 
 Where ARGS is a list of arguments to the guile program.")
 
+(defconst guix-config-dot-program "@DOT_USER_PROGRAM@"
+  "Name of the 'dot' executable defined at configure time.")
+
 (provide 'guix-config)
 
 ;;; guix-config.el ends here
diff --git a/emacs/guix-external.el b/emacs/guix-external.el
index 580676e..cf4bd26 100644
--- a/emacs/guix-external.el
+++ b/emacs/guix-external.el
@@ -23,11 +23,16 @@
 
 ;;; Code:
 
+(require 'guix-config)
+
 (defgroup guix-external nil
   "Settings for external programs."
   :group 'guix)
 
-(defcustom guix-dot-program (executable-find "dot")
+(defcustom guix-dot-program
+  (if (file-name-absolute-p guix-config-dot-program)
+      guix-config-dot-program
+    (executable-find "dot"))
   "Name of the 'dot' executable."
   :type 'string
   :group 'guix-external)
-- 
2.5.0

Reply via email to