GNU bug report logs - #65734
29.1.50; kill-whole-line and visibility of Org subtrees

Previous Next

Full log

Message #95 received at 65734 <at> debbugs.gnu.org (full text, mbox):

From: Sebastian Miele <iota <at> whxvd.name>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: yantar92 <at> posteo.net, 65734 <at> debbugs.gnu.org
Subject: Re: bug#65734: [BUG] kill-whole-line on folded subtrees [9.6.8
 (release_9.6.8-3-g21171d @ /home/w/usr/emacs/0/29/0/lisp/org/)]
Date: Mon, 25 Dec 2023 19:53:36 +0100

[Message part 1 (text/plain, inline)]

Attached are two patches.  The first patch introduces the tests,
including two tests that are expected to be failing for the current
`kill-whole-line'.

The test `kill-whole-line-read-only' does not fail because of the bug
reported in this bug report, but because of another bug that I stumbled
upon while investigating and testing: `kill-whole-line' always kills by
two calls to `kill-region'.  When the buffer is readonly, the first of
the two calls to `kill-region' errors out / exits non-locally.  That
causes `kill-region' to omit to put the remaining stuff (from the second
`kill-region') into the kill ring.

The second patch fixes both bugs, and removes the corresponding
`:expected-result :failed' from the tests.

> From: Eli Zaretskii <eliz <at> gnu.org>
> Date: Sun, 2023-09-10 19:57 +0300
>
>> From: Sebastian Miele <iota <at> whxvd.name>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>, Ihor Radchenko <yantar92 <at> posteo.net>
>> Date: Sun, 10 Sep 2023 18:31:20 +0200
>>
>> I removed emacs-orgmode <at> gnu.org from CC.
>>
>> > From: Sebastian Miele <iota <at> whxvd.name>
>> > Date: Wed, 2023-09-06 15:30 +0200
>> >
>> > I will write the tests.  And I will probably come up with an updated
>> > version of the original patch.  There is at least one cosmetic change.
>> > And something else that I want to have tried.  May take some time.
>>
>> Please have a look at the following patch.  For now it contains three
>> tests, two of them with :expected-result :failed.  (They do not fail on
>> the bug-fixed version of `kill-whole-line'.)
>
> Yes, there should be more tests, ideally: there are situations where
> kill-whole-line signals an error, and I don't think I see tests where
> some of the text is invisible (as the function uses
> forward-visible-line and end-of-visual-line).

I added tests for cases when `kill-whole-line' signals errors.

The tests for the invisible stuff were already there, in the test
`kill-whole-line-invisible'.  The calls that introduce invisibility and
`after-change-functions' in the test are:

  (org-mode)
  (org-fold-hide-sublevels 1)

Even though its only five tests, I think they are rather very thorough,
now.  Except one test, all tests tests have several subtests.  All
branches of `kill-whole-line' are covered.

>> There probably will be more tests and further questions.  But for
>> now, I would like to basically have a statement of whether the style
>> of writing the tests goes in an acceptable direction.
>
> Looks reasonable, but I'm not sure I understand what will the test
> show if one of the tests fails: will the information shown then tell
> enough to understand which of the sub-tests failed and why?

I found the `ert-info' macro which allows to add arbitrary annotiations
to `should's in its body, used it for all subtests.  They now are
clearly distinguishable in the output of `ert', even when run from a
terminal.

[tests.patch (text/x-patch, attachment)]

[fix.patch (text/x-patch, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.