Re: [Quilt-dev] [PATCH 1/4] quilt.el: Refactor config reading functions

[Jean Delvare]

Hi Ondřej,

On Thu, 2020-05-14 at 21:59 +0200, Ondřej Lysoněk wrote:
> quilt-patches-directory is a copy-paste of
> quilt-pc-directory. Refactor the common code into a separate
> function.
>
> Signed-off-by: Ondřej Lysoněk <address@hidden>
> ---
>  lib/quilt.el | 29 +++++++++++------------------
>  1 file changed, 11 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/quilt.el b/lib/quilt.el
> index ae73f3d..66fb41a 100644
> --- a/lib/quilt.el
> +++ b/lib/quilt.el
> @@ -29,12 +29,12 @@
>    "Return t if there is on the bottom of patch stack, return nil if 
> otherwise."
>    (if (> (call-process "quilt" nil nil nil "applied") 0) 1))
>  
> -(defun quilt-patches-directory ()
> -  "Return the location of patch files."
> +(defun quilt--get-config-variable (var)
> +  "Return the value of a configuration variable. Return nil if it is unset."
>    (or (with-current-buffer (generate-new-buffer " *cmd")
>          (shell-command
>           (concat "test -f ~/.quiltrc && . ~/.quiltrc ;"
> -                 "echo -n $QUILT_PATCHES")
> +                 "echo -n $" var)
>           t)
>          (unwind-protect
>              (let ((v (buffer-string)))
> @@ -42,24 +42,17 @@
>                    nil
>                  v))
>            (kill-buffer (current-buffer))))
> -      (or (getenv "QUILT_PATCHES")
> -          "patches")))
> +      (getenv var)))
> +
> +(defun quilt-patches-directory ()
> +  "Return the location of patch files."
> +  (or (quilt--get-config-variable "QUILT_PATCHES")
> +      "patches"))
>  
>  (defun quilt-pc-directory ()
>    "Return the location of patch files."
> -  (or (with-current-buffer (generate-new-buffer " *cmd")
> -        (shell-command
> -         (concat "test -f ~/.quiltrc && . ~/.quiltrc ;"
> -                 "echo -n $QUILT_PC")
> -         t)
> -        (unwind-protect
> -            (let ((v (buffer-string)))
> -              (if (string= "" (buffer-string))
> -                  nil
> -                v))
> -          (kill-buffer (current-buffer))))
> -      (or (getenv "QUILT_PC")
> -          ".pc")))
> +  (or (quilt--get-config-variable "QUILT_PC")
> +      ".pc"))
>  
>  (defun quilt-find-dir (fn &optional prefn)
>    "Return the top level dir of quilt from FN."

Disclaimer: not an emacs user, so I can't test this nor other patches
in this series.

Looks good to me, although it could be the right time to fix the
description of function quilt-pc-directory. "Return the location of
patch files." looks like a copy-and-paste error from function quilt-
patches-directory in commit f7b69c58d21903baacb290840e7bed9282e357e2.

The ".pc" directory contains quilt's working state.

-- 
Jean Delvare
SUSE L3 Support