unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
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.





  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).