Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it.

[João Távora]

Hello again, Alan

On Thu, Jan 17, 2019 at 4:54 PM Alan Mackenzie <address@hidden> 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