From: Eli Zaretskii <eliz@gnu.org>
To: Olivier Certner <ocert.dev@free.fr>
Cc: dmitry@gutov.dev, 62693@debbugs.gnu.org
Subject: bug#62693: 28.2; VC: CVS: Fix lost file reporting and enable reverting it
Date: Mon, 17 Apr 2023 20:48:27 +0300 [thread overview]
Message-ID: <83a5z69z84.fsf@gnu.org> (raw)
In-Reply-To: <2020915.n1Ql7ez4OO@ravel> (message from Olivier Certner on Mon, 17 Apr 2023 11:24:26 +0200)
> From: Olivier Certner <ocert.dev@free.fr>
> Cc: 62693@debbugs.gnu.org
> Date: Mon, 17 Apr 2023 11:24:26 +0200
>
> > The patch is AFAICS basically a complete rewrite of an important
> > function, so I don't see how I could agree applying this to the
> > release branch. Sorry. (Was the introduction of so many CL-isms
> > really necessary, btw?)
>
> This paragraph leaves me astonished.
>
> First, because it is wrong: `vs-cvs-parse-root' is not a major function, it is
> a very minor one.
I should have said "non-trivial". Sorry for potentially confusing
selection of words.
> Second, because "Was the introduction of so many CL-isms really necessary,
> btw?" is both unproductive and rude, and as such doubly inappropriate.
Dmitry asked whether it was okay to install this on the release
branch, which is currently undergoing pretest. So I looked at the
changes from the POV of the potential for unintended consequences and
regressions. This job is much harder when the code is thoroughly
rewritten, let alone uses a different macro system and style. That is
why I asked the question: I presume that if the changes were less
radical, they could perhaps have been deemed appropriate for the
release branch, or at least the chances for that were higher.
> If
> there is a policy of banishing CL forms, then say so, and even better, write
> it in the documentation. If there is already one (I'm not aware of any), then
> point me to it. You even didn't bother naming the constructs you're
> questioning. So let me review all candidates for you. The "so many" (!) CL-
> isms are of 3 kinds only: `cl-defun', `cl-labels' and `cl-ecase'. The first is
> used to bail out early from the function without complicating code. It is much
> more appropriate from a language point of view than the `throw'/`catch' it
> expands to (to readers, and implementors as well if at some point Emacs gains
> true lexical non-local exits). The second fills an Emacs Lisp gap (definition
> of internal recursive functions), so is necessary (unless you want me to
> define these functions with `defun' whereas they are only internal and not
> meant to be called standalone, or use `let' with `lambda` forms and
> funcalls?). The third is the most concise way to express the intent, even
> before `pcase'. Sure, I could use the latter and define a catch-all case with
> an internal error, but then I'll have to do that the 4 times `cl-ecase' is
> used: 4 additional lines with no added value. Does using `cl-ecase' really
> change the face of Emacs world?
>
> To be frank, I'm quite worried that I have to explain all that to an Emacs
> maintainer.
You didn't have to. That's not why I asked the question.
> I find "file name" or "directory name" even worse because they are ambiguous.
> So I've settled on using "pathname" instead (the (UNIX/Windows) "path" to some
> file, not just the filename (i.e., no slashes)). Is that OK for you?
Not really, but that's a minor point. We can always fix the
terminology later.
Thanks.
next prev parent reply other threads:[~2023-04-17 17:48 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-06 9:51 bug#62693: 28.2; VC: CVS: Fix lost file reporting and enable reverting it Olivier Certner
2023-04-12 22:48 ` Dmitry Gutov
2023-04-13 16:40 ` Olivier Certner
2023-04-14 22:59 ` Dmitry Gutov
2023-04-15 9:28 ` Eli Zaretskii
2023-04-17 9:24 ` Olivier Certner
2023-04-17 17:48 ` Eli Zaretskii [this message]
2023-04-17 20:10 ` Olivier Certner
2023-04-19 0:54 ` Dmitry Gutov
2023-04-19 8:10 ` Olivier Certner
2023-04-20 8:55 ` Eli Zaretskii
2023-04-20 9:42 ` Olivier Certner
2023-04-20 10:22 ` Dmitry Gutov
2023-04-17 8:04 ` Olivier Certner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=83a5z69z84.fsf@gnu.org \
--to=eliz@gnu.org \
--cc=62693@debbugs.gnu.org \
--cc=dmitry@gutov.dev \
--cc=ocert.dev@free.fr \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).