Jean Delvare <[email protected]> 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 <[email protected]> >> --- >> 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 _______________________________________________ Quilt-dev mailing list [email protected] https://lists.nongnu.org/mailman/listinfo/quilt-dev
