unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
@ 2019-01-17 14:57 João Távora
  2019-01-17 16:43 ` Alan Mackenzie
  0 siblings, 1 reply; 27+ messages in thread
From: João Távora @ 2019-01-17 14:57 UTC (permalink / raw)
  To: emacs-devel, Alan Mackenzie

Hi Alan,

Please revert this change ASAP:

commit 54f297904e0c641fcfd81f16e9a87177124a27be
Author: Alan Mackenzie
Date:   Thu Jan 17 12:51:40 2019 +0000

    Temporarily comment out CC Mode from tests which are incompatible
with it.

    * tests/electric-tests (electric-pair-test-for): comment out
c++-mode from the
    list of modes to be used in tests.
    (electric-pair-whitespace-chomping-2-at-point-4-in-c++-mode-in-strings)
    (ert-deftest electric-layout-int-main-kernel-style)
    (ert-deftest electric-layout-int-main-allman-style): Comment out.

I thought we had agreed that the way to "work around" other people's
unit tests, even if temporarily, is to work in a separate git branch.

The other electric-pair-test that I disabled 6 months ago, that was one that
also temporary, is till there. But now you destroyed even the "expected
failure" mark.  Why?? Is the test passing unexpectedly?

@@ -396,10 +397,10 @@ whitespace-chomping-2
 ;; mode will sort this out eventually, using some new e-p-m machinery.
 ;; See
 ;; https://lists.gnu.org/archive/html/emacs-devel/2018-06/msg00535.html
-(setf
- (ert-test-expected-result-type
-  (ert-get-test
'electric-pair-whitespace-chomping-2-at-point-4-in-c++-mode-in-strings))
- :failed)
+;; (setf
+;;  (ert-test-expected-result-type
+;;   (ert-get-test
'electric-pair-whitespace-chomping-2-at-point-4-in-c++-mode-in-strings))
+;;  :failed)

But this is much more intrusive.  In particular

;; Tests commented out, since C Mode does not use
;; electric-layout-mode.  2019-01-17, ACM

C Mode doesn't use electric-layout mode, but a user can surely
decide we wants to use it in c-mode, can he not??  These tests
pass fine currently.

Please revert this fix and lets discuss why you need to disable tests.

If we come to the conclusion that some tests are asserting unreasonable
expectations about the functionality you develop, we can disable them on a
case by case basis!

If on the other hand, if you need to do some work "temporarily", then
the best way to do it without disturbing other people's developments
is to do it in a separate branch.

João



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-17 14:57 Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it João Távora
@ 2019-01-17 16:43 ` Alan Mackenzie
  2019-01-17 17:57   ` João Távora
  0 siblings, 1 reply; 27+ messages in thread
From: Alan Mackenzie @ 2019-01-17 16:43 UTC (permalink / raw)
  To: João Távora; +Cc: emacs-devel

Hello, João.

On Thu, Jan 17, 2019 at 14:57:04 +0000, João Távora wrote:
> Hi Alan,

> Please revert this change ASAP:

> commit 54f297904e0c641fcfd81f16e9a87177124a27be
> Author: Alan Mackenzie
> Date:   Thu Jan 17 12:51:40 2019 +0000

>     Temporarily comment out CC Mode from tests which are incompatible
> with it.

That would leave lots of failed tests in make check.  People have
already remarked on those failures, implicitly asking me to fix them.

> I thought we had agreed that the way to "work around" other people's
> unit tests, even if temporarily, is to work in a separate git branch.

My understanding, from a previous encounter, was that having no failing
unit tests was of a high priority.  I've only commented a little bit
out, I haven't made any permanent, unreverseable changes.

> The other electric-pair-test that I disabled 6 months ago, that was one that
> also temporary, is till there. But now you destroyed even the "expected
> failure" mark.  Why?? Is the test passing unexpectedly?

With that test in, I got the error message: "No test named
`electric-pair-whitespace-chomping-2-at-point-4-in-c++-mode-in-strings'",
and no other tests were performed, leaving an electric-tests.log file 86
bytes long.  That's why I commented it out.  This may be some glitch in
the testing system.

> @@ -396,10 +397,10 @@ whitespace-chomping-2
>  ;; mode will sort this out eventually, using some new e-p-m machinery.
>  ;; See
>  ;; https://lists.gnu.org/archive/html/emacs-devel/2018-06/msg00535.html
> -(setf
> - (ert-test-expected-result-type
> -  (ert-get-test
> 'electric-pair-whitespace-chomping-2-at-point-4-in-c++-mode-in-strings))
> - :failed)
> +;; (setf
> +;;  (ert-test-expected-result-type
> +;;   (ert-get-test
> 'electric-pair-whitespace-chomping-2-at-point-4-in-c++-mode-in-strings))
> +;;  :failed)

> But this is much more intrusive.  In particular

> ;; Tests commented out, since C Mode does not use
> ;; electric-layout-mode.  2019-01-17, ACM

> C Mode doesn't use electric-layout mode, but a user can surely
> decide he wants to use it in c-mode, can he not??

No.  Certainly not at the moment.

> These tests pass fine currently.

When I ran them, there were lots of failures, because the tests assumed
electric-layout-mode was active.

> Please revert this fix and lets discuss why you need to disable tests.

It's not a fix, it's a temporary workaround.

Anyhow, surely the implementation of Emacs should not be constrained by
its tests?  Rather the tests should test the chosen implementation.

> If we come to the conclusion that some tests are asserting unreasonable
> expectations about the functionality you develop, we can disable them on a
> case by case basis!

I would have done that, indeed tried to do that, but the lack of
documentation of electric-pair-test-for, electric-pair-define-test-form
and so on, many of them with 8, 9 or more parameters, made that too
difficult, given the urgency I felt to produce a workaround.

> If on the other hand, if you need to do some work "temporarily", then
> the best way to do it without disturbing other people's developments
> is to do it in a separate branch.

I fixed bug #33794[*] on master, as I always do with CC Mode bugs (and most
other sorts of bugs, too).  That fix is, in principle, sound.  I didn't
discover the problems with the unit tests till later (though I admit I
should have done).

[*] That is, Beatrix Klebe's bug about CC Mode's auto-newlines not
working with electric-pair-mode.

Anyhow, apologies, and all that, but I don't want to spend any more time
on this topic today or until tomorrow evening, since I've got an exam
coming up tomorrow.  But I promise I'll get back to you (including
answering your other post) either late tomorrow or on Saturday.

> João

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-17 16:43 ` Alan Mackenzie
@ 2019-01-17 17:57   ` João Távora
  2019-01-17 18:55     ` João Távora
  2019-01-18 17:54     ` Alan Mackenzie
  0 siblings, 2 replies; 27+ messages in thread
From: João Távora @ 2019-01-17 17:57 UTC (permalink / raw)
  To: Alan Mackenzie, Beatrix Klebe; +Cc: emacs-devel

Hello again, Alan

On Thu, Jan 17, 2019 at 4:54 PM Alan Mackenzie <acm@muc.de> wrote:

> >     Temporarily comment out CC Mode from tests which are incompatible
> > with it.
>
> That would leave lots of failed tests in make check.  People have
> already remarked on those failures, implicitly asking me to fix them.

Yes. But the correct fix was to revert the commit that broke
the tests, not this volkswagenesque atrocity.

> My understanding, from a previous encounter, was that having no failing
> unit tests was of a high priority.  I've only commented a little bit
> out, I haven't made any permanent, unreverseable changes.

Then you'll certainly be OK if I revert your two commits myself:
the commit that broke the tests and the commit that disables the test.
That is certainly not permanent or unreversable, too!

> With that test in, I got the error message: "No test named
> `electric-pair-whitespace-chomping-2-at-point-4-in-c++-mode-in-strings'",
> and no other tests were performed, leaving an electric-tests.log file 86
> bytes long.  That's why I commented it out.  This may be some glitch in
> the testing system.

I see, you disabled all c++ tests, *sigh*

> > C Mode doesn't use electric-layout mode, but a user can surely
> > decide he wants to use it in c-mode, can he not??
>
> No.  Certainly not at the moment.

No? Will you come to his house and tell him personally not to
M-x electric-layout-mode? What if that's what he _prefers_?

> > These tests pass fine currently.
>
> When I ran them, there were lots of failures, because the tests assumed
> electric-layout-mode was active.

The tests should activate electric-layout-mode temporarily if needed and
the revert to the previous situation.  If they aren't doing this, that is what
needs to be fixed, but it works for me and I don't have electric-layout-mode,
too.

> > Please revert this fix and lets discuss why you need to disable tests.
>
> It's not a fix, it's a temporary workaround.

With all due respect, I've heard that one before many many times.

Do you realize that you've broken many many things about
electric-pair-mode?

For example Beatrix, and me and everyone else will now lose
the default "autobalancing" functionality of electric-pair-mode,
which is its best feature! Autobalance used to work identically in
most modes, but now you broke that. Is it worth breaking so
many things just for fixing a much smaller case?

Just one example of one of the many cases you broke.

electric-pair-mixed-paren-3-at-point-4-in-c++-mode is a test defined
in `electric-tests.elc'.

Electricity test in a `c++-mode' buffer.

Start with point at 4 in a 7-char-long buffer
like this one:

  |  (])  |   (buffer start and end are denoted by `|')

Now call this:

#[nil "\300\301!\207"
      [electric-pair-mode 1]
      2]


Now press the key for: (

The buffer's contents should become:

  |  (()])  |

, and point should be at 5.

[back]

You turn this into (())]) so now I have two unbalanced parenthesis types
in my buffer.

> Anyhow, surely the implementation of Emacs should not be constrained by
> its tests?  Rather the tests should test the chosen implementation.

Really Alan, you completely broke electric-pair-mode by trying to
reimplement it in cc-mode.el where it was working perfectly but for a small
glitch (which really isn't its fault).

If you're going to reimplement it, reimplement *all* of it (of course you might
come to the conclusion that its best to use the existing implementation...)

> > If we come to the conclusion that some tests are asserting unreasonable
> > expectations about the functionality you develop, we can disable them on a
> > case by case basis!
>
> I would have done that, indeed tried to do that, but the lack of
> documentation of electric-pair-test-for, electric-pair-define-test-form
> and so on, many of them with 8, 9 or more parameters, made that too
> difficult, given the urgency I felt to produce a workaround.

If this is indeed urgent, you can add some conditional check to the
offending code using 'ert-running-test'.

> > If on the other hand, if you need to do some work "temporarily", then
> > the best way to do it without disturbing other people's developments
> > is to do it in a separate branch.
>
> I fixed bug #33794[*] on master, as I always do with CC Mode bugs (and most
> other sorts of bugs, too).  That fix is, in principle, sound.  I didn't
> discover the problems with the unit tests till later (though I admit I
> should have done).

It's *not* sound if it breaks tests. At least not without first arguing
that the tests are placing unreasonable expectations on your code.

> [*] That is, Beatrix Klebe's bug about CC Mode's auto-newlines not
> working with electric-pair-mode.

Beatrix Klebe, can, as a workaround, use electric-layout-mode
perfectly well for her use case (even though you insist she can't)
Until you can sort out c++-mode.

It's much easier than creating this mess that really interferes
with other's peoples work.

> Anyhow, apologies, and all that, but I don't want to spend any more time
> on this topic today or until tomorrow evening, since I've got an exam
> coming up tomorrow.  But I promise I'll get back to you (including
> answering your other post) either late tomorrow or on Saturday.

Alright. Good luck for your exam.

In the meantime. I will have to fix this differently. I will add a
temporary variable that I can set to have sane C++ behaviour
in the meantime. I will set this variable when running tests, so
the test people will see correct results.

