bug#56809: file-name-with-extension: Improve docstring.

bug#56809: file-name-with-extension: Improve docstring.

[Damien Cassou]

[-- Attachment #1: Type: text/plain, Size: 239 bytes --]

Tags: patch

Hi,

I found the docstring of file-name-with-extension very confusing so I
wrote another one.

Best

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-files.el-file-name-with-extension-Improve-docst.patch --]
[-- Type: text/patch, Size: 1167 bytes --]

From dccdf3dec9a82d4ce05e9978b762d9538e11bdfd Mon Sep 17 00:00:00 2001
From: Damien Cassou <damien@cassou.me>
Date: Thu, 28 Jul 2022 08:27:45 +0200
Subject: [PATCH] ; * lisp/files.el (file-name-with-extension): Improve
 docstring.

---
 lisp/files.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 37ed796a68..39b8586a94 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -5119,13 +5119,13 @@ file-name-extension
             "")))))
 
 (defun file-name-with-extension (filename extension)
-  "Set the EXTENSION of a FILENAME.
+  "Return a string resulting from the concatenation of FILENAME and EXTENSION.
 The extension (in a file name) is the part that begins with the last \".\".
 
-Trims a leading dot from the EXTENSION so that either \"foo\" or
-\".foo\" can be given.
+If EXTENSION doesn't start with a \".\", one is inserted anyway
+between FILENAME and EXTENSION.
 
-Errors if the FILENAME or EXTENSION are empty, or if the given
+Signal an error if FILENAME or EXTENSION are empty, or if the given
 FILENAME has the format of a directory.
 
 See also `file-name-sans-extension'."
-- 
2.36.0

bug#56809: file-name-with-extension: Improve docstring.

[Eli Zaretskii]

> From: Damien Cassou <damien@cassou.me>
> Date: Thu, 28 Jul 2022 08:31:24 +0200
> 
> I found the docstring of file-name-with-extension very confusing so I
> wrote another one.

Thanks.  It definitely needed its wording and its grammar fixed.

I've now fixed that on the emacs-28 release branch.  The modified doc
string says this:

    "Return FILENAME modified to have the specified EXTENSION.
  The extension (in a file name) is the part that begins with the last \".\".
  This function removes any existing extension from FILENAME, and then
  appends EXTENSION to it.

  EXTENSION may include the leading dot; if it doesn't, this function
  will provide it.

  It is an error if FILENAME or EXTENSION is empty, or if FILENAME
  is in the form of a directory name according to `directory-name-p'.

  See also `file-name-sans-extension'."

Looks OK?

bug#56809: file-name-with-extension: Improve docstring.

[Damien Cassou]

Eli Zaretskii <eliz@gnu.org> writes:
> Looks OK?

looks better than my version :-). The only thing I don't really like is

> "Return FILENAME modified to…

Most readers will know that FILENAME is not going to be modified but the
phrasing is still confusing in my opinion. I would prefer a version
around the word "concatenate" or similar. Here is another version:

  Concatenate FILENAME without its extension and EXTENSION.

It doesn't say that the result of the concatenation is returned but I'm
fine having it implied.

Your call.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

bug#56809: file-name-with-extension: Improve docstring.

[Eli Zaretskii]

> From: Damien Cassou <damien@cassou.me>
> Cc: 56809@debbugs.gnu.org
> Date: Thu, 28 Jul 2022 10:35:01 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > Looks OK?
> 
> looks better than my version :-). The only thing I don't really like is
> 
> > "Return FILENAME modified to…
> 
> Most readers will know that FILENAME is not going to be modified but the
> phrasing is still confusing in my opinion.

Why confusing?  And what do you mean by "will know that FILENAME is
not going to be modified"?

> I would prefer a version
> around the word "concatenate" or similar. Here is another version:
> 
>   Concatenate FILENAME without its extension and EXTENSION.

I don't want to say how the function does its job: that's not what a
doc string should describe.  ("Concatenate" is also inaccurate,
because if EXTENSION lacks the leading period, that's not really
what's going on there.)  So please help me understand why "modified"
is problematic, and let's take it from there.

Thanks.

bug#56809: file-name-with-extension: Improve docstring.

[Damien Cassou]

Eli Zaretskii <eliz@gnu.org> writes:
>> From: Damien Cassou <damien@cassou.me>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>> Looks OK?
>> 
>> looks better than my version :-). The only thing I don't really like is
>> 
>>> "Return FILENAME modified to…
>> 
>> Most readers will know that FILENAME is not going to be modified but the
>> phrasing is still confusing in my opinion.
>
> Why confusing?


To me, "Return FILENAME modified" means that FILENAME is going to be
modified but that is not true: FILENAME will still reference the same
place in memory and this place won't be changed either. Said
differently: I understand the sentence as "there is going to be some
side-effects" even though there are none.


> And what do you mean by "will know that FILENAME is not going to be
> modified"?


I meant that reasonably-knowledgeable elisp developers will know that
the content of FILENAME isn't going to change after this function is
executed. As a result, a docstring starting with "Return FILENAME
modified to" will be correctly understood by most developers so you
should take my feedback with a grain of salt. That being said, I like
when things are clear and not subject to wrong interpretations so I
would prefer a different phrasing.


>>   Concatenate FILENAME without its extension and EXTENSION.
> I don't want to say how the function does its job


To me "Concatenate" doesn't say how the function is implemented (which
would be something like "Call `concatenate' to…") but what the user
should expect the result to look like (i.e., "the approximate
concatenation of 2 strings").


> "Concatenate" is also inaccurate, because if EXTENSION lacks the
> leading period, that's not really what's going on there.


I agree it is inaccurate. The inaccuracy is acceptable to me in the
first sentence as this sentence is only meant to give an idea of what
the function is supposed to do. The rest of the docstring explains the
details and why it's not a simple concatenation. I think the
misunderstanding here is just because "Concatenate" is an implementation
detail for you (so you expect it to correctly reflect the
implementation) while it is just a user-visible result to me (so I don't
really care if the implementation doesn't really concatenate).


As a conclusion, I'm fine with the docstring and you should merge it and
do more important stuff rather than discussing unimportant details with
me 😃. I'm sorry I made you loose your time.

Best

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

bug#56809: file-name-with-extension: Improve docstring.

[Eli Zaretskii]

> From: Damien Cassou <damien@cassou.me>
> Cc: 56809@debbugs.gnu.org
> Date: Thu, 28 Jul 2022 11:32:55 +0200
> 
> >>> "Return FILENAME modified to…
> >> 
> >> Most readers will know that FILENAME is not going to be modified but the
> >> phrasing is still confusing in my opinion.
> >
> > Why confusing?
> 
> To me, "Return FILENAME modified" means that FILENAME is going to be
> modified but that is not true: FILENAME will still reference the same
> place in memory and this place won't be changed either. Said
> differently: I understand the sentence as "there is going to be some
> side-effects" even though there are none.

Well, the "return" part was supposed to prevent such an
interpretation.  Moreover, that sentence is just the summary; the doc
string goes on to say

  This function removes any existing extension from FILENAME, and then
  appends EXTENSION to it.

I could have the first sentence to say something like

  Return a file name made from the base name of FILENAME and EXTENSION.

But is this really better?  We'd need at least to explain what is a
"base name" (and it's also a bit inaccurate, since "base name"
generally means without the leading directories).

> I agree it is inaccurate. The inaccuracy is acceptable to me in the
> first sentence as this sentence is only meant to give an idea of what
> the function is supposed to do. The rest of the docstring explains the
> details and why it's not a simple concatenation.

Well, the same is true for the "modified" part, isn't it?

> As a conclusion, I'm fine with the docstring and you should merge it and
> do more important stuff rather than discussing unimportant details with
> me 😃. I'm sorry I made you loose your time.

No need to be sorry, this is part of my job as a maintainer.

Thanks.

bug#56809: file-name-with-extension: Improve docstring.

[Visuwesh]

[வியாழன் ஜூலை 28, 2022] Damien Cassou wrote:

>> "Return FILENAME modified to…
>
> Most readers will know that FILENAME is not going to be modified but the
> phrasing is still confusing in my opinion. I would prefer a version
> around the word "concatenate" or similar. Here is another version:
>
>   Concatenate FILENAME without its extension and EXTENSION.

I find this confusing.  Eli's version looks better (the other one which
mentions basename is no good either).

bug#56809: file-name-with-extension: Improve docstring.

[Damien Cassou]

Eli Zaretskii <eliz@gnu.org> writes:
> Well, the "return" part was supposed to prevent such an
> interpretation.  Moreover, that sentence is just the summary.

Feel free to merge your patch Eli, it's great. I don't want us to keep
discussing on such a tiny detail.

Thank you.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

bug#56809: file-name-with-extension: Improve docstring.

[Eli Zaretskii]

> From: Damien Cassou <damien@cassou.me>
> Cc: 56809@debbugs.gnu.org
> Date: Thu, 28 Jul 2022 14:01:56 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> > Well, the "return" part was supposed to prevent such an
> > interpretation.  Moreover, that sentence is just the summary.
> 
> Feel free to merge your patch Eli, it's great. I don't want us to keep
> discussing on such a tiny detail.

I already merged it to the emacs-28 (a.k.a. "release") branch.  It
will be merged to master soon enough.

But if someone can come up with a better wording, mine is not carved
in stone.

Thanks, closing.