Re: master c9cb59b 2/2: * etc/TODO: Entry about converting to defvar-keymap.

Re: master c9cb59b 2/2: * etc/TODO: Entry about converting to defvar-keymap.

[Eli Zaretskii]

> +** Convert defvar foo-mode-map to defvar-keymap
> +Verify the conversion by comparing the value of the keymap before
> +converting it and after (you can see the value in 'C-h v').

Really?  What are the benefits of doing such a conversion?  I see only
a lot of code churn and a risk of breaking things that are perfectly
fine as they are.

And that goes for many of the changes in this direction you made
lately, btw: why are we making all those changes for purely stylistic
reasons?  It was always our policy not to do that except when fixing
real bugs in the same area.

I'd prefer that the energy and the time you have to work on Emacs
would be invested to make more important changes: fixing bugs, adding
new features, etc.  We don't have enough resources to expend them on
changes like these ones.

TIA

Re: master c9cb59b 2/2: * etc/TODO: Entry about converting to defvar-keymap.

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> +** Convert defvar foo-mode-map to defvar-keymap
>> +Verify the conversion by comparing the value of the keymap before
>> +converting it and after (you can see the value in 'C-h v').
>
> Really?  What are the benefits of doing such a conversion?  I see only
> a lot of code churn and a risk of breaking things that are perfectly
> fine as they are.

I think this is a good entry-level exercise for someone who is looking
to start contributing to Emacs.  To minimize the risk, I added the
instruction that one should check the value before and after.

Please feel free to revert it if you disagree.

> And that goes for many of the changes in this direction you made
> lately, btw: why are we making all those changes for purely stylistic
> reasons?

Yes, I'm not making any friends doing that, am I?  :-)

Personally, I do such things when I don't have the time or energy to
invest in anything significant.  I can come up with other ways to
procrastinate, of course.

Re: master c9cb59b 2/2: * etc/TODO: Entry about converting to defvar-keymap.

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Fri, 10 Dec 2021 15:37:32 +0000
> Cc: emacs-devel@gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> +** Convert defvar foo-mode-map to defvar-keymap
> >> +Verify the conversion by comparing the value of the keymap before
> >> +converting it and after (you can see the value in 'C-h v').
> >
> > Really?  What are the benefits of doing such a conversion?  I see only
> > a lot of code churn and a risk of breaking things that are perfectly
> > fine as they are.
> 
> I think this is a good entry-level exercise for someone who is looking
> to start contributing to Emacs.

I think it shouldn't be done, not by beginners, not by anyone.

> Please feel free to revert it if you disagree.

I will, unless Lars disagrees.

> > And that goes for many of the changes in this direction you made
> > lately, btw: why are we making all those changes for purely stylistic
> > reasons?
> 
> Yes, I'm not making any friends doing that, am I?  :-)
> 
> Personally, I do such things when I don't have the time or energy to
> invest in anything significant.  I can come up with other ways to
> procrastinate, of course.

Can I convince you to do more useful things when that happens?  How
about reading a random section in the manual, or a random set of doc
strings, and making them more clear?

Re: master c9cb59b 2/2: * etc/TODO: Entry about converting to defvar-keymap.

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> Please feel free to revert it if you disagree.
>
> I will, unless Lars disagrees.

Are we talking about reverting the TODO item?  That's fine with me,
because I think it's a bit early to mandate rewriting all the keymaps,
but I do think that should be our long term goal.

Normally I'm against code churn, but that's mainly because it makes it
hard to do code archaeology to determine what the meaning of a
particular piece of code was.  It's often unclear what some piece of
code was meant to achieve, so you have to look at the other changes made
at the same time, and then try to piece things together.

That's not really the case with keymaps, though -- there's little logic
to unravel, so if they change syntactically, it's no big deal.

The advantages of the conversion has to outweigh that "no big deal",
though, but I think in this case they do:

1) In the future, we can envision programmers only having to deal with
one single way to represent key bindings in Emacs Lisp code.  It would
be nice if that were the case, but to get there, we have to covert the
keymaps.

2) The temptation to rewrite a map when doing adding a key or two is
great, but it makes that single commit harder to read.  So converting
maps as a separate commit makes things easier to read, not harder.

3) Today the vast majority of the keys are set as such:

  (define-key calc-mode-map "dp" 'calc-show-plain)

While doing conversion to defvar-keymap, we'd also convert to #', which
gives us byte compilation support for mis-spelled commands.  I found a
couple of bindings that pointed to commands that doesn't exist when
doing the few conversions that I've done.  It's mainly a help when
adding new bindings, of course, but people will cargo cult the previous
line when adding new bindings, and won't get help from the byte compiler.

4) The shorter syntax is less error-prone in general, since you don't
repeat the keymap name in every line.  This means that people that hack
on this in the future will be able to move slightly faster if the maps
have been converted.

