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

[Ondřej Lysoněk]

Hi Jean,

sorry for the late reply.

Jean Delvare <jdelvare@suse.de> writes:

> 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 <olysonek@redhat.com>
>> ---
>>  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.

I'm aware. I hope you didn't have to learn eLisp just to review these
patches :).

> 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.

Yes, I'll fix it in v2.

Ondrej