Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Michael Albinus]

eliz@gnu.org (Eli Zaretskii) writes:

> diff --git a/src/fileio.c b/src/fileio.c
> index 6d505fd..d6b3e7b 100644
> --- a/src/fileio.c
> +++ b/src/fileio.c
> @@ -751,9 +751,10 @@ For that reason, you should normally use `make-temp-file' instead.  */)
>
>  DEFUN ("directory-append", Fdirectory_append, Sdirectory_append, 1, MANY, 0,
>         doc: /* Append COMPONENTS to DIRECTORY and return the resulting string.
> -COMPONENTS must be a list of strings.  DIRECTORY or the non-final
> -elements in COMPONENTS may or may not end with a slash -- if they don't
> -end with a slash, a slash will be inserted before contatenating.
> +COMPONENTS must be strings.
> +DIRECTORY or the non-final elements in COMPONENTS may or may not end
> +with a slash -- if they don't end with a slash, a slash will be
> +inserted before contatenating.
>  usage: (record DIRECTORY &rest COMPONENTS) */)
>    (ptrdiff_t nargs, Lisp_Object *args)
>  {

FTR, I believe COMPONENTS might also contain nil elements, which should
be ignored.

Furthermore, we have

(directory-append "foo/" "/bar") => "foo//bar"

I would expect "foo/bar", meaning that all leading slashes in COMPONENTS
are removed.

Best regards, Michael.

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Lars Ingebrigtsen]

Michael Albinus <michael.albinus@gmx.de> writes:

> FTR, I believe COMPONENTS might also contain nil elements, which should
> be ignored.

I'm not sure I follow.  You want to extend the interface to allow nil
elements?  I'm not sure that would be very useful?

> Furthermore, we have
>
> (directory-append "foo/" "/bar") => "foo//bar"
>
> I would expect "foo/bar", meaning that all leading slashes in COMPONENTS
> are removed.

Well, I think that's over into DWIM-land again (which is what
expand-file-name does, and this function deliberately doesn't do).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Michael Albinus]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> FTR, I believe COMPONENTS might also contain nil elements, which should
>> be ignored.
>
> I'm not sure I follow.  You want to extend the interface to allow nil
> elements?  I'm not sure that would be very useful?

It's rather a nice to have. Sometimes, I have a list of file name
fragments, and I hope to avoid the checks for being non-empty strings
...

But I can do this in my code.

>> Furthermore, we have
>>
>> (directory-append "foo/" "/bar") => "foo//bar"
>>
>> I would expect "foo/bar", meaning that all leading slashes in COMPONENTS
>> are removed.
>
> Well, I think that's over into DWIM-land again (which is what
> expand-file-name does, and this function deliberately doesn't do).

expand-file-name and file-name-as-directory are functions which call a
handler, for example Tramp. directory-append is much cheaper, and if it
could do this string handling, I could avoid those functions. It would
be more performant.

Best regards, Michael.

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Lars Ingebrigtsen]

Michael Albinus <michael.albinus@gmx.de> writes:

> It's rather a nice to have. Sometimes, I have a list of file name
> fragments, and I hope to avoid the checks for being non-empty strings
> ...
>
> But I can do this in my code.

Thinking about it a bit more, I can see use cases where ignoring nil/""
would be useful (without making the function less difficult to reason
about).  (And it's analogous to `concat', besides.)  So I think I'll go
ahead and make that change...

>>> Furthermore, we have
>>>
>>> (directory-append "foo/" "/bar") => "foo//bar"
>>>
>>> I would expect "foo/bar", meaning that all leading slashes in COMPONENTS
>>> are removed.
>>
>> Well, I think that's over into DWIM-land again (which is what
>> expand-file-name does, and this function deliberately doesn't do).
>
> expand-file-name and file-name-as-directory are functions which call a
> handler, for example Tramp. directory-append is much cheaper, and if it
> could do this string handling, I could avoid those functions. It would
> be more performant.

