From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: =?UTF-8?B?Sm/Do28gVMOhdm9yYQ==?= Newsgroups: gmane.emacs.devel Subject: Re: Apropos 54f297904e0c: Temporarily comment out CC Mode from tests which are incompatible with it. Date: Thu, 17 Jan 2019 17:57:24 +0000 Message-ID: References: <20190117164350.GA18314@ACM> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Trace: blaine.gmane.org 1547747823 8124 195.159.176.226 (17 Jan 2019 17:57:03 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Thu, 17 Jan 2019 17:57:03 +0000 (UTC) Cc: emacs-devel To: Alan Mackenzie , Beatrix Klebe Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Jan 17 18:56:59 2019 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1gkBuR-0001uE-GK for ged-emacs-devel@m.gmane.org; Thu, 17 Jan 2019 18:56:55 +0100 Original-Received: from localhost ([127.0.0.1]:49213 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gkBwY-0001QW-NG for ged-emacs-devel@m.gmane.org; Thu, 17 Jan 2019 12:59:06 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:48780) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gkBv8-0000XK-Q1 for emacs-devel@gnu.org; Thu, 17 Jan 2019 12:57:39 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gkBv7-0003cq-H8 for emacs-devel@gnu.org; Thu, 17 Jan 2019 12:57:38 -0500 Original-Received: from mail-qt1-x829.google.com ([2607:f8b0:4864:20::829]:46136) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gkBv7-0003a7-51 for emacs-devel@gnu.org; Thu, 17 Jan 2019 12:57:37 -0500 Original-Received: by mail-qt1-x829.google.com with SMTP id y20so12234124qtm.13 for ; Thu, 17 Jan 2019 09:57:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=5001cm0ohcG5c/NdMqCZV+O0M51jgDLT1Ctf9uykim8=; b=NYdVXpxY0L9J+TLvPCRBEIS3+Egvv2Koyx+mLQ40JTijoOLvEKn1ViIoRhv8aCnE9r T6FGi6mwAX2qMP7NtO6DcccfeBCupPFYpvtr4CaFhAwDKQQL+810uBYgl/BOv133NC23 SJdmDW4E0iBGO8N3vwRqJjHeLtMfDH0/IjA/UZMZKNLSO0PcpvoNyHpjhr5M0NJ20BXc ZMVNCArDqeJS40MEeaYqaVI2ojRgg8tgjkVNT1lwvRWFAmWUWDe6ijTDqwJiNf3V9Hw4 XMMFa/fr8WvFBL8fmADa0v0ejNpGTN/YZDDm0hcBOddQSJNvkSNC+I+yCtoKPoAazqmQ gzlw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=5001cm0ohcG5c/NdMqCZV+O0M51jgDLT1Ctf9uykim8=; b=Oxr5Sou8Rl75M5P8yVfsLZWJM86LZN0lLqm6qxT4zY48mva14ttoLNX687TgdCGvBe pqAhdrN6jCL/L8ROXjqBDGfKCgAlkxjgc63vUAkcQ8ARv02prbKHkLcRmulvGKudpGJ6 XE8pFDkRx2v9L0Fjk0Zehazl1s45TXOiY7KJ827fLi4e/G8AB1UYwZEsGVdxildRIaOL qOtRwocpYvGmwL+u2aWNtnbJDKNqCjgqyUHEeBMpX/gEdAVGbUy73fIhQ7CHYmkFrR4d G+MKHKKSMPMv1dkeO75q8hn5ck5Jksqd1/lX2sn0XMVSRqZRflJf2+V2pTwhNyxxy2H8 r2+g== X-Gm-Message-State: AJcUukfr8pULh3zhDi3BMYUKq48SKCehqUC6UkZAWTkNKNZiO0dHkMlW HGCMNObQyKH32mAuErN0ay8vWVptYw6IHI7yx8A= X-Google-Smtp-Source: ALg8bN7uXmbfkvWq0Bz7ZIc+Hr2D2hzFTAkX9vPDqevSio68JFhcl0TZA6dauk3a3qVvUbJi9KAGr8HV7Vh7qUJNmQ0= X-Received: by 2002:a0c:b919:: with SMTP id u25mr12434096qvf.104.1547747855736; Thu, 17 Jan 2019 09:57:35 -0800 (PST) In-Reply-To: <20190117164350.GA18314@ACM> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4864:20::829 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:232433 Archived-At: Hello again, Alan On Thu, Jan 17, 2019 at 4:54 PM Alan Mackenzie 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 w= hat needs to be fixed, but it works for me and I don't have electric-layout-mod= e, 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 m= ight 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 o= n 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 mo= st > 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=C3=A3o