GNU bug report logs - #63337
[PATCH] package-vc--build-documentation: Fix relative @include statements

Previous Next

Full log

Message #44 received at 63337 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 63337 <at> debbugs.gnu.org, philipk <at> posteo.net
Subject: Re: bug#63337: [PATCH] package-vc--build-documentation: Fix
 relative @include statements
Date: Tue, 09 May 2023 07:36:42 +0300

> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 63337 <at> debbugs.gnu.org
> Date: Mon, 08 May 2023 12:05:51 -0700
> 
> Note about the following two lines:
> 
> + (file-path (expand-file-name file (package-desc-dir pkg-desc)))
> + (default-directory (expand-file-name (file-name-directory file-path)))
> 
> (package-desc-dir pkg-desc) may return a relative path with or without a
> directory, e.g. "doc/manual.org" or "manual.org". In the latter case,
> (file-name-directory "manual.org") would return `nil' and
> (expand-file-name nil) would signal an error.
> 
> Therefore, in the `file-path' `let'-binding, we first expand the return
> value of (package-desc-dir pkg-desc) to ensure that it contains a directory.

Please don't use "path" for anything that is not a PATH-style list of
directory: the GNU Coding Standards frown on such usage.  We use
file-name instead.  For the same reasons, please don't give your
variables names that include "path" unless they are lists of
directories.

> --- a/lisp/emacs-lisp/package-vc.el
> +++ b/lisp/emacs-lisp/package-vc.el
> @@ -376,14 +376,17 @@ Package specs are loaded from trusted package archives."
>  FILE can be an Org file, indicated by its \".org\" extension,
>  otherwise it's assumed to be an Info file."
>    (let* ((pkg-name (package-desc-name pkg-desc))
> -         (default-directory (package-desc-dir pkg-desc))
> +         (file-path (expand-file-name file (package-desc-dir pkg-desc)))
> +         ;; `let'-bind `default-directory' to the directory containing the .org or .info FILE
> +         ;; so that makeinfo can resolve relative @include statements in the docs directory.
> +         (default-directory (expand-file-name (file-name-directory file-path)))

There should be no reason to call expand-file-name in the last line,
since the argument of file-name-directory is already expanded.

Also, please make the comment lines shorter, preferably less than 75
columns.

Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.