Re: master 71ef012: Map redo records for undo in region to 'undo-in-region

Re: master 71ef012: Map redo records for undo in region to 'undo-in-region

[Lars Ingebrigtsen]

monnier@iro.umontreal.ca (Stefan Monnier) writes:

> branch: master
> commit 71ef0122abf5215eafa2dc414b75630a709de008
> Author: Yuan Fu <casouri@gmail.com>
> Commit: Stefan Monnier <monnier@iro.umontreal.ca>
>
>     Map redo records for undo in region to 'undo-in-region
>
>     * lisp/simple.el (undo-equiv-table): Add explaination for
>     undo-in-region, undo to the beginning of undo list and null undo.
>     (undo): If equiv is 'undo-in-region, empty or t, set pending-undo-list
>     to t.  If the redo is undo-in-region, map buffer-undo-list to
>     'undo-in-region instead of t, if it is an identity mapping, map to
>     'empty.

This seems to lead to the following two test failures:

2 unexpected results:
   FAILED  simple-tests--undo
   FAILED  simple-tests--undo-equiv-table

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

Re: master 71ef012: Map redo records for undo in region to 'undo-in-region

[Yuan Fu]

> On Mar 11, 2021, at 7:26 PM, Lars Ingebrigtsen <larsi@gnus.org> wrote:
> 
> monnier@iro.umontreal.ca (Stefan Monnier) writes:
> 
>> branch: master
>> commit 71ef0122abf5215eafa2dc414b75630a709de008
>> Author: Yuan Fu <casouri@gmail.com>
>> Commit: Stefan Monnier <monnier@iro.umontreal.ca>
>> 
>>    Map redo records for undo in region to 'undo-in-region
>> 
>>    * lisp/simple.el (undo-equiv-table): Add explaination for
>>    undo-in-region, undo to the beginning of undo list and null undo.
>>    (undo): If equiv is 'undo-in-region, empty or t, set pending-undo-list
>>    to t.  If the redo is undo-in-region, map buffer-undo-list to
>>    'undo-in-region instead of t, if it is an identity mapping, map to
>>    'empty.
> 
> This seems to lead to the following two test failures:
> 
> 2 unexpected results:
>   FAILED  simple-tests--undo
>   FAILED  simple-tests--undo-equiv-table
> 

How is the test above ran? Maybe I can setup something similar and investigate. All the tests passed on my machine (with master and -q).

Yuan

Re: master 71ef012: Map redo records for undo in region to 'undo-in-region

[Lars Ingebrigtsen]

Yuan Fu <casouri@gmail.com> writes:

>> This seems to lead to the following two test failures:
>> 
>> 2 unexpected results:
>>   FAILED  simple-tests--undo
>>   FAILED  simple-tests--undo-equiv-table
>> 
>
> How is the test above ran? Maybe I can setup something similar and
> investigate. All the tests passed on my machine (with master and -q).

cd test/; make simple-tests

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

Re: master 71ef012: Map redo records for undo in region to 'undo-in-region

[Yuan Fu]

> On Mar 11, 2021, at 8:40 PM, Lars Ingebrigtsen <larsi@gnus.org> wrote:
> 
> Yuan Fu <casouri@gmail.com> writes:
> 
>>> This seems to lead to the following two test failures:
>>> 
>>> 2 unexpected results:
>>>  FAILED  simple-tests--undo
>>>  FAILED  simple-tests--undo-equiv-table
>>> 
>> 
>> How is the test above ran? Maybe I can setup something similar and
>> investigate. All the tests passed on my machine (with master and -q).
> 
> cd test/; make simple-tests
> 

I see. It fails when executing regional undo. Does Emacs not have regional undo when running in batch mode? (Maybe push-mark-command behaves differently?)

Yuan

Re: master 71ef012: Map redo records for undo in region to 'undo-in-region

[Lars Ingebrigtsen]

Yuan Fu <casouri@gmail.com> writes:

> I see. It fails when executing regional undo. Does Emacs not have
> regional undo when running in batch mode? (Maybe push-mark-command
> behaves differently?)

