[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#63337: [PATCH] package-vc--build-documentation: Fix relative @includ
From: |
Philip Kaludercic |
Subject: |
bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements |
Date: |
Wed, 10 May 2023 06:51:53 +0000 |
Joseph Turner <joseph@breatheoutbreathe.in> writes:
> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Joseph Turner <joseph@breatheoutbreathe.in>
>>> Cc: Eli Zaretskii <eliz@gnu.org>, 63337@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.
>
> Good to know, thank you! I changed `file-path' to `file-name'.
>
>>> --- 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.
>
> Good catch! Fixed.
>
>> Also, please make the comment lines shorter, preferably less than 75
>> columns.
>
> Done.
>
> Thank you!!
>
> Joseph
Ok, do you have a few example repositories that we can use to test edge-cases?
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, (continued)
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Joseph Turner, 2023/05/07
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Eli Zaretskii, 2023/05/07
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Philip Kaludercic, 2023/05/07
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Joseph Turner, 2023/05/07
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Philip Kaludercic, 2023/05/08
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Joseph Turner, 2023/05/08
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Ruijie Yu, 2023/05/08
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Joseph Turner, 2023/05/08
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Eli Zaretskii, 2023/05/09
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Joseph Turner, 2023/05/09
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements,
Philip Kaludercic <=
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Joseph Turner, 2023/05/10
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Philip Kaludercic, 2023/05/12
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Eli Zaretskii, 2023/05/12
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Philip Kaludercic, 2023/05/12
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Joseph Turner, 2023/05/13
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Philip Kaludercic, 2023/05/12
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Joseph Turner, 2023/05/13
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Philip Kaludercic, 2023/05/13
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Joseph Turner, 2023/05/13
- bug#63337: [PATCH] package-vc--build-documentation: Fix relative @include statements, Philip Kaludercic, 2023/05/13