[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Quilt-dev] [PATCH 4/4] quilt.el: Fix quilt-editable when QUILT_PATC
From: |
Ondřej Lysoněk |
Subject: |
Re: [Quilt-dev] [PATCH 4/4] quilt.el: Fix quilt-editable when QUILT_PATCHES_PREFIX is set |
Date: |
Sat, 30 May 2020 18:11:41 +0200 |
Jean Delvare <jdelvare@suse.de> writes:
> On Thu, 2020-05-14 at 21:59 +0200, Ondřej Lysoněk wrote:
>> This patch fixes a bug in quilt-editable: if QUILT_PATCHES_PREFIX is
>> set, quilt-editable will always return nil, even if the file being
>> edited is part of the topmost patch.
>>
>> If QUILT_PATCHES_PREFIX is set, then 'quilt top' prints the patch name
>> as a relative path to the patch. Since in quilt-editable we're running
>> 'quilt top' from the top level directory, the printed patch path is in
>> the form $QUILT_PATCHES/patch-name.
>>
>> Later on, we're looking for a cached version of the file that we're
>> editing in .pc/. The patch directories are stored directly under .pc/,
>> rather than .pc/$QUILT_PATCHES/, so we must remove the $QUILT_PATCHES/
>> prefix from the patch path.
>>
>> Signed-off-by: Ondřej Lysoněk <olysonek@redhat.com>
>> ---
>> lib/quilt.el | 16 +++++++++++++++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/quilt.el b/lib/quilt.el
>> index a872aab..4c31bd2 100644
>> --- a/lib/quilt.el
>> +++ b/lib/quilt.el
>> @@ -58,6 +58,10 @@
>> (or (quilt--get-config-variable "QUILT_PC")
>> ".pc"))
>>
>> +(defun quilt-patches-prefix ()
>> + "Return the value of the QUILT_PATCHES_PREFIX config variable. Return nil
>> if it is unset."
>> + (quilt--get-config-variable "QUILT_PATCHES_PREFIX"))
>> +
>> (defun quilt-find-dir (fn &optional prefn)
>> "Return the top level dir of quilt from FN."
>> (if (or (not fn) (equal fn "/") (equal fn prefn))
>> @@ -178,6 +182,13 @@
>> (setq n (1+ n))))
>> (completing-read p l nil t))
>>
>> +(defun quilt--strip-patchname (pn)
>> + "Return the name of patch PN sans the path to the patches directory."
>> + (let ((patches-path (concat (quilt-patches-directory) "/")))
>> + (if (string-prefix-p patches-path pn)
>
> Considering that you only call this function when quilt-patches-
> directory is true, I think this test is pretty much guaranteed to
> succeed?
Yes, this is mostly correct. The test was meant to be a "safety check",
but to be fair, it was mostly laziness on my part. Thanks for not
letting me get away with it.
As it turns out, there is a small catch, though. The
quilt-patches-directory function currently reads only the quiltrc
files. However, quilt saves the patches directory to .pc/.quilt_patches
and uses that to read the directory name from there on. So if the
QUILT_PATCHES variable is later changed in quiltrc, the prefix stripping
would no longer work correctly.
I'll fix it in v2.
> Note that stripping the prefix "just in case" is generally not a good
> idea, as nothing prevents users from creating a patches/ subdirectory
> under the main patches/ directory (would be confusing, but is legal and
> must be supported).
Agreed. That's not what I was trying to do.
>
>> + (substring pn (length patches-path))
>> + pn)))
>> +
>> (defvar quilt-edit-top-only 't)
>> (defun quilt-editable (f)
>> "Return t if F is editable in terms of current patch. Return nil if
>> otherwise."
>> @@ -188,7 +199,10 @@
>> (if (quilt-bottom-p)
>> (quilt-cmd "applied") ; to print error message
>> (setq dirs (if quilt-edit-top-only
>> - (list (substring (quilt-cmd-to-string "top") 0 -1))
>> + (list (let ((patch (substring (quilt-cmd-to-string
>> "top") 0 -1)))
>> + (if (cquilt-patches-prefix)
>
> Please note that QUILT_PATCHES_PREFIX is considered true if not empty.
> That means that quilt will print the prefix if QUILT_PATCHES_PREFIX if
> set to "0" for example (admittedly confusing, but that's how it is
> implemented). As I read your code, this stupid setting would be
> interpreted incorrectly by emacs.
It will work correctly in this case. In eLisp, "0" is a true value. In
fact, everything non-nil is considered true [1]. The following
expression evaluates to 42:
(if "0" 42 43)
Version 2 of the patch series coming right up...
[1] https://www.gnu.org/software/emacs/manual/html_node/elisp/Conditionals.html
Ondrej
>
>> + (quilt--strip-patchname patch)
>> + patch)))
>> (cdr (cdr (directory-files (concat qd
>> (quilt-pc-directory) "/"))))))
>> (while (and (not result) dirs)
>> (if (file-exists-p (concat qd (quilt-pc-directory) "/" (car dirs)
>> "/" fn))
>
> --
> Jean Delvare
> SUSE L3 Support