all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Karl Fogel <kfogel@red-bean.com>
To: Emacs Development <emacs-devel@gnu.org>
Subject: Re: [Emacs-diffs] master b88e7c8: Make transpose-regions interactive (Bug#30343)
Date: Tue, 20 Mar 2018 16:33:52 -0500	[thread overview]
Message-ID: <87a7v2zb27.fsf@red-bean.com> (raw)
In-Reply-To: <87po44jb7w.fsf@red-bean.com>

>Richard Copley <rcopley@gmail.com> writes:
>> I agree with Eli, it seems fine to me, but I'm not sure what the
>> alternative is. If it's to move the body of the interactive spec into
>> a function in an elisp file, and replace the spec with a call to that
>> function, it seems overly complicated. (It also feel like a layering
>> violation, but that can only be true if there is a rule or guideline
>> saying so and I don't know of any.)
> 
> One solution would be to rename the C-defined function to
> Ftranspose_regions_internal (with no interactive spec), and move the
> entry point `transpose-regions' out to Lisp with the same interactive
> spec.  The Lisp function would just invoke `transpose-regions-internal'.
> 
> I'd be happy to do this, if there's consensus that it's a good solution.

Some more follow-up on this:

Richard sent an off-list reply (I didn't realize it was off-list until later) saying that the above looked like a good solution.  So I made the change (commit 3a3aa0e05), then reverted (commit 6d2e8fdd) after Eli reiterated that he didn't see a problem with the original code.  I mistakenly thought we had consensus on the change, but clearly I was wrong and further discussion is needed.

So, here is why I would prefer to see that interactive spec moved to Lisp:

Having it in C makes it less hackable.  It's a rather complex interactive spec, and I can imagine someone wanting to play around with it, perhaps with the goal of suggesting a change to the interactive behavior of `transpose-regions'.  If it were in an Elisp file, it would be much easier for people to make those experiments.  (Yes, technically one could find ways to do it anyway, but it would be harder, and in general I think reducing barriers to hackability is a Good Thing.)

Originally I thought my concerns were aesthetic, but when I thought about it more carefully, my concern is really about ease of hacking.  The same argument could apply to the other non-trivial interactive specs defined as `eval'-able strings in C (there are not many of them -- I listed them in my previous mail, quoted below).  To keep things simple, my proposal right now is just about `transpose-regions', though.

If other people have arguments for or against restoring commit 3a3aa0e05, please speak up.  I'll wait at least a week before coming to any conclusions about consensus, to ensure there's time for discussion.

Best regards,
-Karl

> (I wrote the original non-interactive Ftranspose_regions, so am perhaps
> motivated by nostalgia.  Interestingly, while checking my memory to make
> sure, I discovered that although src/ChangeLog.4 has the correct record
> for 1994-04-29, in the git history that change is recorded as being
> authored by RMS -- commit b229b8d187a.  This suggests that there was
> some information lossage in a VC-system conversion somewhere along the
> way.  I don't know if we knew that already or not.)
> 
> I looked for all other C-defined functions that have a Lisp expression
> starting with "(" for their interactive spec in the DEFUN, i.e., are not
> just using the standard code-letter strings for `interactive'.  There
> appear to be a very small number of such functions:
> 
>   - Finsert_char
>   - Ftranspose_regions
>   - Frename_buffer
>   - Fupcase_region
>   - Fdowncase_region
>   - Fdelete_file
>   - Fset_file_modes
> 
> Of these, Ftranspose_regions is certainly the most complex, with
> Fdelete_file and Frename_buffer as runners up.  The others are
> relatively simple.  Maybe it's fine to just rely on our collective
> aesthetic judgement to say that the one in Ftranspose_regions is too
> complex to be tolerated, while the others are okay.
> 
> Best regards,
> -Karl



  reply	other threads:[~2018-03-20 21:33 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20180311105533.30002.78782@vcs0.savannah.gnu.org>
     [not found] ` <20180311105534.3DAFD23CF3@vcs0.savannah.gnu.org>
2018-03-11 16:05   ` [Emacs-diffs] master b88e7c8: Make transpose-regions interactive (Bug#30343) Stefan Monnier
2018-03-11 17:23     ` Charles A. Roelli
2018-03-12  9:56       ` Leo Liu
2018-03-11 18:13     ` Eli Zaretskii
2018-03-15 20:20       ` Richard Copley
2018-03-16 15:23         ` Karl Fogel
2018-03-20 21:33           ` Karl Fogel [this message]
2018-03-20 21:46             ` Richard Copley
2018-03-21  7:22             ` Eli Zaretskii
2018-03-22  5:30               ` Karl Fogel
2018-03-22  7:12                 ` Eli Zaretskii
2018-03-22  7:15                 ` Unnecessarily moving stiff between files considered harmful (Was: [Emacs-diffs] master b88e7c8: Make transpose-regions interactive) (Bug#30343) Eli Zaretskii
2018-03-23 18:03                   ` Unnecessarily moving stiff between files considered harmful Karl Fogel
2018-03-22 13:39                 ` [Emacs-diffs] master b88e7c8: Make transpose-regions interactive (Bug#30343) Stefan Monnier
2018-03-23 18:05                   ` Karl Fogel
2018-03-25 10:03               ` Charles A. Roelli
2018-03-25 15:41                 ` Eli Zaretskii
2018-03-25 20:29                   ` Charles A. Roelli
2018-03-26 15:59                     ` Eli Zaretskii
2018-03-28 20:19                     ` Juri Linkov
2018-03-29  4:53                       ` Karl Fogel
2018-03-29 11:38                         ` Eli Zaretskii
2018-03-29 12:17                         ` Stefan Monnier

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a7v2zb27.fsf@red-bean.com \
    --to=kfogel@red-bean.com \
    --cc=emacs-devel@gnu.org \
    /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 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.