emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests whi


From: João Távora
Subject: Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
Date: Sat, 19 Jan 2019 03:18:03 +0000
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux)

Hi Alan

Alan Mackenzie <address@hidden> writes:

> I'm answering you with a top post (with no bottom) because our exchange
> has become so voluminous.

In my understanding, it's because I made specific questions that you
decline to answer.

> In my turn, I will not mess with the electric-... functions.  If you
> prefer, I will also not mess with electric-tests.el.  I did not realise
> you felt proprietorial over it.

It's not about the file at all!  I am not "proprietorial" over a
computer file.  Many people have added tests to that file.  It's about
the behaviour that you destroyed.  The file is guarding that behviour!
That's what automated tests are for!

> And I request you to tone down your aggressiveness.

Alan, I am being assertive, not aggressive.  If I don't speak now, it's
much harder down the line to convince you of the wrong path you're
taking it.  So I take a firm stance.  If I don't speak now, this
"temporary" thing will be like the other "temporary workaround" from 6
month ago.

> The aggressiveness is entirely on the side of electric-....  I merely
> want CC Mode to continue working as it has done for several decades.
> You are continually attacking it.

For the last half-decade it has worked consistently with
electric-pair-mode.  Then you get a bug report about electric-pair-mode
and a part of cc-mode and decide to fix.  Alright.  You do it by nixing
electric-pair-mode's much more substantially in other parts.  And you do
this in the face of evidence that there are alternatives in Emacs.

> CC Mode and I are under constant siege, but just want to be left in
> peace.  Yet I get from you continually "change CC Mode this way,
> introduce c-self-insert-command, change CC Mode the other way", on and
> on and on, ceaselessly.  All for your convenience.

You could very well have left CC Mode untouched for all I care, and just
tell Beatrix she could find any other alternative.  This alternative
exists Alan, I've given you actual evidence, not vapourware!

> I insist you respect the need for the correct functioning of
> c-electric-brace and friends.  And that you cease false insinuations
> such as c-auto-newline being merely a "corner case" - it is an integral
> part of CC Mode's functionality, and it will remain supported, and it
> will remain.

I don't ask that cease supporting it!  You're completely misrepresenting
me.  Keep it for those who find no problem with it, for whatever older
version of Emacs you want to support, by all means keep it, I mean it no
harm.

> Beatrix Klebe's bug was about c-auto-newline.  It was not about
> electric-layout-mode.  It is now fixed.

No. It was about c-auto-newline's interaction with electric-pair-mode.

It would much better that she disable electric-pair-mode completely when
working with c-auto-newline.