5) If we never get to where C-i/TAB can be round-tripped reliably by
other means (Stefan M had some ideas, but I think we stalled?), I have a
patch waiting to fix this by other means -- but not for `define-key', of
course.  So `defvar-keymap'd keymaps will be displayed the same as the
way they were defined in `C-h b', which is nice.

So I'm all for Stefan K continuing to convert keymaps while
procrastinating.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master c9cb59b 2/2: * etc/TODO: Entry about converting to defvar-keymap.

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> 2) The temptation to rewrite a map when doing adding a key or two is
> great, but it makes that single commit harder to read.  So converting
> maps as a separate commit makes things easier to read, not harder.

Agreed.

(IMHO, often it is better to first make the cleanup, and then the
logical change in a separate commit.  My impression is that this style
is not often used in Emacs development.)

> While doing conversion to defvar-keymap, we'd also convert to #', which
> gives us byte compilation support for mis-spelled commands.  I found a
> couple of bindings that pointed to commands that doesn't exist when
> doing the few conversions that I've done.

In addition to this, and keeping in mind also the other points you make,
I think getting some experience with defvar-keymap will help stabilize
and work out any problems in its interface well in advance of Emacs 29.

For example, Bug#52374 was found by me poking at the keymaps in
re-builder.el.  Although presumably that bug would have been eventually
been found anyways, it's always nice to discover such things early.
I have the feeling that there might exist more bugs and opportunities
for improvement that we haven't yet noticed.

Re: master c9cb59b 2/2: * etc/TODO: Entry about converting to defvar-keymap.

[dick]

EZ> I see only a lot of code churn and a risk of breaking things that
EZ> are perfectly fine as they are.

To most gatekeepers, spaghetti code is "perfectly fine" if no one
complains.  And so long as the spaghetti is of their own making, let
newcomers to the code spin their tires trying to modify it.
Incomprehensibility is an excellent, natural defense.

Churn should never be a deterrent; cleanliness is next to godliness.
Any programmer with his salt can see that.

Re: master c9cb59b 2/2: * etc/TODO: Entry about converting to defvar-keymap.

[Alan Mackenzie]

Hello, Dick.

On Fri, Dec 10, 2021 at 10:59:23 -0500, dick wrote:
> EZ> I see only a lot of code churn and a risk of breaking things that
> EZ> are perfectly fine as they are.

> To most gatekeepers, spaghetti code is "perfectly fine" if no one
> complains.  And so long as the spaghetti is of their own making, let
> newcomers to the code spin their tires trying to modify it.
> Incomprehensibility is an excellent, natural defense.

> Churn should never be a deterrent; cleanliness is next to godliness.
> Any programmer with his salt can see that.

Really, must you keep on doing this?

It seems the only purpose of your post here was to aggravate Eli.  Any
policy direction can be countered by a trite meaningless phrase like
"cleanliness is next to godliness"; it seems likely, had Eli expressed
the opposite direction, you would have chastised him for encouraging the
wasting of time, in an equally trite and unpleasant manner.

Please stop posting like this.  It doesn't help anybody.

If you want to contribute positively to Emacs, in whatever manner, I'm
sure you'd be welcome.  But you know how the project works, what it's
doing, and likely why; you're not going to be able to change these
things.  It you don't like them, why stick around?

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: master c9cb59b 2/2: * etc/TODO: Entry about converting to defvar-keymap.

[Po Lu]

dick <dick.r.chiang@gmail.com> writes:

> To most gatekeepers, spaghetti code is "perfectly fine" if no one
> complains.  And so long as the spaghetti is of their own making, let
> newcomers to the code spin their tires trying to modify it.
> Incomprehensibility is an excellent, natural defense.

There is nothing spaghetti-like or "incomprehensible" about a
traditional keymap definition, which looks something like this:

(defvar xwidget-webkit-mode-map
  (let ((map (make-sparse-keymap)))
    (define-key map "g" 'xwidget-webkit-browse-url)
    (define-key map "a" 'xwidget-webkit-adjust-size-dispatch)
    (define-key map "b" 'xwidget-webkit-back)
    (define-key map "f" 'xwidget-webkit-forward)
    (define-key map "r" 'xwidget-webkit-reload)
    (define-key map "\C-m" 'xwidget-webkit-insert-string)
    (define-key map "w" 'xwidget-webkit-current-url)
    (define-key map "+" 'xwidget-webkit-zoom-in)
    (define-key map "-" 'xwidget-webkit-zoom-out)
    (define-key map "e" 'xwidget-webkit-edit-mode)
    (define-key map "\C-r" 'xwidget-webkit-isearch-mode)
    (define-key map "\C-s" 'xwidget-webkit-isearch-mode)
    (define-key map "H" 'xwidget-webkit-browse-history)

    ;;similar to image mode bindings
    (define-key map (kbd "SPC")                 'xwidget-webkit-scroll-up)
    (define-key map (kbd "S-SPC")               'xwidget-webkit-scroll-down)
    (define-key map (kbd "DEL")                 'xwidget-webkit-scroll-down)

    (define-key map [remap scroll-up]           'xwidget-webkit-scroll-up-line)
    (define-key map [remap scroll-up-command]   'xwidget-webkit-scroll-up)

    (define-key map [remap scroll-down]         'xwidget-webkit-scroll-down-line)
    (define-key map [remap scroll-down-command] 'xwidget-webkit-scroll-down)

    (define-key map [remap forward-char]        'xwidget-webkit-scroll-forward)
    (define-key map [remap backward-char]       'xwidget-webkit-scroll-backward)
    (define-key map [remap right-char]          'xwidget-webkit-scroll-forward)
    (define-key map [remap left-char]           'xwidget-webkit-scroll-backward)
    (define-key map [remap previous-line]       'xwidget-webkit-scroll-down-line)
    (define-key map [remap next-line]           'xwidget-webkit-scroll-up-line)

    ;; (define-key map [remap move-beginning-of-line] 'image-bol)
    ;; (define-key map [remap move-end-of-line]       'image-eol)
    (define-key map [remap beginning-of-buffer] 'xwidget-webkit-scroll-top)
    (define-key map [remap end-of-buffer]       'xwidget-webkit-scroll-bottom)
    map)
  "Keymap for `xwidget-webkit-mode'.")