load-path contains directories or directory names?

load-path contains directories or directory names?

[Stephen Leake]

As part of the file completion code I'm working on, I need to build an
absolute file name from a directory and a non-directory. That's either:

(concat dir name)

or

(concat dir "/" name)

depending on whether `dir' is a directory (ends in '/') or a directory
name.

Here `dir' is taken from load-path, or a similar user-provided path. The
doc string for `load-path' says it consists of "directory names". In
emacs -Q, `load-path' indeed has no elements that end in '/'.

However, after (package-initialize), `load-path' had elements that end
in '/'. This comes from package-autoload-ensure-default-file in
package.el, which adds lines like this to each package's autoload:

"(add-to-list 'load-path (or (file-name-directory #$) (car load-path)))\n"

Is this a bug, or should code that uses `load-path' tolerate this? It's
easy to do:

(concat (file-name-as-directory dir) name)

but I'd rather avoid redundant code if possible. I discovered this when
I added a cl-assert to check the precondition that the path contains
only directory names.

On the other hand, since `load-path' can be modified by users, perhaps
code must be tolerant anyway.

-- 
-- Stephe

Re: load-path contains directories or directory names?

[Eli Zaretskii]

> From: Stephen Leake <stephen_leake@stephe-leake.org>
> Date: Thu, 22 Oct 2015 23:26:54 -0500
> 
> Here `dir' is taken from load-path, or a similar user-provided path. The
> doc string for `load-path' says it consists of "directory names". In
> emacs -Q, `load-path' indeed has no elements that end in '/'.
> 
> However, after (package-initialize), `load-path' had elements that end
> in '/'. This comes from package-autoload-ensure-default-file in
> package.el, which adds lines like this to each package's autoload:
> 
> "(add-to-list 'load-path (or (file-name-directory #$) (car load-path)))\n"
> 
> Is this a bug, or should code that uses `load-path' tolerate this?

It's a bug, patches are welcome to fix it.

> On the other hand, since `load-path' can be modified by users, perhaps
> code must be tolerant anyway.

That's a separate issue.  Defensive programming is frequently a good
thing.  But IMO we shouldn't _force_ Lisp packages into defensive
programming, so I think Emacs itself should only put directory names
into load-path.

Re: load-path contains directories or directory names?

[Stephen Leake]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stephen Leake <stephen_leake@stephe-leake.org>
>> Date: Thu, 22 Oct 2015 23:26:54 -0500
>> 
>> Here `dir' is taken from load-path, or a similar user-provided path. The
>> doc string for `load-path' says it consists of "directory names". In
>> emacs -Q, `load-path' indeed has no elements that end in '/'.
>> 
>> However, after (package-initialize), `load-path' had elements that end
>> in '/'. This comes from package-autoload-ensure-default-file in
>> package.el, which adds lines like this to each package's autoload:
>> 
>> "(add-to-list 'load-path (or (file-name-directory #$) (car load-path)))\n"
>> 
>> Is this a bug, or should code that uses `load-path' tolerate this?
>
> It's a bug, patches are welcome to fix it.

Pushed in 4d3a595d8d3e6a111399e9f1c7dd3c3c30184e61

-- 
-- Stephe

Re: load-path contains directories or directory names?

[Michael Heerdegen]

Stephen Leake <stephen_leake@stephe-leake.org> writes:

> As part of the file completion code I'm working on, I need to build an
> absolute file name from a directory and a non-directory. That's either:
>
> (concat dir name)
>
> or
>
> (concat dir "/" name)

I try to avoid string operations on file names.

In the above case, using

  (expand-file-name name dir)

seems cleaner to me.  And for DIR (docstring) even "both the
directory name and a directory’s file name are accepted".


Michael.

Re: load-path contains directories or directory names?

[John Wiegley]

>>>>> Michael Heerdegen <michael_heerdegen@web.de> writes:

> I try to avoid string operations on file names.
> In the above case, using
>   (expand-file-name name dir)
> seems cleaner to me.

+1

John