electric-layout-mode would have been an alternative to getting

  int main () <press { here>

turned into

  int main () {
    
  }

That's all it was about.  You decided to fix this by reimplementing
electric-pair-mode inside cc-cmds.el.  You failed Alan, there are many
cases that you missed.  And even if you didn't, you are using internal
functions of elec-pair.el that may change in the future.

> You want electric-layout etc., to be the same for every major mode?
> Then please create interfaces to them which are usable by every mode,
> including CC Mode (see above).

It is _already_ usable by CC Mode: it does not (yet) use the
c-style-vars, but that's a detail that some users don't care about.  But
it will use them soon.  There is code in this list and in the Emacs repo
that demonstrates this.  What can I do to explain this better show the
actual code that makes this happen.  Try it!

> I'm not happy with your response to my request for recipes which show
> how CC Mode supposedly doesn't work with electric-pair-mode.  That
> around 80 tests fail shows nothing - (at least some of) the tests are
> broken.

I suggested that you read the names of the failed tests and then
type

  M-x ert-describe-test RET name-of-failed-test RET

to understand what the behaviour is that used to work and now doesn't.

I'm sorry If I sound exasperated but I've told you this already 3 times
in the past to no avail!

> You suggest that I should put in the hard work of extracting
> recipes from your tests myself.

No! The tests have generated docstrings! Use M-x ert-describe-test.
What's the difference between what it types out in the screen and what I
can write here?

Here's another failure:

M-x ert-describe-test RET 
electric-pair-angle-brackets-everywhere-at-point-2-in-c++-mode

   electric-pair-angle-brackets-everywhere-at-point-2-in-c++-mode is a
   test defined in ‘electric-tests.el’.
    
   Electricity test in a ‘c++-mode’ buffer.
    
   Start with point at 2 in a 2-char-long buffer
   like this one:
    
     |<>|   (buffer start and end are denoted by ‘|’)
    
   Now call this:
    
   (lambda nil
     (electric-pair-mode 1))
    
    
   Ensure the following bindings:
    
   ’((electric-pair-pairs
      (60 . 62)))
    
   Now press the key for: >
    
   The buffer’s contents should stay:
    
     |<>|
    
   , and point should be at 3.

Your change makes <>> instead.

Fix this and all the other ones.  Incidentally, most of the test
failures are inside strings and comments, so maybe they are easy fixes.

> The false assumption that these tests make is that they can rely on
> certain settings in post-self-insert-hook.  Any major mode is at liberty
> to bind or set this hook, and as pointed out above, CC Mode _must_ bind
> this to nil in c-electric-brace, etc..  Would you please amend these
> tests to take this into account.
>
> And I ask you, have you tried using c-auto-newline?  It is easy to set
> up (a single line in your c-mode-common-hook, or interactively C-c C-a).
> The CC Mode style system takes care of the rest of the setup most of the
> time.  If not, why not?

I prefer electric-layout-mode since I also use it for other languages.
Is this not a valid reason?  Can not I _have_ this preference?  Am I
allowed, not definitely hating it, and not even dreaming of asking you
to change it, to *not particularly enjoy* the CC Mode style system?

> I do not attack electric-layout-mode.  I merely note that it would be
> less good (if it could work) than CC Mode's own features in the context
> of CC Mode.  For example, c-auto-newline handles different braces
> differently, depending on their syntactic context.  electric-layout-mode
> does not.  electric-layout-mode is not needed in CC Mode.

For now I just want braces to generate newlines. That's very easy to
achieve with electric-layout-mode. For braces you can just add a
function that calls (c-brace-newlines (c-point-syntax)) to
electric-layout-rules and proceeds accordingly.  I have already shown a
draft of this function that works pretty well.  Have you tried it?  That
will read the current style vars and do the right thing.

If I ever become interested in inserting newlines for other delimiter
types, I can code it myself with and consult the style vars (or you
could add functions such as c-brace-newlines that make it even easier
for me).

For the moment, though, I am not personally interested in that.

> You say that things you've had working since Emacs 24.4 are now not
> working.  Do you mean electric-layout-mode?  If so, that "working" was
> an illusion, not reality.  I hope I've said enough to explain why that
> minor mode can't work in CC Mode, and that a better alternative exists.
>
> Sometime in the next few days, I'm going to revert your patch to
> cc-cmds.el. I earnestly request you to modify electric-tests.el to
> take account of this.

Fine, revert it.  Can you give me one good objective reason why adding

(defun c-self-insert-command (arg)
   (let (post-self-insert-hook)
      (self-insert-command arg)))

isn't a better solution, if still temporary?  It'll save some lines off
your original change and enable me, the tests, and other users to just
override it with add-function or advice-add or something.  Then pack the
two or three other occasions where you call electric-pair-mode directly
into functions so that they can also be overriden.  Surely you are not
worried about the runtime cost of calling a single function on a
keypress.

If you agree to this I can show you a patch -- no "sarcastic" variables
involved.

If you don't, and you still insist on this, I honestly give up.  I'll
have to resort to some crazy ad-hoc solution that does the same, just to
stay out of the precious cc-cmds.el file. I can advise the
c-electric-whatever functions to save the variables
post-self-insert-hook and electric-pair-mode, set electric-pair-mode to
nil, and then advise self-insert-command to recover them.  This could be
put in some package in Emacs (see below my sig).  It would be silly: but
you would make it for a completely needless reasons, the only way.

Goodbye Alan,
João

;;; foo.el --- The one and only. Makes electric.el great again, tests and all.  
-*- lexical-binding: t; -*-

;; Copyright (C) 2019  Joao Tavora

;;; Code:

(defvar foo--saved-post-self-insert-hook nil)
(defvar foo--saved-electric-pair-mode nil)
(defvar foo--recover-self-insert-command nil)

(defun foo--save-self-insert-command (oldfun &rest args)
  (let* ((foo--saved-post-self-insert-hook post-self-insert-hook)
         (foo--saved-electric-pair-mode electric-pair-mode)
         (foo--recover-self-insert-command t)
         (electric-pair-mode nil))
    (apply oldfun args)))

(advice-add 'c-electric-brace :around #'foo--save-self-insert-command)
(advice-add 'c-electric-lt-gt :around #'foo--save-self-insert-command)
(advice-add 'c-electric-paren :around #'foo--save-self-insert-command)

(advice-add 'self-insert-command :around
            (lambda (oldfun &rest args)
              (if foo--recover-self-insert-command
                  (let* ((post-self-insert-hook 
foo--saved-post-self-insert-hook)
                         (electric-pair-mode foo--saved-electric-pair-mode)
                         (foo--recover-self-insert-command nil))
                    (apply oldfun args))
                (apply oldfun args)))
            '((name . foo--recover-self-insert-command)))

(provide 'foo)
;;; foo.el ends here





reply via email to

[Prev in Thread] Current Thread [Next in Thread]