The reason `directory-append' is useful as a function at all (instead of
just using `concat') is that we're very vague about the difference
between a directory name and a file name in Emacs.  That is, we treat
"/tmp/" and "/tmp" as equivalent in more than 99% of cases when dealing
with directories.  This has naturally led to people setting
`foo-directory' variables etc to something ending with a slash or not
arbitrarily, which again means that you can't just use `concat' to
construct file names.

But "/bar.txt" does not happen naturally as a leaf file name, so I think
it'd be counter-productive to have `directory-append' do anything
particular about the start of file name components.

By the way -- I'm not sure that `directory-append' is the best function
name here...  `file-name-concat' would perhaps be more natural?  But
it's really about directories...  `directory-name-concat'?  Hm.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Stefan Monnier]

> By the way -- I'm not sure that `directory-append' is the best function
> name here...  `file-name-concat' would perhaps be more natural?  But
> it's really about directories...  `directory-name-concat'?  Hm.

I vote for `file-name-concat` (while some of the arguments are
directory names, the last one usually/often isn't and neither is the
return value).


        Stefan

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I vote for `file-name-concat` (while some of the arguments are
> directory names, the last one usually/often isn't and neither is the
> return value).

OK; renamed.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Kévin Le Gouguec]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> I vote for `file-name-concat` (while some of the arguments are
>> directory names, the last one usually/often isn't and neither is the
>> return value).
>
> OK; renamed.

Should etc/NEWS be updated as well?

diff --git a/etc/NEWS b/etc/NEWS
index f1635ae2e6..0d1abfd852 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -3109,8 +3109,8 @@ The former is now declared obsolete.
 * Lisp Changes in Emacs 28.1
 
 +++
-*** New function 'directory-append'.
-This appends a file name to a directory name and returns the result.
+*** New function 'file-name-concat'.
+This appends path components to a directory name and returns the result.
 
 +++
 *** New function 'split-string-shell-command'.

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Lars Ingebrigtsen]

Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:

> Should etc/NEWS be updated as well?

Yup; thanks.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Andreas Schwab]

On Jul 25 2021, Kévin Le Gouguec wrote:

> diff --git a/etc/NEWS b/etc/NEWS
> index f1635ae2e6..0d1abfd852 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -3109,8 +3109,8 @@ The former is now declared obsolete.
>  * Lisp Changes in Emacs 28.1
>  
>  +++
> -*** New function 'directory-append'.
> -This appends a file name to a directory name and returns the result.
> +*** New function 'file-name-concat'.
> +This appends path components to a directory name and returns the result.

s/path/file name/

This should probably emphasize the fact that it ignores file name
handlers.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Kévin Le Gouguec]

Andreas Schwab <schwab@linux-m68k.org> writes:

>> -*** New function 'directory-append'.
>> -This appends a file name to a directory name and returns the result.
>> +*** New function 'file-name-concat'.
>> +This appends path components to a directory name and returns the result.
>
> s/path/file name/

Fair enough; I shied away from the three-word compound, but I see now
that we have "(elisp) File Name Components".

diff --git a/etc/NEWS b/etc/NEWS
index 0d1abfd852..ce34236500 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -3110,7 +3110,8 @@ The former is now declared obsolete.
 
 +++
 *** New function 'file-name-concat'.
-This appends path components to a directory name and returns the result.
+This appends file name components to a directory name and returns the
+result.
 
 +++
 *** New function 'split-string-shell-command'.


> This should probably emphasize the fact that it ignores file name
> handlers.

The NEWS entry specifically, or the docstring and/or info entry?

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Lars Ingebrigtsen]

Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:

> Fair enough; I shied away from the three-word compound, but I see now
> that we have "(elisp) File Name Components".

Thanks; applied to Emacs 28.

>> This should probably emphasize the fact that it ignores file name
>> handlers.
>
> The NEWS entry specifically, or the docstring and/or info entry?

I'm not entirely confident that it'll keep ignoring file name handlers,
so the less said the better.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Michael Albinus]

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