João



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-17 17:57   ` João Távora
@ 2019-01-17 18:55     ` João Távora
  2019-01-18 17:54     ` Alan Mackenzie
  1 sibling, 0 replies; 27+ messages in thread
From: João Távora @ 2019-01-17 18:55 UTC (permalink / raw)
  To: Alan Mackenzie, Beatrix Klebe; +Cc: emacs-devel

> > Anyhow, apologies, and all that, but I don't want to spend any more time
> > on this topic today or until tomorrow evening, since I've got an exam
> > coming up tomorrow.  But I promise I'll get back to you (including
> > answering your other post) either late tomorrow or on Saturday.
>
> Alright. Good luck for your exam.
>
> In the meantime. I will have to fix this differently. I will add a
> temporary variable that I can set to have sane C++ behaviour
> in the meantime. I will set this variable when running tests, so
> the test people will see correct results.

This is done in 4bdc03746915c36313b33b6998b855eef514cdd1 and
be505726b68d407a44fdcd9c7ac1ef722398532d.

The fix for bug#33794 can now be disabled by setting a variable
to t.  The variable's default value is nil, so the fix is in place
and so are all the problems that come with it.

Meanwhile, users of c-mode and c++-mode can:

   (setq c--disable-fix-of-bug-33794 t)

if they don't care about the fix for bug#33794, which only
affects users of c-toggle-auto-newline, off by default.

Thanks again,
João

commit be505726b68d407a44fdcd9c7ac1ef722398532d
Date:   Thu Jan 17 18:47:00 2019 +0000

    Fix electric-pair-tests by disabling bug#33794's fix with a variable

    The variable c--disable-fix-of-bug-33794, which should be removed in
    the short term in favor of a permanent solution, is introduced.

    It is bound to nil by default.  This means that breakage is still
    happening in actual c-mode and c++-mode usage, though the tests no
    longer show it.

    To get around this breakage, put

       (setq c--disable-fix-of-bug-33794 t)

    In your init file.  Evidently, you will lose the fix for bug#33794,
    but that only affects a small corner case of c-toggle-auto-newline,
    which is not turned on by default.

    See https://lists.gnu.org/archive/html/emacs-devel/2019-01/msg00360.html
    for more information.

    * lisp/progmodes/cc-cmds.el (c--disable-fix-of-bug-33794): New
    variable.
    (c--with-post-self-insert-hook-maybe): New macro.
    (c-electric-pound, c-electric-brace, c-electric-slash)
    (c-electric-star, c-electric-semi&comma, c-electric-colon)
    (c-electric-lt-gt, c-electric-paren): Use it.
    (c-electric-paren, c-electric-brace): Check
    c--disable-fix-of-bug-33794.

    * test/lisp/electric-tests.el (c--disable-fix-of-bug-33794):
    Forward declare.
    (electric-pair-test-for)
    (electric-layout-int-main-kernel-style)
    (electric-modes-in-c-mode-with-self-insert-command): Use it.

commit 4bdc03746915c36313b33b6998b855eef514cdd1
Date:   Thu Jan 17 18:08:01 2019 +0000

    Revert "Temporarily comment out CC Mode from tests..."

    This reverts commit 54f297904e0c641fcfd81f16e9a87177124a27be.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-17 17:57   ` João Távora
  2019-01-17 18:55     ` João Távora
@ 2019-01-18 17:54     ` Alan Mackenzie
  2019-01-18 19:55       ` João Távora
                         ` (3 more replies)
  1 sibling, 4 replies; 27+ messages in thread
From: Alan Mackenzie @ 2019-01-18 17:54 UTC (permalink / raw)
  To: João Távora; +Cc: emacs-devel

Hello, João.

First of all, I will say that I take exception, great exception, to your
means of expressing yourself, both in your post and (even more so) in
the commit you made.  It verges on sarcasm, and is totally over the top
and uncalled for.

I will say no more about this from now on.

On Thu, Jan 17, 2019 at 17:57:24 +0000, João Távora wrote:
> Hello again, Alan

> On Thu, Jan 17, 2019 at 4:54 PM Alan Mackenzie <acm@muc.de> wrote:

> > >     Temporarily comment out CC Mode from tests which are incompatible
> > > with it.

> > That would leave lots of failed tests in make check.  People have
> > already remarked on those failures, implicitly asking me to fix them.

> Yes. But the correct fix was to revert the commit that broke
> the tests, not this volkswagenesque atrocity.

The tests are now broken.  They make assumptions about CC Mode's workings
which don't hold.  Why can't we cooperate to fix them?

> > My understanding, from a previous encounter, was that having no failing
> > unit tests was of a high priority.  I've only commented a little bit
> > out, I haven't made any permanent, unreverseable changes.

> Then you'll certainly be OK if I revert your two commits myself:

I am not at all OK about this.

> the commit that broke the tests and the commit that disables the test.
> That is certainly not permanent or unreversable, too!

No, but the amount of work it now requires is greater that immediately
after my commit.

> > With that test in, I got the error message: "No test named
> > `electric-pair-whitespace-chomping-2-at-point-4-in-c++-mode-in-strings'",
> > and no other tests were performed, leaving an electric-tests.log file 86
> > bytes long.  That's why I commented it out.  This may be some glitch in
> > the testing system.

> I see, you disabled all c++ tests, *sigh*

Yes.  I couldn't understand the (undocumented) structure of these tests
well enough to make better targetted amendments.

> > > C Mode doesn't use electric-layout mode, but a user can surely
> > > decide he wants to use it in c-mode, can he not??

> > No.  Certainly not at the moment.

> No? Will you come to his house and tell him personally not to
> M-x electric-layout-mode? What if that's what he _prefers_?

That user is purely hypothetical.  And if there actually were such a
user, I would urge him to use CC Mode's auto-newline mode, which is much
better tailored to CC Mode than a generic function could be.

And if he insisted, then maybe we could look at adding
electric-layout-mode support to CC Mode.  But it would be a lot of work
for little return, given that auto-newline mode exists, works, and works
well.

> > > These tests pass fine currently.

> > When I ran them, there were lots of failures, because the tests assumed
> > electric-layout-mode was active.

> The tests should activate electric-layout-mode temporarily if needed and
> the revert to the previous situation.  If they aren't doing this, that is what
> needs to be fixed, but it works for me and I don't have electric-layout-mode,
> too.

electric-layout-mode doesn't, and can't work with CC Mode, in particular
with the c-electric-.... functions, because these are incompatible with a
non-nil post-self-insert-hook.

> > > Please revert this fix and lets discuss why you need to disable tests.

> > It's not a fix, it's a temporary workaround.

> With all due respect, I've heard that one before many many times.

> Do you realize that you've broken many many things about
> electric-pair-mode?

No, I don't.  It would have been good if you could have supplied me with
specific recipes to reproduce where it (electric-pair-mode) goes wrong,
thus allowing me to fix them.

> For example Beatrix, and me and everyone else will now lose
> the default "autobalancing" functionality of electric-pair-mode,
> which is its best feature! Autobalance used to work identically in
> most modes, but now you broke that. Is it worth breaking so
> many things just for fixing a much smaller case?

> Just one example of one of the many cases you broke.

What is "autobalancing" (the term doesn't appear in elec-pair.el) and how
is it broken?  Why can't you cite a recipe to reproduce the alleged
breakage?

> electric-pair-mixed-paren-3-at-point-4-in-c++-mode is a test defined
> in `electric-tests.elc'.

> Electricity test in a `c++-mode' buffer.

> Start with point at 4 in a 7-char-long buffer
> like this one:

>   |  (])  |   (buffer start and end are denoted by `|')

> Now call this:

> #[nil "\300\301!\207"
>       [electric-pair-mode 1]
>       2]

Pardon?  Do you mean M-: (electric-pair-mode 1)?

> Now press the key for: (

Done, having enabled e-p-m via M-:, and toggled CC Mode's electric-mode.

> The buffer's contents should become:

>   |  (()])  |

> , and point should be at 5.

Indeed, this is what I observe.  Forgive me for not hand typing in a
compiled binary.

> [back]

> You turn this into (())]) so now I have two unbalanced parenthesis types
> in my buffer.

Are you sure?  Wherein then, lies the difference between your compiled
binary above and M-: (electric-pair-mode 1)?  Could this be a bug in
e-p-m, or the byte compiler?

> > Anyhow, surely the implementation of Emacs should not be constrained by
> > its tests?  Rather the tests should test the chosen implementation.

> Really Alan, you completely broke electric-pair-mode by trying to
> reimplement it in cc-mode.el where it was working perfectly but for a small
> glitch (which really isn't its fault).

I think you are exaggerating somewhat.  And you'll find I didn't
"reimplement" electric-pair-mode, even though I was talking about this at
one stage.  CC Mode calls electric-pair-post-self-insert-function, but in
a controlled fashion at suitable places.

And I disagree about your "small glitch" characterisation.  Adding
post-self-insert-hook to Emacs broke, fundamentally, c-electric-...  This
was the breakage that Beatrix Klebe's bug uncovered.  My patch fixed it.

> If you're going to reimplement it, reimplement *all* of it (of course you might
> come to the conclusion that its best to use the existing implementation...)

I have used the existing implementation.

> > > If we come to the conclusion that some tests are asserting unreasonable
> > > expectations about the functionality you develop, we can disable them on a
> > > case by case basis!

> > I would have done that, indeed tried to do that, but the lack of
> > documentation of electric-pair-test-for, electric-pair-define-test-form
> > and so on, many of them with 8, 9 or more parameters, made that too
> > difficult, given the urgency I felt to produce a workaround.

> If this is indeed urgent, you can add some conditional check to the
> offending code using 'ert-running-test'.

Maybe you could do this.  I'm not familiar enough with the code in
electric-test.el, important pieces of which, I repeat myself, are wholly
undocumented.

> > > If on the other hand, if you need to do some work "temporarily", then
> > > the best way to do it without disturbing other people's developments
> > > is to do it in a separate branch.

> > I fixed bug #33794[*] on master, as I always do with CC Mode bugs (and most
> > other sorts of bugs, too).  That fix is, in principle, sound.  I didn't
> > discover the problems with the unit tests till later (though I admit I
> > should have done).

> It's *not* sound if it breaks tests. At least not without first arguing
> that the tests are placing unreasonable expectations on your code.

The tests do place such unfounded expectations on the code.  In
particular, the contents of post-self-insert-hook.

> > [*] That is, Beatrix Klebe's bug about CC Mode's auto-newlines not
> > working with electric-pair-mode.