I'm not sure, but I see that Glenn has disabled the tests for now, so
perhaps he knows more.

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

Re: master 71ef012: Map redo records for undo in region to 'undo-in-region

[Yuan Fu]

[-- Attachment #1: Type: text/plain, Size: 732 bytes --]



> On Mar 12, 2021, at 12:00 PM, Lars Ingebrigtsen <larsi@gnus.org> wrote:
> 
> Yuan Fu <casouri@gmail.com> writes:
> 
>> I see. It fails when executing regional undo. Does Emacs not have
>> regional undo when running in batch mode? (Maybe push-mark-command
>> behaves differently?)
> 
> I'm not sure, but I see that Glenn has disabled the tests for now, so
> perhaps he knows more.
> 
> -- 
> (domestic pets only, the antidote for overdose, milk.)
>   bloggy blog: http://lars.ingebrigtsen.no

This patch should fix it for batch mode. I’m not sure if I need to mention Glenn’s commit in the commit message (like “Revert commit xxx" or something), maybe someone can edit it to the correct format.

Yuan


[-- Attachment #2: fix-test.patch --]
[-- Type: application/octet-stream, Size: 1472 bytes --]

From fd3a6ce674c86af42087c685c514dbd1b9909f81 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Fri, 12 Mar 2021 12:18:22 -0500
Subject: [PATCH] Fix simple-tests

* test/lisp/simple-tests.el (simple-tests--undo-in-region,
simple-tests--undo-equiv-table): Re-enable in batch mode. Enable
'transient-mark-mode' in temp buffer.
---
 test/lisp/simple-tests.el | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/test/lisp/simple-tests.el b/test/lisp/simple-tests.el
index 8fa8295765..601eca6cd4 100644
--- a/test/lisp/simple-tests.el
+++ b/test/lisp/simple-tests.el
@@ -469,8 +469,11 @@ simple-tests--undo
 
 (ert-deftest simple-tests--undo-in-region ()
   ;; Test undo/redo in region.
-  (skip-unless (not noninteractive))
   (with-temp-buffer
+    ;; Enable `transient-mark-mode' so `region-active-p' works as
+    ;; expected. `region-active-p' is used to determine whether to
+    ;; perform regional undo in `undo'.
+    (transient-mark-mode)
     (buffer-enable-undo)
     (dolist (x '("a" "b" "c" "d" "e"))
       (insert x)
@@ -506,9 +509,9 @@ simple-tests--sans-leading-nil
   lst)
 
 (ert-deftest simple-tests--undo-equiv-table ()
-  (skip-unless (not noninteractive))
   (with-temp-buffer
     (buffer-enable-undo)
+    (transient-mark-mode)
     (let ((ul-hash-table (make-hash-table :test #'equal)))
       (dolist (x '("a" "b" "c"))
         (insert x)
-- 
2.24.3 (Apple Git-128)

Re: master 71ef012: Map redo records for undo in region to 'undo-in-region

[Glenn Morris]

Lars Ingebrigtsen wrote:

> I'm not sure, but I see that Glenn has disabled the tests for now, so
> perhaps he knows more.

I know nothing, I'm just really tired of people breaking the test suite.

Re: master 71ef012: Map redo records for undo in region to 'undo-in-region

[Yuan Fu]

> On Mar 12, 2021, at 12:31 PM, Glenn Morris <rgm@gnu.org> wrote:
> 
> Lars Ingebrigtsen wrote:
> 
>> I'm not sure, but I see that Glenn has disabled the tests for now, so
>> perhaps he knows more.
> 
> I know nothing, I'm just really tired of people breaking the test suite.

Sorry Glenn, I didn’t know that running the test in batch mode would turn out differently. Do you see the patch I attached to my previous message? That should fix the problem.

Yuan

Re: master 71ef012: Map redo records for undo in region to 'undo-in-region

[Stefan Monnier]

> This patch should fix it for batch mode. I’m not sure if I need to mention
> Glenn’s commit in the commit message (like “Revert commit xxx" or
> something), maybe someone can edit it to the correct format.

LGTM, pushed to `master`, thanks,


        Stefan