* bug#56809: file-name-with-extension: Improve docstring.
@ 2022-07-28 6:31 Damien Cassou
2022-07-28 7:49 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Damien Cassou @ 2022-07-28 6:31 UTC (permalink / raw)
To: 56809
[-- 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* bug#56809: file-name-with-extension: Improve docstring.
2022-07-28 6:31 bug#56809: file-name-with-extension: Improve docstring Damien Cassou
@ 2022-07-28 7:49 ` Eli Zaretskii
2022-07-28 8:35 ` Damien Cassou
0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2022-07-28 7:49 UTC (permalink / raw)
To: Damien Cassou; +Cc: 56809
> 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?
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#56809: file-name-with-extension: Improve docstring.
2022-07-28 7:49 ` Eli Zaretskii
@ 2022-07-28 8:35 ` Damien Cassou
2022-07-28 8:57 ` Eli Zaretskii
2022-07-28 10:34 ` Visuwesh
0 siblings, 2 replies; 9+ messages in thread
From: Damien Cassou @ 2022-07-28 8:35 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 56809
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#56809: file-name-with-extension: Improve docstring.
2022-07-28 8:35 ` Damien Cassou
@ 2022-07-28 8:57 ` Eli Zaretskii
2022-07-28 9:32 ` Damien Cassou
2022-07-28 10:34 ` Visuwesh
1 sibling, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2022-07-28 8:57 UTC (permalink / raw)
To: Damien Cassou; +Cc: 56809
> 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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#56809: file-name-with-extension: Improve docstring.
2022-07-28 8:57 ` Eli Zaretskii
@ 2022-07-28 9:32 ` Damien Cassou
2022-07-28 9:40 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Damien Cassou @ 2022-07-28 9:32 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 56809
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#56809: file-name-with-extension: Improve docstring.
2022-07-28 9:32 ` Damien Cassou
@ 2022-07-28 9:40 ` Eli Zaretskii
2022-07-28 12:01 ` Damien Cassou
0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2022-07-28 9:40 UTC (permalink / raw)
To: Damien Cassou; +Cc: 56809
> 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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#56809: file-name-with-extension: Improve docstring.
2022-07-28 8:35 ` Damien Cassou
2022-07-28 8:57 ` Eli Zaretskii
@ 2022-07-28 10:34 ` Visuwesh
1 sibling, 0 replies; 9+ messages in thread
From: Visuwesh @ 2022-07-28 10:34 UTC (permalink / raw)
To: Damien Cassou; +Cc: Eli Zaretskii, 56809
[வியாழன் ஜூலை 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).
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#56809: file-name-with-extension: Improve docstring.
2022-07-28 9:40 ` Eli Zaretskii
@ 2022-07-28 12:01 ` Damien Cassou
2022-07-28 13:34 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Damien Cassou @ 2022-07-28 12:01 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 56809
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#56809: file-name-with-extension: Improve docstring.
2022-07-28 12:01 ` Damien Cassou
@ 2022-07-28 13:34 ` Eli Zaretskii
0 siblings, 0 replies; 9+ messages in thread
From: Eli Zaretskii @ 2022-07-28 13:34 UTC (permalink / raw)
To: Damien Cassou; +Cc: 56809-done
> 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.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-07-28 13:34 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-28 6:31 bug#56809: file-name-with-extension: Improve docstring Damien Cassou
2022-07-28 7:49 ` Eli Zaretskii
2022-07-28 8:35 ` Damien Cassou
2022-07-28 8:57 ` Eli Zaretskii
2022-07-28 9:32 ` Damien Cassou
2022-07-28 9:40 ` Eli Zaretskii
2022-07-28 12:01 ` Damien Cassou
2022-07-28 13:34 ` Eli Zaretskii
2022-07-28 10:34 ` Visuwesh
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.