> Beatrix Klebe, can, as a workaround, use electric-layout-mode
> perfectly well for her use case (even though you insist she can't)
> Until you can sort out c++-mode.

I've sorted out CC Mode.  Beatrix Klebe does not need any workarounds,
given that the bug she reported has been properly fixed.  It is likely
she is currently using that fix.

> It's much easier than creating this mess that really interferes
> with other's peoples work.

I would ask you to consider that the mess was created by you, last night,
and it interferes greatly with my work.  This mess was an order of
magnitude greater than the mess you assert I made in electric-tests.el.

I don't consider the temporary changes I made in electric-tests.el were
unreasonable, and if you disagree you could have discussed it with me in
a less disagreeable fashion than what transpired.

And now, it seems your idea is to leave special purpose code in
CC Mode, just to obviate the need to amend electric-tests.el.  As you can
imagine, I'm not keen on that.

[ .... ]

> In the meantime. I will have to fix this differently. I will add a
> temporary variable that I can set to have sane C++ behaviour
> in the meantime. I will set this variable when running tests, so
> the test people will see correct results.

That variable must go.

I have another suggestion: you could amend electric-tests.el not to
attempt to use CC Mode at all - or stick with plainer-c-mode, possibly
writing a plainer-c++-mode.  After all, the purpose is to test the
electric-... functions, not CC mode.  That could save a lot of argument.

> João

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-18 17:54     ` Alan Mackenzie
@ 2019-01-18 19:55       ` João Távora
  2019-01-18 22:53         ` Alan Mackenzie
  2019-01-18 21:06       ` Stefan Monnier
                         ` (2 subsequent siblings)
  3 siblings, 1 reply; 27+ messages in thread
From: João Távora @ 2019-01-18 19:55 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: emacs-devel

Hello Alan,

Alan Mackenzie <acm@muc.de> writes:

> First of all, I will say that I take exception, great exception, to
> your means of expressing yourself, both in your post and (even more
> so) in the commit you made.  It verges on sarcasm, and is totally over
> the top and uncalled for.

The only sarcastic expression I can read in my post is "volkswagenesque
atrocity".  All the rest is dead serious.  And even the VW reference is
appropriate.

It's not over the top, and I hope to explain why further down.

>> > > with it.
>
>> > That would leave lots of failed tests in make check.  People have
>> > already remarked on those failures, implicitly asking me to fix them.
>
>> Yes. But the correct fix was to revert the commit that broke
>> the tests, not this volkswagenesque atrocity.
>
> The tests are now broken.  They make assumptions about CC Mode's workings
> which don't hold.  Why can't we cooperate to fix them?

Of course we can!  But let's first analyse what these assumptions are
instead of breaking the tests.

Perhaps you have the impression that tests are somehow non-real, because
they are just code.  But you broke actual things in my (and probably
other's people usage of C-mode, C++-mode, Awk-mode etc).  Things that
have been working since Emacs 24.4 and that suddently stop working.  If
I don't stand up now it'll be like the last time and the bug will stay.
And this personally affects my work and other's.  You must take that
into account.

>> > My understanding, from a previous encounter, was that having no failing
>> > unit tests was of a high priority.  I've only commented a little bit
>> > out, I haven't made any permanent, unreverseable changes.
>> Then you'll certainly be OK if I revert your two commits myself:
> I am not at all OK about this.

Why?  I'm using the exact same argument you made: It's not permanent or
"unreversable", since we have a VCS.

>> the commit that broke the tests and the commit that disables the test.
>> That is certainly not permanent or unreversable, too!
> No, but the amount of work it now requires is greater that immediately
> after my commit.

?

Have read commit be505726b68d407a44fdcd9c7ac1ef722398532d?

You do realize that your commit is still in place right?  As we speak
users that build Emacs masters or install "emacs-snapshot" builds from
Ubuntu are seeing the behaviour you intend.  (And they have had
electric-pair-mode's features blown away from them).  I'm not saying
we're going to see pouring bug reports, but the damage is real?

>> > With that test in, I got the error message: "No test named
>> > `electric-pair-whitespace-chomping-2-at-point-4-in-c++-mode-in-strings'",
>> > and no other tests were performed, leaving an electric-tests.log file 86
>> > bytes long.  That's why I commented it out.  This may be some glitch in
>> > the testing system.
>> I see, you disabled all c++ tests, *sigh*
> Yes.  I couldn't understand the (undocumented) structure of these tests
> well enough to make better targetted amendments.

Then you should have asked before.

Now, I will explain something. c++ exists in this line

          (modes '(quote (ruby-mode c++-mode)))

But it could very well be

          (modes '(quote (ruby-mode c-mode awk-mode js-mode c++-mode perl-mode)))

This runs 1250 tests instead of the usual 400

    Selector: t
    Passed:  1247
    Failed:  4 (3 unexpected)  ;; this is related to the thing you broke
                               ;; 6 months ago, they're all cc-mode-based
    Skipped: 0
    Total:   1251/1251

That is to say, every mode you plug there should behave exactly the same
regarding delimiter insertion and deletion.  This is
electric-pair-mode's contract.

c++-mode is there only as a "lab mouse", it stands for all other cc-mode
related modes.

>> > > C Mode doesn't use electric-layout mode, but a user can surely
>> > > decide he wants to use it in c-mode, can he not??
>
>> > No.  Certainly not at the moment.
>
>> No? Will you come to his house and tell him personally not to
>> M-x electric-layout-mode? What if that's what he _prefers_?
>
> That user is purely hypothetical.

No, he's not! I use electric-layout-mode from time to time!  And
Beatrix's problem could very well be solved by it.  In fact if you put
this in your .emacs today, you're very close to getting
c-toggle-auto-newline for braces, and still follow your style variables
and whatnot.  All without c-toggle-auto-newline.

(setq electric-layout-rules '(electric-layout-for-c-style-du-jour))

(setq c--disable-fix-of-bug-33794 t)

(defun electric-layout-for-c-style-du-jour (inserted)
  (when (and (derived-mode-p 'c-mode 'c++-mode)
             (memq inserted '(?{ ?})))
    (save-excursion
      (backward-char)
      (c-brace-newlines (c-point-syntax)))))


> And if there actually were such a user, I would urge him to use CC
> Mode's auto-newline mode, which is much better tailored to CC Mode
> than a generic function could be.

In what ways?  Alan, can you make an objective criticism of
electric-layout-mode?  I'm not saying it can't be improved, but since
you like to bash it so much, can we read a reason?  I could use that to
improve it.

> And if he insisted, then maybe we could look at adding
> electric-layout-mode support to CC Mode.  But it would be a lot of work
> for little return, given that auto-newline mode exists, works, and works
> well.

In that case I insist!

I've shown above that it isn't!  Perhaps it's not perfect yet, but
certainly doesn't look like a lot of work.

Please please read the new electric-layout-mode API.  Just M-x
describe-variable RET eletric-layout-rules.

>> > > These tests pass fine currently.
>
>> > When I ran them, there were lots of failures, because the tests assumed
>> > electric-layout-mode was active.
>
>> The tests should activate electric-layout-mode temporarily if needed and
>> the revert to the previous situation.  If they aren't doing this, that is what
>> needs to be fixed, but it works for me and I don't have electric-layout-mode,
>> too.
>
> electric-layout-mode doesn't, and can't work with CC Mode, in particular
> with the c-electric-.... functions, because these are incompatible with a
> non-nil post-self-insert-hook.

They aren't generally incompatible, it's just this corner that Beatrix
reported that kicked the hornets nest.  I've been working with c/c++ and
electric-pair-mode for 5+ years now, and it works perfectly if you don't
turn on c-auto-newline.

But if you want to improve the situation for those that do turn on
c-auto-newline, why not create a

(defun c-self-insert-command (arg)
    (let ((post-self-insert-hook
            (if (some-condition-that-joaot-and-I-have-agreed)
                nil
                post-self-insert-hook)))
         (self-insert-command arg)))

It's much easier.


>> > > Please revert this fix and lets discuss why you need to disable tests.
>
>> > It's not a fix, it's a temporary workaround.
>
>> With all due respect, I've heard that one before many many times.
>
>> Do you realize that you've broken many many things about
>> electric-pair-mode?
>
> No, I don't.  It would have been good if you could have supplied me with
> specific recipes to reproduce where it (electric-pair-mode) goes wrong,
> thus allowing me to fix them.

The tests themselves supply those recipes.  M-x ert-describe-test does
really good job, if you don't get hung up. 

>
>> For example Beatrix, and me and everyone else will now lose
>> the default "autobalancing" functionality of electric-pair-mode,
>> which is its best feature! Autobalance used to work identically in
>> most modes, but now you broke that. Is it worth breaking so
>> many things just for fixing a much smaller case?
>
>> Just one example of one of the many cases you broke.
>
> What is "autobalancing" (the term doesn't appear in elec-pair.el) and how
> is it broken?  Why can't you cite a recipe to reproduce the alleged
> breakage?

See the docstring of electric-pair-preserve-balance.  And read the
test's descriptions.

>> #[nil "\300\301!\207"
>>       [electric-pair-mode 1]
>>       2]
>
> Pardon?  Do you mean M-: (electric-pair-mode 1)?

Of course I do mean that. Come on, this doc is autogenerated and I was
in a hurry last time, I can do some form-walking later-on to provide a
perfectly pristine instruction without that glitch.

>> The buffer's contents should become:
>
>>   |  (()])  |
>
>> , and point should be at 5.
>
> Indeed, this is what I observe.

That's because the tests are setting c--disable-fix-of-bug-33794 to t!

IOW I made my own Volkswagenesque atrocity, but at least mine is easier
to identify while we analyse the problem.  And it gives me (and perhaps
other) the opportunity to develop in a master Emacs in C/C++/whatever
without the failures.

If you roll back to just after your fix (the version Michael Albinus was
using), you'll see it break.  You can also see it break if you remove
the line of c--disable-fix-of-bug-33794.


>> You turn this into (())]) so now I have two unbalanced parenthesis types
>> in my buffer.
>
> Are you sure?  Wherein then, lies the difference between your compiled
> binary above and M-: (electric-pair-mode 1)?  Could this be a bug in
> e-p-m, or the byte compiler?

No, you're just testing with the wrong version.

This one test was picked out of the 85 or so that were broken (just for
C++, of course this number is repeated in C/Awk/Pike, whatever by )

>> > Anyhow, surely the implementation of Emacs should not be constrained by
>> > its tests?  Rather the tests should test the chosen implementation.
>
>> Really Alan, you completely broke electric-pair-mode by trying to
>> reimplement it in cc-mode.el where it was working perfectly but for a small
>> glitch (which really isn't its fault).
>
> I think you are exaggerating somewhat.  And you'll find I didn't
> "reimplement" electric-pair-mode, even though I was talking about this at
> one stage.  CC Mode calls electric-pair-post-self-insert-function, but in
> a controlled fashion at suitable places.
>
> And I disagree about your "small glitch" characterisation.  Adding
> post-self-insert-hook to Emacs broke, fundamentally, c-electric-...  This
> was the breakage that Beatrix Klebe's bug uncovered.  My patch fixed
> it.

It broke the interaction of c-auto-newline with electric-pair-mode mode
as far as I know.  This is what was described in bug#33794 by Beatrix
Klebe.  Does it break more things?  Does it break more things related to
electric-pair-mode?

>> If you're going to reimplement it, reimplement *all* of it (of course you might
>> come to the conclusion that its best to use the existing implementation...)
> I have used the existing implementation.

The function you called is not meant to be called from that context.
It's not an "external" function.  Shall I rename it
electric----pair-post-self-insert-function :-)

>> > > If we come to the conclusion that some tests are asserting unreasonable
>> > > expectations about the functionality you develop, we can disable them on a
>> > > case by case basis!
>
>> > I would have done that, indeed tried to do that, but the lack of
>> > documentation of electric-pair-test-for, electric-pair-define-test-form
>> > and so on, many of them with 8, 9 or more parameters, made that too
>> > difficult, given the urgency I felt to produce a workaround.
>
>> If this is indeed urgent, you can add some conditional check to the
>> offending code using 'ert-running-test'.
>
> Maybe you could do this.  I'm not familiar enough with the code in
> electric-test.el, important pieces of which, I repeat myself, are wholly
> undocumented.

It's much easier to do this with a variable that I used: then ert, and
some users can set that variable.  And it is even easier if we introduce
c-self-insert-command and think about the problem.

>> > > If on the other hand, if you need to do some work "temporarily", then
>> > > the best way to do it without disturbing other people's developments
>> > > is to do it in a separate branch.
>
>> > I fixed bug #33794[*] on master, as I always do with CC Mode bugs (and most
>> > other sorts of bugs, too).  That fix is, in principle, sound.  I didn't
>> > discover the problems with the unit tests till later (though I admit I
>> > should have done).
>
>> It's *not* sound if it breaks tests. At least not without first arguing
>> that the tests are placing unreasonable expectations on your code.
>
> The tests do place such unfounded expectations on the code.  In
> particular, the contents of post-self-insert-hook.

The tests assume modes won't

  (let ((post-self-insert-hook)) (self-insert-command))

yes.  I think that's a reasonable assumption.

Besides tests I also assume that!  I want my post-self-insert-hook to
work.  I want to write modes based on it, and I want to order pizzas for
Donald Trump every for every character I type!

>> > [*] That is, Beatrix Klebe's bug about CC Mode's auto-newlines not
>> > working with electric-pair-mode.
>> Beatrix Klebe, can, as a workaround, use electric-layout-mode
>> perfectly well for her use case (even though you insist she can't)
>> Until you can sort out c++-mode.
> I've sorted out CC Mode.  Beatrix Klebe does not need any workarounds,
> given that the bug she reported has been properly fixed.  It is likely
> she is currently using that fix.

You opened 83 or so other bugs times just for C++-mode!  Why couldn't
you tell Beatrix: look don't use electric-pair-mode in cc-mode when
using c-auto-newline??  

>> It's much easier than creating this mess that really interferes
>> with other's peoples work.
>
> I would ask you to consider that the mess was created by you, last night,
> and it interferes greatly with my work.  This mess was an order of
> magnitude greater than the mess you assert I made in
> electric-tests.el.

How so?  Really, explain how!  I did *not* remove your fix.  I just made
it possible to disable it!

> I don't consider the temporary changes I made in electric-tests.el were
> unreasonable, and if you disagree you could have discussed it with me in
> a less disagreeable fashion than what transpired.
>
> And now, it seems your idea is to leave special purpose code in
> CC Mode, just to obviate the need to amend electric-tests.el.  As you can
> imagine, I'm not keen on that.

It is you who left a buggy, incomplete re-implementation of
electric-pair-mdoe in cc-mode.el in the first place.  It's not needed,
Alan.

1. CC-mode can be thinner after all this
2. Beatrix can be happy
3. You could be happy
4. I could be happy
5. Donald Trump would have to pay and eat a million pizzas

> That variable must go.

It can.  Let's use a c-self-insert-command function.  Or macro, if you
prefer.  Then let's check clearly the conditions it should delegate to
the previous or the new behaviour.

But let's do this is a branch yes?  I've just pushed

   git push origin scratch/resolve-cc-mode-and-e-p-m
   Total 0 (delta 0), reused 0 (delta 0)
   remote: Sending notification emails to: emacs-diffs@gnu.org
   To git.sv.gnu.org:/srv/git/emacs.git
    * [new branch]            scratch/resolve-cc-mode-and-e-p-m -> scratch/resolve-cc-mode-and-e-p-m

> I have another suggestion: you could amend electric-tests.el not to
> attempt to use CC Mode at all - or stick with plainer-c-mode, possibly
> writing a plainer-c++-mode.  After all, the purpose is to test the
> electric-... functions, not CC mode.  That could save a lot of argument.

I would have to write plainer-<everything-based-on-cc-mode>-for that.
And give that to people working with me.  And teach them to enable these
new version.  It's ridiculous.

But I guess it's an option if we make them default in auto-mode-alist
and such.  Quite a nuclear option, tho.

João



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-18 17:54     ` Alan Mackenzie
  2019-01-18 19:55       ` João Távora
@ 2019-01-18 21:06       ` Stefan Monnier
  2019-01-19  3:25         ` João Távora
  2019-01-18 22:47       ` Michael Albinus
  2019-01-19 20:18       ` Richard Stallman
  3 siblings, 1 reply; 27+ messages in thread
From: Stefan Monnier @ 2019-01-18 21:06 UTC (permalink / raw)
  To: emacs-devel

> After all, the purpose is to test the electric-... functions, not
> CC mode.

The purpose is to test that those features also work in the same way in
CC-mode.  And that's important.  I haven't found an electric pairing
mode that pairs exactly when I want it, so the second best choice is for
me to learn when it will and when it won't: the exact choice doesn't
matter, but it's important that the details of its working that I learn
in mode X also apply in mode Y, otherwise it's irritating.


        Stefan




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-18 17:54     ` Alan Mackenzie
  2019-01-18 19:55       ` João Távora
  2019-01-18 21:06       ` Stefan Monnier
@ 2019-01-18 22:47       ` Michael Albinus
  2019-01-19 20:18       ` Richard Stallman
  3 siblings, 0 replies; 27+ messages in thread
From: Michael Albinus @ 2019-01-18 22:47 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: João Távora, emacs-devel

Alan Mackenzie <acm@muc.de> writes:

> Hello, João.

Hi everybody,

>> > >     Temporarily comment out CC Mode from tests which are incompatible
>> > > with it.
>
>> > That would leave lots of failed tests in make check.  People have
>> > already remarked on those failures, implicitly asking me to fix them.
>
>> Yes. But the correct fix was to revert the commit that broke
>> the tests, not this volkswagenesque atrocity.
>
> The tests are now broken.  They make assumptions about CC Mode's workings
> which don't hold.  Why can't we cooperate to fix them?

Please do not comment out tests which are broken (I have no idea
why). The proper way is to tag them as :unstable, as explained in
test/README. By this, all of us won't see them when running "make
check", and you could still run the tests on your local machine while
fixing them.

Once stabilized, remove the :unstable tag.

Best regards, Michael.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-18 19:55       ` João Távora
@ 2019-01-18 22:53         ` Alan Mackenzie
  2019-01-19  3:18           ` João Távora
  0 siblings, 1 reply; 27+ messages in thread
From: Alan Mackenzie @ 2019-01-18 22:53 UTC (permalink / raw)
  To: João Távora; +Cc: emacs-devel

Hello, João.

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

Firstly I insist that you respect my maintainership of CC Mode; that I
understand it better than you, I know its history, its tradeoffs, its
problems.  And that things like your patch of yesterday, which was
grossly disrespectful, will not be repeated.

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.

And I request you to tone down your aggressiveness.  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.  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.

c-electric-brace and friends depend for their proper working on knowing
or controlling every character that is inserted into or deleted from the
buffer.  When random functionality (from the point of view of
c-electric-brace) is added to self-insert-command, these functions
cannot work.  That is why it is essential to bind post-self-insert-hook
to nil in c-electric-brace.  It is why c-electric-brace is incompatible
with the (ab)use of post-self-insert-hook by certain electric-...
functions.

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.

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

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

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.  You suggest that I should put in the hard work of extracting
recipes from your tests myself.  Sorry, that's a lot of work and I've
got other things to do.  The successful uses of electric-pair-mode I
reported earlier were on the Emacs after my patch but without your
patch.  As far as I'm concerned, electric-pair-mode works just fine in
CC Mode, until I see a coherent recipe that breaks it.  Then I will fix
it.

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

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.

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-18 22:53         ` Alan Mackenzie
@ 2019-01-19  3:18           ` João Távora
  2019-01-19 11:07             ` Alan Mackenzie
  0 siblings, 1 reply; 27+ messages in thread
From: João Távora @ 2019-01-19  3:18 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: emacs-devel

Hi Alan

Alan Mackenzie <acm@muc.de> 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





^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-18 21:06       ` Stefan Monnier
@ 2019-01-19  3:25         ` João Távora
  0 siblings, 0 replies; 27+ messages in thread
From: João Távora @ 2019-01-19  3:25 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> After all, the purpose is to test the electric-... functions, not
>> CC mode.
>
> The purpose is to test that those features also work in the same way in
> CC-mode.  And that's important.  I haven't found an electric pairing
> mode that pairs exactly when I want it, so the second best choice is for
> me to learn when it will and when it won't: the exact choice doesn't
> matter, but it's important that the details of its working that I learn
> in mode X also apply in mode Y, otherwise it's irritating.

Of course.  But the first best choice is for you to code up the the
criteria that "pairs exactly when [you] want it" and then put function
in electric-pair-inhibit-predicate, and add it to Emacs, perhaps even
make it the default (if everyone else agrees).  And then it will behave
like that in every mode.

João






^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-19  3:18           ` João Távora
@ 2019-01-19 11:07             ` Alan Mackenzie
  2019-01-19 13:52               ` João Távora
  0 siblings, 1 reply; 27+ messages in thread
From: Alan Mackenzie @ 2019-01-19 11:07 UTC (permalink / raw)
  To: João Távora; +Cc: emacs-devel

Hello, João.

On Sat, Jan 19, 2019 at 03:18:03 +0000, João Távora wrote:
> Alan Mackenzie <acm@muc.de> writes:

> It's not about the file [electric-tests.el] 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!

I did not "destroy" any behaviour.  The behaviour you referred to was
broken, and has been for a long time, ever since its introduction.  It
can be fixed.

I'm now going to ask you to consider things at a more abstract level.
electric-thing's abuse of post-self-insert-hook is fundamentally flawed.
It usurps control of the buffer contents from the current major mode.  It
is an inversion of control.  It is the tail wagging the dog.

Instead of continually harrassing me to break CC Mode, why don't you just
fix this flaw in electric-thing?  I suggest gathering pertinent
information in a before-change or after-change function (or both), and
evaluating it in, say, a post-command-hook function.  Or something like
that.  post-self-insert-hook was a cheap and easy "solution" to a
problem, which had nasty side effects which weren't considered at the
time.  In particular, it broke CC Mode, even though it took time for that
to become apparent.

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

It's you who's "going down a path", not me.  I'm staying where I am,
keeping CC Mode working.  And let me be quite emphatic.  It's going to
stay working in all use cases, not just the ones you use.

[ .... ]

> 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!

My way is to fix reported bugs, rather than telling bug reporters to take
a running jump.  Your "evidence" of alternatives is mainly hand waving
and harrassment to break CC Mode.  You refuse to understand the
incompatibility of c-electric-brace etc. with electric-thing's abuse of
post-self-insert-command.

[ .... ]

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

So telling users to change their work habits is a solution to bugs?  I
think not.

[ .... ]

> > 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!

Telling eachother things 3 times over is unfortunately a characteristic
of this exchange.  :-(

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

[ Description of use of electric-pair-mode on a > snipped. ]

> Your change makes <>> instead.

Yes.  Automatically pairing a less-than sign with a greater-than sign is
stupid, certainly in CC Mode, and likely in most other programming modes.
It would quickly become intolerably annoying to the user.  The only
contexts where it would be sensible are in a template declaration, or in
a #include directive.  Only the major mode can determine these things.

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

OK, I'll have a look at strings and comments.  Why does
electric-pair-mode operate inside literals?  Is this really the Right
Thing?

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

This is a critical point, which you have declined to address.  Would you
please answer the point in your next post.

> > 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?

You didn't answer the question.  Have you tried it?  You can have that
preference, but given the incompatibility, it won't get you anywhere.
Fix electric-layout-mode and then you should be able to.

[ .... ]

> > 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 just your personal use case.  For those using the BSD styles, for
example, they want a newline after a function opening brace, but not
after a substatement brace.  electric-layout-mode can't do this.

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

No, I haven't tried it.  There is no usable interface with
electric-layout-mode for CC Mode.  Also that strategy would fragment CC
Mode's functionality, making it less easy to understand and more
difficult to maintain.  Why do you have this stubborn desire for
electric-l-m in CC Mode?

[ .... ]

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

No answer?

> > 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 doesn't really do much, it doesn't help much, it's a matter of taste.
It seems your intention is to modify that function in a way which will
break c-electric-thing.

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

I will not encourage users to break CC Mode.  If that is what some users
really want to do, they have access to the source code.

[ .... ]

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

If you like, yes, but it seems likely to lead to problems in the future.
Why don't you just fix electric-layout-mode and electric-pair-mode?

> Goodbye Alan,
> João

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-19 11:07             ` Alan Mackenzie
@ 2019-01-19 13:52               ` João Távora
  2019-01-19 17:45                 ` Alan Mackenzie
  2019-01-20 19:05                 ` Richard Stallman
  0 siblings, 2 replies; 27+ messages in thread
From: João Távora @ 2019-01-19 13:52 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: emacs-devel

Alan Mackenzie <acm@muc.de> writes:

> I did not "destroy" any behaviour.  The behaviour you referred to was
> broken, and has been for a long time, ever since its introduction.  It
> can be fixed.

That's not for you to decide whether electric-pair-mode, an optional
minor mode with a well-defined contract, is broken.  Its main goal is to
work the same is all modes, as any global minor mode.

> problem, which had nasty side effects which weren't considered at the
> time.  In particular, it broke CC Mode, even though it took time for that
> to become apparent.

post-self-insert-hook can be used for lots of things, not only
electric-pair-mode and electric-layout-mode.  From requesting
completions from LSP servers[1] to emulating typewriter sounds[2].
Don't break these things, even if they sound useless to you.

[1] https://github.com/joaotavora/eglot/commit/444a8c3b3ec29eceeda26b72493a60c44a9bd951
[2] https://stackoverflow.com/questions/11206140/typewriter-sounds-for-emacs

>> 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.
> So telling users to change their work habits is a solution to bugs?  I
> think not.

Presumably Beatrix just wants to pair braces automatically and insert
newlines.  There are alternatives in Emacs for that.

>> Your change makes <>> instead.
>
> Yes.  Automatically pairing a less-than sign with a greater-than sign is
> stupid, certainly in CC Mode

It's not "stupid" if you tweak electric-pair-inhibit-predicate to don't
do it in certain syntactic context.  But this isn't the point of this
test!  The point is that modes react to electric-pair-pairs, a
customization variable.  You broke that for '<' and '>'.  I am unable to
do use this electric-pair-mode feature, after your "not stupid at all"
change.

> It would quickly become intolerably annoying to the user.  The only
> contexts where it would be sensible are in a template declaration, or in
> a #include directive.  Only the major mode can determine these things.

Excuse me but this is just wrong.  It's just wrong.  Users and programs
can use whatever code to "determine" these things, including cc-mode's
code.  That code is plugged into electric-pair-inhibit-predicate and
that's it!

>> Fix this and all the other ones.  Incidentally, most of the test
>> failures are inside strings and comments, so maybe they are easy fixes.
>
> OK, I'll have a look at strings and comments.  Why does
> electric-pair-mode operate inside literals?  Is this really the Right
> Thing?

It's the default behaviour, but you can customize
electric-pair-inhibit-predicate and electric-pair-skip-self with M-x
customize, or advice-add or somesuch if you don't like it personally.

"The Right Thing" is that is works the same way in all modes and follows
the customization variables consistently.

>> > 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.
>
> This is a critical point, which you have declined to address.  Would you
> please answer the point in your next post.

Major modes are "at liberty" to do that, but they break all minor-mode
horizontal to Emacs that rely on this functionality, or have to
reimplement them.  They're also "at liberty" to change say
buffer-undo-list or post-command-hook, or what do I know.  But that
doesn't mean it's a good idea.  It's not.

>> > 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?
> You didn't answer the question.  Have you tried it?

Yes, of course.  It worked ok as long as you don't use
electric-pair-mode.  When I turned it on, things broke, so I switched to
electric-layout-mode.  Which is exactly what happened to Beatrix, but
she decided to report a bug instead.  And now she and everybody, even
those who *don't* use c-auto-newline, get a doubly broken
electric-pair-mode in CC-mode.  Gee thanks for that, what an
accomplishment!

> You can have that preference, but given the incompatibility, it won't
> get you anywhere. 

I return the challenge.  Have you tried electric-layout-mode before
making these claims?

>> > 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 just your personal use case.  For those using the BSD styles, for
> example, they want a newline after a function opening brace, but not
> after a substatement brace.  electric-layout-mode can't do this.

For the third of fourth time Have you actually *seen* and read the 8
lines of code below that I posted already before?  Have you perhaps
considered spending 5 minutes trying them out?

    (setq electric-layout-rules '(electric-layout-for-c-style-du-jour))

    (defun electric-layout-for-c-style-du-jour (inserted)
      (when (and (derived-mode-p 'c-mode 'c++-mode)
                 (memq inserted '(?{ ?})))
        (save-excursion
          (backward-char)
          (c-brace-newlines (c-point-syntax)))))

    (electric-layout-mode 1)

It might need a tweak or two to deal with functions that return
before|after lists, but nothing special.  For braces it is actually
useful already.

> No, I haven't tried it.  There is no usable interface with
> electric-layout-mode for CC Mode.

I just gave you one, for the second time.  But the interface *is*
cc-mode.  You get this wrong.  You have a distored notion of
minor-modes.  The point is that you don't do anything deliberately
destroying them, and they work in your mode, and you don't have to worry
about reimplementing their functionality.  Don't you think it's silly to
be copying implementation details from elec-pair.el into your beloved cc
mode files just to support it.  Wouldn't you rather delete code like
this?

    ;; Emulate `electric-pair-mode'.
    (when (and (boundp 'electric-pair-mode)
	       electric-pair-mode)
      (let ((size (buffer-size))
	    (c-in-electric-pair-functionality t)
	    post-self-insert-hook)
	(electric-pair-post-self-insert-function)
	(setq got-pair-} (and at-eol
			      (eq (c-last-command-char) ?{)
			      (eq (char-after) ?}))
	      electric-pair-deletion (< (buffer-size) size))))

It's becoming a salad in there, and it doesn't have to!

> Also that strategy would fragment CC Mode's functionality, making it
> less easy to understand and more difficult to maintain.

Really? Refactoring things into functions so they can be used by other
things is bad?  So why did you come up with c-brace-newlines, may I ask?
I suppose I shouldn't even draw your attention to it because now i fear
you're going to take it apart and obfuscate its behaviour out of spite,
just to make my job harder.

> Why do you have this stubborn desire for electric-l-m in CC Mode?

Because I like minor modes that work consistently across all of Emacs.
I've told you this a million times already.

>> > 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.
> No answer?

This is ridiculous.  You say it doesn't work.  I show that it works.
You don't try it or verify for yourself.  GOTO ridiculous.

>> 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 doesn't really do much

This is completely true.

> it doesn't help much, it's a matter of taste.

This is completely false.

> It seems your intention is to modify that function in a way which will
> break c-electric-thing.

What's c-electric-thing?  I don't want to break anything for anyone.
Explain clearly what would be broken for whom.  Explain that exactly, I
beg you.

You didn't even give me a change to explain how I would use that
function, how can you know?  Let me try one last time:

If you make that function, or some other configurable abstraction, we
can modify it so that:

* If c-auto-newline is on is works exactly like you want it;
* Otherwise it works like it did before 223e7b87872d4a010ae1c9a6f09a9c15aee46692

Now explain exactly how and for whom this breaks things, in the present
or in the future.

>> 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.
> I will not encourage users to break CC Mode.  If that is what some users
> really want to do, they have access to the source code.

Are you going to make users learn Emacs Lisp understand cc-mode's
sources?  That's the pinnacle of cruelty.  

João



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-19 13:52               ` João Távora
@ 2019-01-19 17:45                 ` Alan Mackenzie
  2019-01-19 19:15                   ` João Távora
  2019-01-20 19:05                 ` Richard Stallman
  1 sibling, 1 reply; 27+ messages in thread
From: Alan Mackenzie @ 2019-01-19 17:45 UTC (permalink / raw)
  To: João Távora; +Cc: emacs-devel

Hello, Joãa.

On Sat, Jan 19, 2019 at 13:52:53 +0000, João Távora wrote:
> Alan Mackenzie <acm@muc.de> writes:

> That's not for you to decide whether electric-pair-mode, an optional
> minor mode with a well-defined contract, is broken.  Its main goal is to
> work the same is all modes, as any global minor mode.

What you are in denial about, is that CC Mode and
electric-{pair,layout}-mode (as currently implemented) are incompatible
with each other.  I've explained this to you more times than I can
count, but you still refuse to accept it.

As always, your "solution" to this is for CC Mode to be broken, rather
than fixing the problems in your own bailiwick.

Our goals are not the same.  Mine is to preserve the integrity of CC
Mode and keep it working, yours is to make electric-*-mode universal.
It might be that your goal is unattainable.  My suggestion is that you
leave CC Mode in peace, and investigate ways you can change e-p/l-m so
that they can become truly universal.  I've already made some
suggestions for this.

[ .... ]

> > Automatically pairing a less-than sign with a greater-than sign is
> > stupid, certainly in CC Mode

> It's not "stupid" if you tweak electric-pair-inhibit-predicate to don't
> do it in certain syntactic context.  But this isn't the point of this
> test!  The point is that modes react to electric-pair-pairs, a
> customization variable.  You broke that for '<' and '>'.  I am unable to
> do use this electric-pair-mode feature, after your "not stupid at all"
> change.

> > It would quickly become intolerably annoying to the user.  The only
> > contexts where it would be sensible are in a template declaration, or in
> > a #include directive.  Only the major mode can determine these things.

OK, I'll make you a peace offering.  I'll put electric-pair-mode into
c-electric-lt-gt in the above contexts, and you will change the test
code so that in the C++ version of the < tests, lines start with #include.

[ .... ]

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

> > OK, I'll have a look at strings and comments.  Why does
> > electric-pair-mode operate inside literals?  Is this really the Right
> > Thing?

> It's the default behaviour, but you can customize
> electric-pair-inhibit-predicate and electric-pair-skip-self with M-x
> customize, or advice-add or somesuch if you don't like it personally.

The default in CC Mode has always been that electric actions aren't done
inside strings and comments, and in some other cases.  I'm currently
considering whether electric-pair-mode should be an exception.

> "The Right Thing" is that is works the same way in all modes and follows
> the customization variables consistently.

No, the Right Thing is whatever is best for the user in the current
major mode, taking into account configuration settings.

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

> > This is a critical point, which you have declined to address.  Would you
> > please answer the point in your next post.

> Major modes are "at liberty" to do that, but they break all minor-mode
> horizontal to Emacs that rely on this functionality, or have to
> reimplement them.

Minor modes shouldn't rely on this hook.  It's a mistaken design
decision.

> They're also "at liberty" to change say buffer-undo-list or
> post-command-hook, or what do I know.  But that doesn't mean it's a
> good idea.  It's not.

buffer-undo-list is frequently bound to t.  Good idea, or not, sometimes
setting or binding these hook variables is the only way to get a result.

> >> > 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?
> > You didn't answer the question.  Have you tried it?

> Yes, of course.  It worked ok as long as you don't use
> electric-pair-mode.  When I turned it on, things broke, so I switched to
> electric-layout-mode.  Which is exactly what happened to Beatrix, but
> she decided to report a bug instead.  And now she and everybody, even
> those who *don't* use c-auto-newline, get a doubly broken
> electric-pair-mode in CC-mode.  Gee thanks for that, what an
> accomplishment!

Despite your histrionics, electric-pair-mode and CC Mode work well
together.  Certainly the scenario in bug #33794 works well.  You keep
slagging it off, but still haven't given a specific defect in a usable
form (besides saying that e-p-m doesn't work in literals).

> > You can have that preference, but given the incompatibility, it won't
> > get you anywhere. 

> I return the challenge.  Have you tried electric-layout-mode before
> making these claims?

No, I don't need to.  I understand the mechanisms which give rise to the
incompatibility.  I urge you to try and understand these, too.

[ .... ]

> For the third of fourth time Have you actually *seen* and read the 8
> lines of code below that I posted already before?  Have you perhaps
> considered spending 5 minutes trying them out?

I've seen them, yes, and no I won't be trying them out.  I have no
interest in electric-layout-mode, beyond fending off attacks from it on
CC Mode.

> > No, I haven't tried it.  There is no usable interface with
> > electric-layout-mode for CC Mode.

> I just gave you one, for the second time.

No, I mean there's no interface by which CC Mode can trigger the actions
of electric-layout-mode.  Except, perhaps, by calling the function on
post-self-insert-hook as a function.  But how, in that case, does the
major mode get information back about what this function has done?

[ .... ]

> Don't you think it's silly to be copying implementation details from
> elec-pair.el into your beloved cc mode files just to support it.
> Wouldn't you rather delete code like this?

>     ;; Emulate `electric-pair-mode'.
>     (when (and (boundp 'electric-pair-mode)
> 	       electric-pair-mode)
>       (let ((size (buffer-size))
> 	    (c-in-electric-pair-functionality t)
> 	    post-self-insert-hook)
> 	(electric-pair-post-self-insert-function)
> 	(setq got-pair-} (and at-eol
> 			      (eq (c-last-command-char) ?{)
> 			      (eq (char-after) ?}))
> 	      electric-pair-deletion (< (buffer-size) size))))

It's not good, but it's the best that's available.  You may recall me
requesting you to provide an interface suitable for CC Mode, but you
didn't do this.

[ .... ]

> What's c-electric-thing?  I don't want to break anything for anyone.

You want to break c-electric-brace in preference to fixing
electric-layout-mode and electric-pair-mode.

> Explain clearly what would be broken for whom.  Explain that exactly, I
> beg you.

By advising that function you want to introduce, it would unfix the
working of c-auto-newline and bug #33794.  Breakage.

> You didn't even give me a change to explain how I would use that
> function, how can you know?  Let me try one last time:

> If you make that function, or some other configurable abstraction, we
> can modify it so that:

> * If c-auto-newline is on is works exactly like you want it;
> * Otherwise it works like it did before 223e7b87872d4a010ae1c9a6f09a9c15aee46692

No.

> Now explain exactly how and for whom this breaks things, in the present
> or in the future.

It's low quality programming, having the low level details of a function
controlled by a flag with no coherent meaning set from outside.  It
will, one way or another, break things.

[ .... ]

> Are you going to make users learn Emacs Lisp understand cc-mode's
> sources?  That's the pinnacle of cruelty.

If they want to do silly things with CC Mode, then they must get to
grips with the source.  Plenty of people have done this.

> João

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-19 17:45                 ` Alan Mackenzie
@ 2019-01-19 19:15                   ` João Távora
  2019-01-19 20:58                     ` Alan Mackenzie
  2019-01-19 22:37                     ` Drew Adams
  0 siblings, 2 replies; 27+ messages in thread
From: João Távora @ 2019-01-19 19:15 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: emacs-devel

Hello Alan

Alan Mackenzie <acm@muc.de> writes:

> What you are in denial about, is that CC Mode and
> electric-{pair,layout}-mode (as currently implemented) are incompatible
> with each other.  I've explained this to you more times than I can
> count, but you still refuse to accept it.

How can I accept a something that you explain, but don't provide
evidence of?  It's like wanting me to accept I'm swedish.

What you say: cc-mode + e-p-m + e-l-m => incompatible is certainly true
*now*.  But it was't until you pushed that change.  There is code in
electric-pair-tests.el that shows this.  Spend a little less time
handwaving and rewind the code back to before you pushed the change.
Then present an Emacs -Q recipe showing whatever you mean that I'm "in
denial" about.  *Then* we can discuss sanely.

> OK, I'll make you a peace offering.  I'll put electric-pair-mode into
> c-electric-lt-gt in the above contexts, and you will change the test
> code so that in the C++ version of the < tests, lines start with
> #include.

It would pass those, but what if the user wants those for templates too?
The goal is that he/she can program that into electric-pair-pairs and
electric-pair-inhibit-predicate.

But if you want to set electric-pair-inhibit-predicate yourself in the
major-mode, that's perfectly OK.  Just make sure to play along with any
the user's settings, i.e. by using something like 'add-function
:after-until' where you inhibit when you detect a specific inhibition of
pairing).  The only place I can think of (right now) where < shouldn't
be paired to >, i.e. where pairing should be inhibited, is in syntactic
positions where < can be an operator or part of an operator.  No idea if
cc-mode has facilities to detect syntactic context, given C++ parsing is
notoriously difficult.

>> Major modes are "at liberty" to do that, but they break all minor-mode
>> horizontal to Emacs that rely on this functionality, or have to
>> reimplement them.
> Minor modes shouldn't rely on this hook.  It's a mistaken design
> decision.

*In your opinion*.  Which, I'm very sorry to break it to you, is now
irrelevant, because nevertheless it made it into Emacs's C core long
ago, and people want to use it.  And they do, and it has worked until
now.

> Despite your histrionics, electric-pair-mode and CC Mode work well
> together.

No, they don't Alan.  Pairing inside comments and literals is failing.
And there's still the lost pairing behaviour in unterminated string
literals.  All of these things used to work before you started messing
with them around June.

> Certainly the scenario in bug #33794 works well.  You keep slagging it
> off, but still haven't given a specific defect in a usable form
> (besides saying that e-p-m doesn't work in literals).

I gave you 85 defects.  They are all in "usable" form.  Fix them all,
repeat '(when electric-pair-mode' everywhere.

>> > You can have that preference, but given the incompatibility, it won't
>> > get you anywhere. 
>> I return the challenge.  Have you tried electric-layout-mode before
>> making these claims?
> No, I don't need to.

Really, you don't have like 2 minutes to spare to try out 8 lines of
elisp?

> I understand the mechanisms which give rise to the
> incompatibility.  I urge you to try and understand these, too.

What incompatibility?  Where?  When?  How?  I can't understand something
that you don't specify.  It's like I asked you to understand that your
code has too much weltschmerz.  Where is the incompatibility with
electric-layout-mode (before your nefarious change, that is)?

>> For the third of fourth time Have you actually *seen* and read the 8
>> lines of code below that I posted already before?  Have you perhaps
>> considered spending 5 minutes trying them out?
> I've seen them, yes, and no I won't be trying them out.  I have no
> interest in electric-layout-mode, beyond fending off attacks from it on
> CC Mode.

You crack me up Alan, you really have a bellic conception of software
design.  In what ways does it attack CC Mode?  What with bananas and
pointy sticks?

>> > No, I haven't tried it.  There is no usable interface with
>> > electric-layout-mode for CC Mode.
>
>> I just gave you one, for the second time.
>
> No, I mean there's no interface by which CC Mode can trigger the actions
> of electric-layout-mode.  Except, perhaps, by calling the function on
> post-self-insert-hook as a function.  But how, in that case, does the

It doesn't need to, my friend!  That's the beauty of it.  Just call
self-insert-command like you always did!  And don't worry about it.

Make an exception for c-auto-newline if you want, i.e. if c-auto-newline
is t, do whatever you want, like nixing post-self-insert-hook just
there.  Let c-auto-newline, if t, have prevalence over
electric-layout-mode, crushing its insolent attacks upon your beloved
domain.  You won't hear me complain.

> It's not good, but it's the best that's available.  You may recall me
> requesting you to provide an interface suitable for CC Mode, but you
> didn't do this.

You requested that I redesign it around some other thing than
post-self-insert-hook.  I can't do that.  It was like that when I picked
it up.

>> What's c-electric-thing?  I don't want to break anything for anyone.
>
> You want to break c-electric-brace in preference to fixing
> electric-layout-mode and electric-pair-mode.
>
>> Explain clearly what would be broken for whom.  Explain that exactly, I
>> beg you.
>
> By advising that function you want to introduce, it would unfix the
> working of c-auto-newline and bug #33794.  Breakage.

No it wouldn't.  Not it that advice that c-auto-newline into account.

>> * If c-auto-newline is on is works exactly like you want it;
>> * Otherwise it works like it did before 223e7b87872d4a010ae1c9a6f09a9c15aee46692
>
> No.

See above.

> It's low quality programming, having the low level details of a function
> controlled by a flag with no coherent meaning set from outside.  It
> will, one way or another, break things.

Low quality programming?  From the man who is ad-hoc reimplementing
electric-pair-mode?

> If they want to do silly things with CC Mode, then they must get to
> grips with the source.  Plenty of people have done this.

Don't be arrogant.

João

PS: do whatever you want now.  Revert my commit, if you want.  I'll
remove the C++ tests.  It's not worth to insist on any of this anymore.
I have a lot of work coming up myself.  I give up.








^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-18 17:54     ` Alan Mackenzie
                         ` (2 preceding siblings ...)
  2019-01-18 22:47       ` Michael Albinus
@ 2019-01-19 20:18       ` Richard Stallman
  3 siblings, 0 replies; 27+ messages in thread
From: Richard Stallman @ 2019-01-19 20:18 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: joaotavora, emacs-devel

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

I urge everyone on the list to review the GNU Kind Communication
Guidelines, https://gnu.org/philosophy/kind-communication.html,
and think about how to practice kindness to others in this discussion.
It will make the discussion work better.

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)





^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-19 19:15                   ` João Távora
@ 2019-01-19 20:58                     ` Alan Mackenzie
  2019-01-19 23:18                       ` João Távora
  2019-01-21 18:04                       ` Stefan Monnier
  2019-01-19 22:37                     ` Drew Adams
  1 sibling, 2 replies; 27+ messages in thread
From: Alan Mackenzie @ 2019-01-19 20:58 UTC (permalink / raw)
  To: João Távora; +Cc: emacs-devel

Hello, João,

On Sat, Jan 19, 2019 at 19:15:25 +0000, João Távora wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > What you are in denial about, is that CC Mode and
> > electric-{pair,layout}-mode (as currently implemented) are incompatible
> > with each other.  I've explained this to you more times than I can
> > count, but you still refuse to accept it.

> How can I accept a something that you explain, but don't provide
> evidence of?

If I said the sky was blue would you say "where's your evidence?"?

It is a fact that c-electric-brace controls, and must control, each
change to the buffer which is done.  This is self evident from reading
the source code.  Please read the source code for c-electric-brace and
its immediate sub-functions, understand it (it's not hard), and then
come back to me with any questions you may have.

It is a fact that electric-layout-mode and electric-pair-mode are
allowed to run from post-self-insert-hook, that they make buffer changes
which are outside the control of c-electric-brace.  This is also self
evident.

To keep control of buffer changes, and thus function correctly,
c-electric-brace must thus prevent the uncontrolled changes referred to
in the previous paragraph.  This is simple logical reasoning from the
previous two paragraphs.

If these uncontrolled buffer changes are prevented, electric-layout-mode
and electric-pair-mode do not work.  I think this is obvious enough not
to need justification.

If these uncontrolled buffer changes are allowed to take place,
c-electric-brace doesn't work properly.  Evidence for this, if any be
needed, is in the scenario in bug #33794.

The two previous paragraphs indicate the incompatibility between
c-electric-brace, and electric-layout/pair-mode.

What part of this argument gives you difficulty?  Which link in the
chain of reasoning do you not accept?

> What you say: cc-mode + e-p-m + e-l-m => incompatible is certainly true
> *now*.  But it was't until you pushed that change.

Bug #33794 demonstrated the incompatibility.

> There is code in electric-pair-tests.el that shows this.  Spend a
> little less time handwaving and rewind the code back to before you
> pushed the change.  Then present an Emacs -Q recipe showing whatever
> you mean that I'm "in denial" about.

Bug #33794 is the recipe you require.

> *Then* we can discuss sanely.

I think it's worth pointing out that whilst you made a lot of noise on
the thread for #33794, you never characterised the cause of the bug, and
you didn't solve the bug either.

I invite you now to say what, at an abstract level, you think is the
cause of bug #33794.

> > OK, I'll make you a peace offering.  I'll put electric-pair-mode into
> > c-electric-lt-gt in the above contexts, and you will change the test
> > code so that in the C++ version of the < tests, lines start with
> > #include.

> It would pass those, but what if the user wants those for templates too?

My offer was to implement e-p-m for template delimiters, too.

> The goal is that he/she can program that into electric-pair-pairs and
> electric-pair-inhibit-predicate.

_Your_ goal, you mean.  I suggest that typical users just want
electric-pair-mode to work, without all the hassle of having to
configure low level things like electric-pair-pairs and having to write
a complicated lisp function to control it.

> But if you want to set electric-pair-inhibit-predicate yourself in the
> major-mode, that's perfectly OK.

I have no interest in manipulating electric-pair internals.  The test
for the syntactic context of the < and > signs would be done by CC Mode,
in c-electric-lt-gt.

> [ .... ] No idea if cc-mode has facilities to detect syntactic
> context, given C++ parsing is notoriously difficult.

It does indeed have such facilities.

> >> Major modes are "at liberty" to do that, but they break all minor-mode
> >> horizontal to Emacs that rely on this functionality, or have to
> >> reimplement them.
> > Minor modes shouldn't rely on this hook.  It's a mistaken design
> > decision.

> *In your opinion*.  Which, I'm very sorry to break it to you, is now
> irrelevant, because nevertheless it made it into Emacs's C core long
> ago, and people want to use it.

That may be true, but these people are mistaken, and deciding to use
this unreliable hook will come back and bite them.  Just look at the
trouble that e-p/l-m's' decision to use that hook is currently causing
us.

> And they do, and it has worked until now.

> > Despite your histrionics, electric-pair-mode and CC Mode work well
> > together.

> No, they don't Alan.  Pairing inside comments and literals is failing.

A detail, which I can fix.  Brace pairing already works in comments and
strings.

> And there's still the lost pairing behaviour in unterminated string
> literals.  All of these things used to work before you started messing
> with them around June.

I'll need to have a look at that.

> > Certainly the scenario in bug #33794 works well.  You keep slagging it
> > off, but still haven't given a specific defect in a usable form
> > (besides saying that e-p-m doesn't work in literals).

> I gave you 85 defects.  They are all in "usable" form.  Fix them all,
> repeat '(when electric-pair-mode' everywhere.

They are not in usable form.

> >> > You can have that preference, but given the incompatibility, it won't
> >> > get you anywhere. 
> >> I return the challenge.  Have you tried electric-layout-mode before
> >> making these claims?
> > No, I don't need to.

> Really, you don't have like 2 minutes to spare to try out 8 lines of
> elisp?

Not when there's no point, no.

> > I understand the mechanisms which give rise to the
> > incompatibility.  I urge you to try and understand these, too.

> What incompatibility?  Where?  When?  How?

<sigh> The incompatibility at the top of this post, which up till now
you've refused to accept, yet can't refute.

[ .... ]

> >> For the third of fourth time Have you actually *seen* and read the 8
> >> lines of code below that I posted already before?  Have you perhaps
> >> considered spending 5 minutes trying them out?
> > I've seen them, yes, and no I won't be trying them out.  I have no
> > interest in electric-layout-mode, beyond fending off attacks from it on
> > CC Mode.

> You crack me up Alan, you really have a bellic conception of software
> design.  In what ways does it attack CC Mode?  What with bananas and
> pointy sticks?

You are attacking CC Mode.  I identified you with your
electric-layout-mode.  That again is obvious, even to you.

> >> > No, I haven't tried it.  There is no usable interface with
> >> > electric-layout-mode for CC Mode.

> >> I just gave you one, for the second time.

> > No, I mean there's no interface by which CC Mode can trigger the actions
> > of electric-layout-mode.  Except, perhaps, by calling the function on
> > post-self-insert-hook as a function.  But how, in that case, does the

> It doesn't need to, my friend!  That's the beauty of it.  Just call
> self-insert-command like you always did!  And don't worry about it.

Bug #33794.

> > It's not good, but it's the best that's available.  You may recall me
> > requesting you to provide an interface suitable for CC Mode, but you
> > didn't do this.

> You requested that I redesign it around some other thing than
> post-self-insert-hook.  I can't do that.  It was like that when I picked
> it up.

I don't think my specific request was in those terms (I may be wrong),
but that certainly seems to be what's required.  Just because these
functions used post-self-insert-hook when you took them over doesn't
mean you can't change to something better.

[ .... ]

> >> * If c-auto-newline is on is works exactly like you want it;
> >> * Otherwise it works like it did before 223e7b87872d4a010ae1c9a6f09a9c15aee46692

> > No.

> See above.

> > It's low quality programming, having the low level details of a function
> > controlled by a flag with no coherent meaning set from outside.  It
> > will, one way or another, break things.

> Low quality programming?  From the man who is ad-hoc reimplementing
> electric-pair-mode?

You can't answer the point, so you descend to ad hominem attack.  I
would have expected something better from you.

[ .... ]

> João

> PS: do whatever you want now.  Revert my commit, if you want.  I'll
> remove the C++ tests.  It's not worth to insist on any of this anymore.
> I have a lot of work coming up myself.  I give up.

I think that's the best solution.  These tests are, after all, for
testing the electric- functions, not CC Mode.

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 27+ messages in thread

* RE: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-19 19:15                   ` João Távora
  2019-01-19 20:58                     ` Alan Mackenzie
@ 2019-01-19 22:37                     ` Drew Adams
  1 sibling, 0 replies; 27+ messages in thread
From: Drew Adams @ 2019-01-19 22:37 UTC (permalink / raw)
  To: João Távora, Alan Mackenzie; +Cc: emacs-devel

> How can I accept a something that you explain, but don't provide
> evidence of?  It's like wanting me to accept I'm swedish.

João, just accept it. Stop denying your Swedishness. ;-)


(Just kidding.  Apologies to all.  I shouldn't take
this thread so lightly, but couldn't resist.)



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-19 20:58                     ` Alan Mackenzie
@ 2019-01-19 23:18                       ` João Távora
  2019-01-21 18:04                       ` Stefan Monnier
  1 sibling, 0 replies; 27+ messages in thread
From: João Távora @ 2019-01-19 23:18 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: emacs-devel

> the source code.  Please read the source code for c-electric-brace and
> Bug #33794 demonstrated the incompatibility.
> Bug #33794 is the recipe you require.

Here's a little code for you to read:

(equal '(cc-mode e-p-m e-l-m) '(cc-mode e-p-m auto-newline-mode)) ;; => nil

Now here's what I wrote

>> What you say: cc-mode + e-p-m + e-l-m => incompatible is certainly true
                 ^^^^^^^^^^^^^^^^^^^^^^^

In bug#33794 you specifically asked the issuer to *not* try the cc-mode
+ e-p-m + e-l-m.  I showed at the time that it worked, I even improved
e-l-m to make it worked specifically for that.

And obviously cc-mode + e-p-m + e-l-m + auto-newline-mode make no sense,
I never said it did.

I didn't read the rest of your mail, sorry.  This has become a sad
spectacle already (though Drew's mail really made me laugh).  Apologies
to all for watching this bickering.

As I said, do what you want to cc-mode and I'll disable c++-tests in the
meantime.

João



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-19 13:52               ` João Távora
  2019-01-19 17:45                 ` Alan Mackenzie
@ 2019-01-20 19:05                 ` Richard Stallman
  2019-01-20 21:18                   ` João Távora
  1 sibling, 1 reply; 27+ messages in thread
From: Richard Stallman @ 2019-01-20 19:05 UTC (permalink / raw)
  To: João Távora; +Cc: acm, emacs-devel

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > That's not for you to decide whether electric-pair-mode, an optional
  > minor mode with a well-defined contract, is broken.

When you state disagreement with someone, please don't use a harsh
tone.  We are all working together here.  So let's be kind to each
other.

Whether it is broken is not a decision, but a judgment.  People can
judge whether the current behavior is useful or not.  People's
judgments on this may disagree.

Alan can't personally tell other people how to judge the question,
but it is useful for him to make the case for his viewpoint.

After people discuss that question, there will be a decision to make
-- whether to change the behavior, and if so how.  The Emacs
maintainers are ultimately in charge of that decision.

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)





^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-20 19:05                 ` Richard Stallman
@ 2019-01-20 21:18                   ` João Távora
  2019-01-21 20:59                     ` Richard Stallman
  0 siblings, 1 reply; 27+ messages in thread
From: João Távora @ 2019-01-20 21:18 UTC (permalink / raw)
  To: Richard Stallman; +Cc: acm, emacs-devel

Richard Stallman <rms@gnu.org> writes:

> When you state disagreement with someone, please don't use a harsh
> tone.  We are all working together here.  So let's be kind to each
> other.

Hello Richard,

Richard, I fully understand that.  But I must remind you, at this point,
that it was Alan who used the words "stupid", "histrionic", "in denial",
"silly" and "low quality" to characterize my position.  Earlier I myself
used the term "atrocity" to characterize his, but I later abandoned this
language, because these are not objective ways to characterize someone's
viewpoint.

I will of course accept that insults aren't the only way to be harsh, so
I must now proceed to explain why I behaved so.  Alan repeatedly
misstates a "fact" without providing evidence.  This is very
exasperating.

Because saying that itself requires justification, I must present my it
as abbreviated as I can: Alan says that the Emacs minor modes
'electric-pair-mode' (hereafter abbreviated e-p-m) and
'electric-layout-mode' (abbrev. e-l-m) are incompatible with cc-mode.
This is strictly true _now_, at the current state of the Emacs source
tree, but it was _not_ so before Alan's change 223e7b8787 of pushed in
2019-01-15.

In fact, I presented a short section of code (which I should like to
improve and install in the future), which demonstrates this.  But Alan
refuses to acknowledge it.  (a variation on this snippet is still
installed in the Emacs source tree under test/lisp/electric-tests.el but
it is not activated to the user, while an updated version of the snippet
I sent to Alan is attached after my signature).

I encouraged Alan to roll back the code to before his change and try
that very simple piece of code.  With it, he could either directly point
to a misbehaviour I'm not aware of, thereby partly justfying his claim.
In that purely hypothetical case I could either fix the misbehaviour or
accept that there is indeed a fundamental incompatibility and give up.

But Alan consistently refuses to do, leaving me with no evidence of his
claim.  Instead, Alan points to bug#33794.  In that bug, a user states
the following:

   https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33794#1

   > When using cc-mode, turning on electric-pair-mode causes the
   > auto-newline minor mode to stop inserting newlines where expected.
   > [...]

She mentions a true, never disputed, incompatibility in the combination
'cc-mode', 'e-p-m', and 'auto-newline' (the latter also known as
'c-auto-newline'), which is notably not the same combination as cc-mode,
e-p-m, and e-l-m.  If you read through that bug, you will read efforts
from my part to encourage the user to try e-l-m *instead* of
'c-auto-newline', paired with efforts to improve 'e-l-m' so that the
user's wishes can be fulfilled.  Those efforts are reviewed by Stefan
Monnier, installes in Emacs' e-l-m (not touching any of cc-mode) and
after some confusion those efforts succeed and the user eventually
understands she has an alternative in the combination cc-mode, e-p-m and
e-l-m:

  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=33794#83

  Beatrix>> Things such as c-toggle-auto-newline, for example, almost seem in this
  >> case that they might be better delegated to electric-layout-mode,
  >> with cc-mode specifying different electric-layout constraints for
  >> its different formatting styles. It seems this is close to what
  >> João was suggesting?
  > 
  JT> Yes, that is precisely what I am suggesting.  I am happy that this
  JT> point made it across.

At this point, the incompatibility between the original combination
cc-mode, e-p-m, and auto-newline remained, and Alan decided, quite
legitimately, to address it: there is no reason why the original bug
reported shouldn't be fixed.

However, he chose a way of doing so that denies the alternative I had
provided earlier (additionally, it opened many other new bugs in simpler
usage combinations that he is only now addressing).  At first, I thought
it was an oversight.  After complaining of the new bugs, I suggested
further alternatives so that *both* combinations can be available to the
user.

I presented actual, very simple, working code that does this.  Code that
would even very slightly reduce the line-of-count count in C++ mode.
But Alan rejects this: he wants the keep the current status quo that his
combination works but the alternative I presented earlier user doesn't,
effectively shutting the user out of using 'electric-layout-mode' in
CC-mode, something he deems "silly".

There are, of course, technical ways around this blockade, but they are,
at the moment, too technical for users to try.  More importantly, they
didn't need to exist at all if Alan just didn't needlessly block e-l-m.

I would like to think that you, Richard, of all people, are sensitive to
one of the issues here, which is about shutting out users out of
software.

Perhaps you have learned in your life to argue this less harshly, and
more effectively, than I did.

Best,
João

(setq electric-layout-rules '(electric-layout-for-c-style-du-jour))

(defun electric-layout-brace-for-c-style-du-jour (inserted)
  (when (and (derived-mode-p 'c-mode 'c++-mode)
             (memq inserted '(?{ ?})))
    (save-excursion
      ;; seem to need this extra call before the one we're interested in
      (backward-char) (c-point-syntax) (forward-char)
      (backward-char)
      (c-brace-newlines (c-point-syntax)))))

(electric-pair-mode 1) (electric-layout-mode 1)



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-19 20:58                     ` Alan Mackenzie
  2019-01-19 23:18                       ` João Távora
@ 2019-01-21 18:04                       ` Stefan Monnier
  2019-01-21 20:45                         ` Alan Mackenzie
  1 sibling, 1 reply; 27+ messages in thread
From: Stefan Monnier @ 2019-01-21 18:04 UTC (permalink / raw)
  To: emacs-devel

> If I said the sky was blue would you say "where's your evidence?"?
>
> It is a fact that c-electric-brace controls, and must control, each
> change to the buffer which is done.

Not only I don't think that this is true, but I don't even know exactly
what it means (IOW, yes it's probably true for some interpretation of
it, but not for others).

> This is self evident from reading the source code.

I don't see this evidence.

> Please read the source code for c-electric-brace and its immediate
> sub-functions, understand it (it's not hard), and then come back to me
> with any questions you may have.

expand-abbrev also can be extensive changes during self-insert-command,
and so can the auto-fill-function.

> It is a fact that electric-layout-mode and electric-pair-mode are
> allowed to run from post-self-insert-hook, that they make buffer changes
> which are outside the control of c-electric-brace.

If you use electric-layout-mode and electric-pair-mode, then all that's
left for c-electric-brace is to call self-insert-command.
And indeed, I hope in the future that the { and } bindings will simply
be removed from CC-mode.

I understand that there's a transition needed between these two and this
intermediate state can require more work, but it's important to keep the
long term goal in mind when designing the current solution.


        Stefan




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-21 18:04                       ` Stefan Monnier
@ 2019-01-21 20:45                         ` Alan Mackenzie
  2019-01-21 21:46                           ` Stefan Monnier
  0 siblings, 1 reply; 27+ messages in thread
From: Alan Mackenzie @ 2019-01-21 20:45 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Hello, Stefan.

On Mon, Jan 21, 2019 at 13:04:57 -0500, Stefan Monnier wrote:
> > If I said the sky was blue would you say "where's your evidence?"?

> > It is a fact that c-electric-brace controls, and must control, each
> > change to the buffer which is done.

> Not only I don't think that this is true, but I don't even know exactly
> what it means (IOW, yes it's probably true for some interpretation of
> it, but not for others).

> > This is self evident from reading the source code.

> I don't see this evidence.

Look in c-do-brace-electrics in an up to date (i.e. within the last few
days) version of cc-cmds.el.  This function is called from several
places in c-electric-brace.

About 60% the way through is the "brace-else-brace" cleanup.  It works
by scanning the buffer for a particular pattern.  If arbitrary changes
(i.e. those not controlled by the major mode) have been made to the
buffer, the pattern might not be found when it should be.  For example.

This was precisely the cause of bug #33794.  The user typed "{", but
unknown to c-electric-brace, self-insert-command randomly inserted "{}".
This extra "}" prevented auto-newline from working.

With the advent of post-self-insert-hook, it is clear that calling
self-insert-command from a function can lead to arbitrary buffer
changes.  To prevent this, c-electric-brace and friends bind
post-self-insert to nil.  Another way to proceed would be your recent
suggestion of using insert rather than self-insert-command.

It is thus clear that the use of post-self-insert-hook is incompatible
with Lisp code which calls self-insert-command, since that use
effectively makes that function undefined.  The way this hook is
typically used is much more like advice (which modifies a function's
function) than a traditional hook (where the hook performs things
incidental to what triggers it, leaving the triggering event untouched).

> > Please read the source code for c-electric-brace and its immediate
> > sub-functions, understand it (it's not hard), and then come back to me
> > with any questions you may have.

> expand-abbrev also can be extensive changes during self-insert-command,
> and so can the auto-fill-function.

In practice, they haven't done so.  If they did, then using insert would
solve the problem.

> > It is a fact that electric-layout-mode and electric-pair-mode are
> > allowed to run from post-self-insert-hook, that they make buffer changes
> > which are outside the control of c-electric-brace.

> If you use electric-layout-mode and electric-pair-mode, then all that's
> left for c-electric-brace is to call self-insert-command.

No.  There's more to it than that.

> And indeed, I hope in the future that the { and } bindings will simply
> be removed from CC-mode.

Maybe in the medium future, when the pertinent generic functions are in
all Emacs versions supported by CC Mode (assuming that XEmacs support
will cease quite soon).

These generic functions have some way to go before they match the
functionality of CC Mode's own features.

> I understand that there's a transition needed between these two and this
> intermediate state can require more work, but it's important to keep the
> long term goal in mind when designing the current solution.

Whose long term goal?  My goal, both long and short term, is to keep CC
Mode working properly.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-20 21:18                   ` João Távora
@ 2019-01-21 20:59                     ` Richard Stallman
  2019-01-21 21:49                       ` João Távora
  0 siblings, 1 reply; 27+ messages in thread
From: Richard Stallman @ 2019-01-21 20:59 UTC (permalink / raw)
  To: João Távora; +Cc: acm, emacs-devel

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

Most of your message addresses the problem in Emacs.  That needs to be
understood and handled, but not by me.  I'm focusing on the
conversation problem that I saw.

  > Richard, I fully understand that.  But I must remind you, at this point,
  > that it was Alan who used the words

I noticed your message, and commented that it seemed harsh.  Perhaps
if I had noticed other people's messages I would have said the same thing
about those messages.

We should all make an effort to restrain ourselves from becoming
harsh.  That's each and every one of us, including me.  I'm paying
attention to this right now.

To ask "who started it" is to oversimplify.  Often what happens
is that a little harshness creeps into a discussion, then people
react to that in a way that is a little more harsh.  So nobody
"started it" but multiple people exacerbated it.

Thus, part of the effort is, when we feel harshness coming at us,
to damp it down rather than hitting back.

We will all have opportunities to do that.  We can all use practice.

Have you read https://gnu.org/philosophy/kind-communication.html?

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)





^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-21 20:45                         ` Alan Mackenzie
@ 2019-01-21 21:46                           ` Stefan Monnier
  2019-01-21 22:41                             ` João Távora
  0 siblings, 1 reply; 27+ messages in thread
From: Stefan Monnier @ 2019-01-21 21:46 UTC (permalink / raw)
  To: emacs-devel

> It is thus clear that the use of post-self-insert-hook is incompatible
> with Lisp code which calls self-insert-command, since that use
> effectively makes that function undefined.

self-insert-command is a command more than a function, so code that
calls it should expect it to do "whatever happens when the user hits
the corresponding key", more or less.  Given all the things
self-insert-command does, there's very little you can rely on.
If you need to know more precisely what happens, then `insert` is
usually a better option.

>> expand-abbrev also can be extensive changes during self-insert-command,
>> and so can the auto-fill-function.
> In practice, they haven't done so.

I'm sure some users of Emacs have used abbrevs or auto-fill-functions in
very funny ways which break your code's assumptions.  Just because you
haven't heard from them doesn't mean the problem doesn't exist.

>> If you use electric-layout-mode and electric-pair-mode, then all that's
>> left for c-electric-brace is to call self-insert-command.
> No.  There's more to it than that.

You mean the c-cleanup-list, I guess.  Indeed, before c-electric-brace
can be turned into an alias for self-insert-command, we'd need to add
another post-self-insert-hook which implements the
c-cleanup-list feature.

AFAICT from the docstring, it's the only thing still missing.

>> I understand that there's a transition needed between these two and this
>> intermediate state can require more work, but it's important to keep the
>> long term goal in mind when designing the current solution.
> Whose long term goal?

At the very least mine.

> My goal, both long and short term, is to keep CC Mode
> working properly.

That's orthogonal.

To give you a similarly general goal, my own goal is to make it so that
any feature which makes sense in many major mode be implemented once and
then used in the relevant major modes, rather than being implemented
independently for those major modes.

This is both for uniformity between major modes, and because it both
simplifies and improves many major modes (which would otherwise either
not provide the feature or only in very primitive ways).

And those maintainers like you who did go to the trouble of being early
implementers of the feature suffer through the pain of having to adapt
their code once the generic version of the feature becomes available.
Sometimes even at the cost of having the new feature working slightly
worse in some corner cases.

But many of them are quite happy to be able to drop their old code and
get rid of that responsibility (i.e. bug reports about regressions can
be redirected to Emacs maintainers).


        Stefan




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-21 20:59                     ` Richard Stallman
@ 2019-01-21 21:49                       ` João Távora
  0 siblings, 0 replies; 27+ messages in thread
From: João Távora @ 2019-01-21 21:49 UTC (permalink / raw)
  To: Richard Stallman; +Cc: acm, emacs-devel

Richard Stallman <rms@gnu.org> writes:

> To ask "who started it" is to oversimplify.

Of course, and this was not my thesis at all.  I had hoped it had become
clear in my message, but I'll state it again: _I started it_, at least I
started this newest string of "harshnesses".  And then I proceeded to
describe the reasons why I did so and also why I ended it.  If you don't
care at the moment for the material causes leading up to this, that's
quite understandable, since they are mostly technical.  But I had to
provide them nonetheless.

> Have you read https://gnu.org/philosophy/kind-communication.html?

No.  Only very lightly.  You have my word I will do so soon.

João



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.
  2019-01-21 21:46                           ` Stefan Monnier
@ 2019-01-21 22:41                             ` João Távora
  0 siblings, 0 replies; 27+ messages in thread
From: João Távora @ 2019-01-21 22:41 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> If you use electric-layout-mode and electric-pair-mode, then all that's
>>> left for c-electric-brace is to call self-insert-command.
>> No.  There's more to it than that.
> You mean the c-cleanup-list, I guess.  Indeed, before c-electric-brace
> can be turned into an alias for self-insert-command, we'd need to add
> another post-self-insert-hook which implements the
> c-cleanup-list feature.

According to its docstring, c-cleanup-list only takes effect when
c-auto-newline is on.  So, in the interim we could consider doing either
this:

  (let ((post-self-insert-hook (unless c-auto-newline post-self-insert-hook)))
    (self-insert-command arg))

Or maybe just

  (let ((electric-layout-mode (not c-auto-newline))
        (electric-pair-mode (not c-auto-newline)))
     (self-insert-command arg))

As fair as I read the code, it is only when c-auto-newline is turned on
that any problems stated thus far exist.  My proposal is that e-l-m and
e-p-m defer to c-auto-newline, if is turned on.

This buys us time while we develop a potentially better alternative that
could, if Alan accepts it, effectively _become_ c-auto-newline.  Or, if
Alan chooses not to, become just an alternative way to do the same.
It's not optimal, but I could live with that.

But during that time (and notably if i.e. Emacs 27 is frozen), then
users that _don't_ use c-auto-newline can enjoy an electric-pair-mode,
electric-layout-mode, or any other global minor-mode that uses on
post-self-insert-hook without any regressions in behaviour.

As the code stands now, if we don't do anything, there's a good chance
that users will complain (I remind everyone that CC-mode is used for the
C++/C/Java modes).

Users that _do_ use c-auto-newline see improved behaviour in some
cc-mode areas, such as partial e-p-m support, and perhaps degraded
behaviour in others, namely if they use other global minor-modes.

João







^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2019-01-21 22:41 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-17 14:57 Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it João Távora
2019-01-17 16:43 ` Alan Mackenzie
2019-01-17 17:57   ` João Távora
2019-01-17 18:55     ` João Távora
2019-01-18 17:54     ` Alan Mackenzie
2019-01-18 19:55       ` João Távora
2019-01-18 22:53         ` Alan Mackenzie
2019-01-19  3:18           ` João Távora
2019-01-19 11:07             ` Alan Mackenzie
2019-01-19 13:52               ` João Távora
2019-01-19 17:45                 ` Alan Mackenzie
2019-01-19 19:15                   ` João Távora
2019-01-19 20:58                     ` Alan Mackenzie
2019-01-19 23:18                       ` João Távora
2019-01-21 18:04                       ` Stefan Monnier
2019-01-21 20:45                         ` Alan Mackenzie
2019-01-21 21:46                           ` Stefan Monnier
2019-01-21 22:41                             ` João Távora
2019-01-19 22:37                     ` Drew Adams
2019-01-20 19:05                 ` Richard Stallman
2019-01-20 21:18                   ` João Távora
2019-01-21 20:59                     ` Richard Stallman
2019-01-21 21:49                       ` João Távora
2019-01-18 21:06       ` Stefan Monnier
2019-01-19  3:25         ` João Távora
2019-01-18 22:47       ` Michael Albinus
2019-01-19 20:18       ` Richard Stallman

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