>>> This should probably emphasize the fact that it ignores file name
>>> handlers.
>>
>> The NEWS entry specifically, or the docstring and/or info entry?
>
> I'm not entirely confident that it'll keep ignoring file name handlers,
> so the less said the better.

If you want file-name-concat being well optimized, you shall not use
file name handlers:

--8<---------------cut here---------------start------------->8---
(benchmark
 1000
 (let (file-name-handlers)
   (concat (file-name-as-directory "/tmp") "foo")))
=> "Elapsed time: -0.000004s"

(benchmark
 1000
 (let nil
   (concat (file-name-as-directory "/tmp") "foo")))
=> "Elapsed time: 0.000049s"
--8<---------------cut here---------------end--------------->8---

It's not heavy in terms of absolute time, but still a factor of 10.

Best regards, Michael.

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Lars Ingebrigtsen]

Michael Albinus <michael.albinus@gmx.de> writes:

> It's not heavy in terms of absolute time, but still a factor of 10.

Yup.  I mean, I don't foresee any reason to add a file name handler to
this function, but they have a tendency to ... happen anyway.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Michael Albinus]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>>>> I would expect "foo/bar", meaning that all leading slashes in COMPONENTS
>>>> are removed.
>>>
>>> Well, I think that's over into DWIM-land again (which is what
>>> expand-file-name does, and this function deliberately doesn't do).
>>
>> expand-file-name and file-name-as-directory are functions which call a
>> handler, for example Tramp. directory-append is much cheaper, and if it
>> could do this string handling, I could avoid those functions. It would
>> be more performant.
>
> The reason `directory-append' is useful as a function at all (instead of
> just using `concat') is that we're very vague about the difference
> between a directory name and a file name in Emacs.  That is, we treat
> "/tmp/" and "/tmp" as equivalent in more than 99% of cases when dealing
> with directories.  This has naturally led to people setting
> `foo-directory' variables etc to something ending with a slash or not
> arbitrarily, which again means that you can't just use `concat' to
> construct file names.
>
> But "/bar.txt" does not happen naturally as a leaf file name, so I think
> it'd be counter-productive to have `directory-append' do anything
> particular about the start of file name components.

So (file-name-concat x y) is roughly equivalent to

(let (file-name-handler-alist)
  (concat (file-name-as-directory x) y))

Perhaps this could be mentioned somewhere in the manual? This would also
precise, that leading slashes in COMPONENTS are kept.

Best regards, Michael.

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Lars Ingebrigtsen]

Michael Albinus <michael.albinus@gmx.de> writes:

> So (file-name-concat x y) is roughly equivalent to
>
> (let (file-name-handler-alist)
>   (concat (file-name-as-directory x) y))

Yes, I think so.

> Perhaps this could be mentioned somewhere in the manual?

In the file-name-concat bit in the manual?  I'm not sure that would be
all that informative for the readers, really...  (But it would explain
things to people who did do a concat/file-name-as-directory thing before
this function existed, so perhaps mentioning this in NEWS would make
sense.)

> This would also precise, that leading slashes in COMPONENTS are kept.

I'm not sure why we'd mention anything about leading slashes here at
all?  Or slashes in the middle of components, for that matter:

(file-name-concat "/tmp/" "//foo////bar")
=> "/tmp///foo////bar"

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master e315544: ; * src/fileio.c (Fdirectory_append): Doc fix.

[Michael Albinus]

Lars Ingebrigtsen <larsi@gnus.org> writes:

Hi Lars,

>> Perhaps this could be mentioned somewhere in the manual?
>
> In the file-name-concat bit in the manual?  I'm not sure that would be
> all that informative for the readers, really...  (But it would explain
> things to people who did do a concat/file-name-as-directory thing before
> this function existed, so perhaps mentioning this in NEWS would make
> sense.)

Well, I let it to you :-) I'm one of the guys doing concat/
file-name-as-directory thing before, and I know now what's up.

At least it shall be said somewhere that there's no file name handler
involved, as Andreas has mentioned already.

Best regards, Michael.