Re: load-path contains directories or directory names?

[Stephen Leake]

"John Wiegley" <johnw@newartisans.com> writes:

>>>>>> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
>> I try to avoid string operations on file names.
>> In the above case, using
>>   (expand-file-name name dir)
>> seems cleaner to me.

(info "(elisp) Directory Names" says to use:

(concat DIRNAME RELFILE)

So if I'm iterating thru a path that is defined to contain directory
names (as `load-path' is, but see other email), this code is correct:

(concat dir name)

However, if the path contains directory file names (as
`load-path' is implemented), then this code is correct:

(concat (file-name-as-directory dir) filename)


However, `file-name-as-directory' tolerates ending slashes or not in
`dir', so the latter code is robust against people putting the wrong
things in the path.

Same for `expand-file-name' of course.

-- 
-- Stephe

Re: load-path contains directories or directory names?

[Michael Heerdegen]

Stephen Leake <stephen_leake@stephe-leake.org> writes:

> (info "(elisp) Directory Names" says to use:
> (concat DIRNAME RELFILE)

Interesting.  Didn't expect that.


Michael.

Re: load-path contains directories or directory names?

[Eli Zaretskii]

Potential bikeshedding alert!

> From: Stephen Leake <stephen_leake@stephe-leake.org>
> Date: Sat, 24 Oct 2015 16:46:19 -0500
> Cc: emacs-devel@gnu.org
> 
> "John Wiegley" <address@hidden> writes:
> 
> >>>>>> Michael Heerdegen <address@hidden> writes:
> >
> >> I try to avoid string operations on file names.
> >> In the above case, using
> >>   (expand-file-name name dir)
> >> seems cleaner to me.

I'm with John on this one, see below for reasons.

> (info "(elisp) Directory Names" says to use:
> 
> (concat DIRNAME RELFILE)

No, it says you CAN use that.  And it continues:

  Be sure to verify that the file name is relative before doing that.  If
  you use an absolute file name, the results could be syntactically
  invalid or refer to the wrong file.

which all but explains why using 'concat' is less desirable than using
'expand-file-name': the code will be more complicated because it needs
to call another function if the directory name doesn't end in a slash.

Also, 'expand-file-name' transparently treats a nil correctly.

In addition, on MS-Windows 'expand-file-name' converts all backslashes
to forward slashes, thus producing nicer and more "standard" results.

> So if I'm iterating thru a path that is defined to contain directory
> names (as `load-path' is, but see other email), this code is correct:
> 
> (concat dir name)
> 
> However, if the path contains directory file names (as
> `load-path' is implemented), then this code is correct:
> 
> (concat (file-name-as-directory dir) filename)
> 
> However, `file-name-as-directory' tolerates ending slashes or not in
> `dir', so the latter code is robust against people putting the wrong
> things in the path.
> 
> Same for `expand-file-name' of course.

Exactly.  So I think 'expand-file-name' is a (slightly) better
alternative in these cases.

Re: load-path contains directories or directory names?

[John Wiegley]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

>> (info "(elisp) Directory Names" says to use:
>> 
>> (concat DIRNAME RELFILE)

> No, it says you CAN use that.

Eli, should we remove that mention in the elisp docs, to avoid confusion?

Telling people they could use `concat' is something (a) an experienced Lisper
would already know, and (b) an inexperienced Lisper shouldn't consider as a
reasonable alternative to `expand-file-name'.

John

Re: load-path contains directories or directory names?

[Eli Zaretskii]

> From: "John Wiegley" <johnw@newartisans.com>
> Cc: Stephen Leake <stephen_leake@stephe-leake.org>,  michael_heerdegen@web.de,  emacs-devel@gnu.org
> Date: Sun, 25 Oct 2015 12:02:32 -0700
> 
> >>>>> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> (info "(elisp) Directory Names" says to use:
> >> 
> >> (concat DIRNAME RELFILE)
> 
> > No, it says you CAN use that.
> 
> Eli, should we remove that mention in the elisp docs, to avoid confusion?
> 
> Telling people they could use `concat' is something (a) an experienced Lisper
> would already know, and (b) an inexperienced Lisper shouldn't consider as a
> reasonable alternative to `expand-file-name'.

IMO, either remove and replace with 'expand-file-name', or describe
both with a preference to 'expand-file-name'.

Re: load-path contains directories or directory names?

[John Wiegley]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

> IMO, either remove and replace with 'expand-file-name', or describe both
> with a preference to 'expand-file-name'.

Oh, it doesn't already talk about expand-file-name? It should state the
solution in terms of `expand-file-name', and then maybe mention in a note that
it's implemented as a platform-path-correct use of `concat' under the hood. Or
not mention `concat' at all, either way.

Stephe, would you be willing to submit a documentation patch?

John

RE: load-path contains directories or directory names?

[Drew Adams]

> > IMO, either remove and replace with 'expand-file-name', or describe both
> > with a preference to 'expand-file-name'.
> 
> Oh, it doesn't already talk about expand-file-name? It should state the
> solution in terms of `expand-file-name', and then maybe mention in a note
> that it's implemented as a platform-path-correct use of `concat' under the hood.
> Or not mention `concat' at all, either way.

I think the doc about file and directory names _should_ mention
`concat', as that is the gotcha: users are often used to using
it to concatenate strings, and some naturally think of using it
for constructing file names.  (File names are strings, but not
all strings are file names.)

I think the doc should explicitly point out that `concat' has no
notion of file names and their syntax subtleties, and that
file-name construction and decomposition are the raison d'etre
for the functions described in (elisp) File Names and its subnodes.

But definitely, at least node Directory Names should not suggest
that users use `concat'.  On that I agree with you, and that's
the main point here.

Re: load-path contains directories or directory names?

[Stephen Leake]

John Wiegley <johnw@newartisans.com> writes:

>>>>>> Eli Zaretskii <eliz@gnu.org> writes:
>
>> IMO, either remove and replace with 'expand-file-name', or describe both
>> with a preference to 'expand-file-name'.
>
> Oh, it doesn't already talk about expand-file-name? It should state the
> solution in terms of `expand-file-name', and then maybe mention in a note that
> it's implemented as a platform-path-correct use of `concat' under the hood. Or
> not mention `concat' at all, either way.
>
> Stephe, would you be willing to submit a documentation patch?

Done; see other branch of this thread


-- 
-- Stephe

Re: load-path contains directories or directory names?

[Stephen Leake]

Eli Zaretskii <eliz@gnu.org> writes:

> In addition, on MS-Windows 'expand-file-name' converts all backslashes
> to forward slashes, thus producing nicer and more "standard" results.

This raises this issue of whether elements in `load-path' can have
backslashes (or other syntax on VMS? not that we support VMS anymore,
but I'm feeling nostalgic :).

I always assume `load-path' has canonical Emacs file name format, but
the doc string doesn't say that, and on Windows it would seem "natural"
to add directory file names with backslashes.

-- 
-- Stephe

Re: load-path contains directories or directory names?

[Eli Zaretskii]

> From: Stephen Leake <stephen_leake@stephe-leake.org>
> Cc: michael_heerdegen@web.de,  emacs-devel@gnu.org
> Date: Mon, 26 Oct 2015 08:46:21 -0500
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > In addition, on MS-Windows 'expand-file-name' converts all backslashes
> > to forward slashes, thus producing nicer and more "standard" results.
> 
> This raises this issue of whether elements in `load-path' can have
> backslashes (or other syntax on VMS? not that we support VMS anymore,
> but I'm feeling nostalgic :).

I didn't write that wrt load-path (or other similar lists), I wrote
that wrt constructing file names by combining a directory name with a
file's basename.

> I always assume `load-path' has canonical Emacs file name format, but
> the doc string doesn't say that, and on Windows it would seem "natural"
> to add directory file names with backslashes.

We can add that requirement to the doc string of load-path (and its
ilk), if we think it's important.  Although using expand-file-name
will work even with backslashes, of course.