bug#42218: 28.0.50; Fix for missed item undecoration on allout-widgets-mode deactivation

bug#42218: 28.0.50; Fix for missed item undecoration on allout-widgets-mode deactivation

[Ken Manheimer]

[-- Attachment #1.1: Type: text/plain, Size: 3982 bytes --]

The attached patch fixes the bug described below.

In the current version (the current Gnu emacs repository master) of
allout-widgets-mode, exiting the mode fails to undecorate the first item if
it's at the start of the buffer. You can confirm this by creating a simple
outline like the following (indented to distinguish it, the bullets need to
be at the left margin in the actual text):

* First item
.* Second item


Then activate allout-mode and allout-widgets-mode, and then
deactivate allout-widgets-mode. The widget decorations will not be properly
removed from the first item.

The attached patch rectifies this bug so all decorations are properly
removed.


In GNU Emacs 28.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.22.30,
cairo version 1.15.10)
 of 2020-07-04 built on gram
Repository revision: 5d1bac0ac951e25d0b0b39a9919f13053162d5df
Repository branch: master
Windowing system distributor 'HC-Consult', version 11.0.12005001
System Description: Ubuntu 18.04.4 LTS

Recent messages:
Allout-Widgets mode disabled in current buffer
M-o C-x o is undefined
Quit
Mark set [2 times]
Quit
Allout mode enabled in current buffer
Allout-Widgets mode enabled in current buffer
M-x is undefined
M-p is undefined
Allout-Widgets mode disabled in current buffer

Configured using:
 'configure --with-gnutls=ifavailable --with-x-toolkit=yes
 --with-xpm=ifavailable --with-jpeg=ifavailable --with-gif=ifavailable
 --with-tiff=ifavailable'

Configured features:
PNG CAIRO SOUND DBUS GSETTINGS GLIB NOTIFY INOTIFY FREETYPE HARFBUZZ
ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS PDUMPER

Important settings:
  value of $LANG: C.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Fundamental

Minor modes in effect:
  allout-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs text-property-search time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils allout-widgets
pcase wid-edit allout ffap thingatpt url-parse auth-source cl-seq eieio
eieio-core cl-macs eieio-loaddefs password-cache json subr-x map seq
byte-opt gv bytecomp byte-compile cconv url-vars vc-git diff-mode
easymenu easy-mmode cl-loaddefs cl-lib bug-reference tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win
x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads dbusbind inotify dynamic-setting system-font-setting
font-render-setting cairo move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 16 74548 7639)
 (symbols 48 8879 1)
 (strings 32 23418 2064)
 (string-bytes 1 775322)
 (vectors 16 13294)
 (vector-slots 8 168908 9258)
 (floats 8 33 150)
 (intervals 56 369 0)
 (buffers 992 15))

[-- Attachment #1.2: Type: text/html, Size: 4643 bytes --]

[-- Attachment #2: 0001-Rectify-allout-widgets-region-undecoration-so-item-a.patch --]
[-- Type: application/octet-stream, Size: 2308 bytes --]

From 186125cd62f88a817aa0872c1be72d4d104270b9 Mon Sep 17 00:00:00 2001
From: Ken Manheimer <ken.manheimer@gmail.com>
Date: Sun, 5 Jul 2020 16:38:13 -0400
Subject: [PATCH] Rectify allout-widgets region undecoration so item at start
 is not missed.

* lisp/allout-widgets.el (allout-widgets-undecorate-region):
Reorganize the loop so an item at the start is not skipped.
---
 lisp/allout-widgets.el | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/lisp/allout-widgets.el b/lisp/allout-widgets.el
index fbdddca7d7..bdfcaa2e55 100644
--- a/lisp/allout-widgets.el
+++ b/lisp/allout-widgets.el
@@ -2050,19 +2050,22 @@ allout-item-actual-position
 ;;;_   > allout-widgets-undecorate-region (start end)
 (defun allout-widgets-undecorate-region (start end)
   "Eliminate widgets and decorations for all items in region from START to END."
-  (let ((next start)
-        widget)
+  (let (done next widget
+        (end (or end (point-max))))
     (save-excursion
       (goto-char start)
-      (while (<  (setq next (next-single-char-property-change next
-                                                              'display
-                                                              (current-buffer)
-                                                              end))
-                 end)
-        (goto-char next)
-        (when (setq widget (allout-get-item-widget))
-          ;; if the next-property/overly progression got us to a widget:
-          (allout-widgets-undecorate-item widget t))))))
+      (while (not done)
+        (when (and (allout-on-current-heading-p)
+                   (setq widget (allout-get-item-widget)))
+            (if widget
+                (allout-widgets-undecorate-item widget t)))
+        (goto-char (setq next
+                         (next-single-char-property-change (point)
+                                                           'display
+                                                           (current-buffer)
+                                                           end)))
+        (if (>= next end)
+            (setq done t))))))
 ;;;_   > allout-widgets-undecorate-text (text)
 (defun allout-widgets-undecorate-text (text)
   "Eliminate widgets and decorations for all items in TEXT."
-- 
2.17.1

bug#42218: 28.0.50; Fix for missed item undecoration on allout-widgets-mode deactivation

[Eli Zaretskii]

Ken, are you waiting for someone or something to install these and
other changes in allout-widgets-mode?  If not, you can go ahead and
push them, you have write access to the Emacs Git repository.  Just
remember to respond to your original bug report, and close the bug
after installing the change.

Thanks.

bug#42218: 28.0.50; Fix for missed item undecoration on allout-widgets-mode deactivation

[Ken Manheimer]

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

On Sat, Jul 18, 2020 at 3:43 AM Eli Zaretskii <eliz@gnu.org> wrote:

> Ken, are you waiting for someone or something to install these and
> other changes in allout-widgets-mode?  If not, you can go ahead and
>

I was not sure what to expect.


> push them, you have write access to the Emacs Git repository.  Just
> remember to respond to your original bug report, and close the bug
> after installing the change.
>

As ever I appreciate the guidance. I'll be happy to merge and push the
patches.

To make sure I'm doing the right thing, I plan to merge the local branches
where I have my various fixes (after rebasing them on current master) to my
local clone of master, and then push it all back to Savannah. I will also
merge to the emacs-27 branch and push that, and close the related bug
reports. Is this correct?

One other question. I have a few more allout-widgets.el patches that are
development-related, and as such don't solve any bugs. I would like to push
them out, as well. Is it preferred that I associate them with a bug report,
or is it sufficient for me to just be clear in the commit messages, without
any bug report?

Thanks again,
Ken


Thanks.
>

[-- Attachment #2: Type: text/html, Size: 1895 bytes --]

bug#42218: 28.0.50; Fix for missed item undecoration on allout-widgets-mode deactivation

[Eli Zaretskii]

> From: Ken Manheimer <ken.manheimer@gmail.com>
> Date: Mon, 20 Jul 2020 13:56:27 -0400
> Cc: 42218@debbugs.gnu.org
> 
> To make sure I'm doing the right thing, I plan to merge the local branches where I have my various fixes
> (after rebasing them on current master) to my local clone of master, and then push it all back to Savannah. I
> will also merge to the emacs-27 branch and push that, and close the related bug reports. Is this correct?

The emacs-27 branch should only get documentation fixes and very safe
bugfixes for serious bugs.  Other than that, please don't push
anything there; when in doubt, please ask.

> One other question. I have a few more allout-widgets.el patches that are development-related, and as such
> don't solve any bugs. I would like to push them out, as well. Is it preferred that I associate them with a bug
> report, or is it sufficient for me to just be clear in the commit messages, without any bug report?

A bug report is always preferable (and then please mention its number
in the commit log), as it leaves an easily identifiable trace.

Thanks.

bug#42218: 28.0.50; Fix for missed item undecoration on allout-widgets-mode deactivation

[Ken Manheimer]

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

On Mon, Jul 20, 2020 at 2:38 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Ken Manheimer <ken.manheimer@gmail.com>
> > Date: Mon, 20 Jul 2020 13:56:27 -0400
> > Cc: 42218@debbugs.gnu.org
> >
> > To make sure I'm doing the right thing, I plan to merge the local
> branches where I have my various fixes
> > (after rebasing them on current master) to my local clone of master, and
> then push it all back to Savannah. I
> > will also merge to the emacs-27 branch and push that, and close the
> related bug reports. Is this correct?
>
> The emacs-27 branch should only get documentation fixes and very safe
> bugfixes for serious bugs.  Other than that, please don't push
> anything there; when in doubt, please ask.
>

I'm sorry I didn't wait for your reply, I already pushed to the emacs-27
branch (as well as master). I will avoid committing to a release branch in
the future.

It would be useful to add something like what you said in the "Backporting
to the release branch" section of admin/notes/git-workflow.


> > One other question. I have a few more allout-widgets.el patches that are
> development-related, and as such
> > don't solve any bugs. I would like to push them out, as well. Is it
> preferred that I associate them with a bug
> > report, or is it sufficient for me to just be clear in the commit
> messages, without any bug report?
>
> A bug report is always preferable (and then please mention its number
> in the commit log), as it leaves an easily identifiable trace.
>

Ok, will do.

Thanks!
Ken

Thanks.
>

[-- Attachment #2: Type: text/html, Size: 2440 bytes --]

bug#42218: Acknowledgement (28.0.50; Fix for missed item undecoration on allout-widgets-mode deactivation)

[Ken Manheimer]

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

Fix committed in 33d85cb768b40794bffcd9ab22fbdec1211a74e5 and merged and
pushed to main repository on master and (mistakenly) emacs-27 branches.

On Sun, Jul 5, 2020 at 5:06 PM GNU bug Tracking System <help-debbugs@gnu.org>
wrote:

> Thank you for filing a new bug report with debbugs.gnu.org.
>
> This is an automatically generated reply to let you know your message
> has been received.
>
> Your message is being forwarded to the package maintainers and other
> interested parties for their attention; they will reply in due course.
>
> Your message has been sent to the package maintainer(s):
>  bug-gnu-emacs@gnu.org
>
> If you wish to submit further information on this problem, please
> send it to 42218@debbugs.gnu.org.
>
> Please do not send mail to help-debbugs@gnu.org unless you wish
> to report a problem with the Bug-tracking system.
>
> --
> 42218: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=42218
> GNU Bug Tracking System
> Contact help-debbugs@gnu.org with problems
>

[-- Attachment #2: Type: text/html, Size: 1738 bytes --]

bug#42218: 28.0.50; Fix for missed item undecoration on allout-widgets-mode deactivation

[Eli Zaretskii]

> From: Ken Manheimer <ken.manheimer@gmail.com>
> Date: Mon, 20 Jul 2020 14:58:01 -0400
> Cc: 42218@debbugs.gnu.org
> 
>  The emacs-27 branch should only get documentation fixes and very safe
>  bugfixes for serious bugs.  Other than that, please don't push
>  anything there; when in doubt, please ask.
> 
> I'm sorry I didn't wait for your reply, I already pushed to the emacs-27 branch (as well as master). I will avoid
> committing to a release branch in the future.

Please revert that commit you made, I don't think it is really needed
on emacs-27.

> It would be useful to add something like what you said in the "Backporting to the release branch" section of
> admin/notes/git-workflow. 

This is described in CONTRIBUTE; git-workflow is not the right
document for these issues.

Thanks.

bug#42218: 28.0.50; Fix for missed item undecoration on allout-widgets-mode deactivation

[Eli Zaretskii]

> Date: Mon, 20 Jul 2020 22:24:49 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 42218@debbugs.gnu.org
> 
> > I'm sorry I didn't wait for your reply, I already pushed to the emacs-27 branch (as well as master). I will avoid
> > committing to a release branch in the future.
> 
> Please revert that commit you made

To clarify: I meant to revert it on emacs-27 only; it should stay on
master.

bug#42218: 28.0.50; Fix for missed item undecoration on allout-widgets-mode deactivation

[Ken Manheimer]

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

On Mon, Jul 20, 2020 at 3:28 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Mon, 20 Jul 2020 22:24:49 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: 42218@debbugs.gnu.org
> >
> > > I'm sorry I didn't wait for your reply, I already pushed to the
> emacs-27 branch (as well as master). I will avoid
> > > committing to a release branch in the future.
> >
> > Please revert that commit you made
>
> To clarify: I meant to revert it on emacs-27 only; it should stay on
> master.
>

Understood.

In my local checkout of the emacs-27 branch I have reversal of the five
commits ready to push upstream. I'm sorry to bother you further with this,
but rather than risk compounding the problem I want to double-check that
reversing the commits (rather than, eg, reversing the changes in new
commits) is the right way to go, before I push upstream. I know that this
is a different situation than to problem-prone one of doing a reset and
pushing that, but haven't pushed reversed commits and want to be sure.
(This time I will wait for your reply rather than just going ahead.)

Thanks!
Ken

[-- Attachment #2: Type: text/html, Size: 1652 bytes --]

bug#42218: 28.0.50; Fix for missed item undecoration on allout-widgets-mode deactivation

[Eli Zaretskii]

> From: Ken Manheimer <ken.manheimer@gmail.com>
> Date: Mon, 20 Jul 2020 16:09:51 -0400
> Cc: 42218@debbugs.gnu.org
> 
> In my local checkout of the emacs-27 branch I have reversal of the five commits ready to push upstream. I'm
> sorry to bother you further with this, but rather than risk compounding the problem I want to double-check
> that reversing the commits (rather than, eg, reversing the changes in new commits) is the right way to go,
> before I push upstream. I know that this is a different situation than to problem-prone one of doing a reset
> and pushing that, but haven't pushed reversed commits and want to be sure. (This time I will wait for your
> reply rather than just going ahead.)

The right way is to say "git revert SHA1" for each of the commits you
want to revert.  This creates a commit with reverse diffs.  Then "git
push" the result.

Thanks.

bug#42218: 28.0.50; Fix for missed item undecoration on allout-widgets-mode deactivation

[Ken Manheimer]

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

Ok, reverts of the emacs-27 backports are pushed to origin.

On Mon, Jul 20, 2020 at 10:24 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Ken Manheimer <ken.manheimer@gmail.com>
> > Date: Mon, 20 Jul 2020 16:09:51 -0400
> > Cc: 42218@debbugs.gnu.org
> >
> > In my local checkout of the emacs-27 branch I have reversal of the five
> commits ready to push upstream. I'm
> > sorry to bother you further with this, but rather than risk compounding
> the problem I want to double-check
> > that reversing the commits (rather than, eg, reversing the changes in
> new commits) is the right way to go,
> > before I push upstream. I know that this is a different situation than
> to problem-prone one of doing a reset
> > and pushing that, but haven't pushed reversed commits and want to be
> sure. (This time I will wait for your
> > reply rather than just going ahead.)
>
> The right way is to say "git revert SHA1" for each of the commits you
> want to revert.  This creates a commit with reverse diffs.  Then "git
> push" the result.
>
> Thanks.
>

[-- Attachment #2: Type: text/html, Size: 1543 bytes --]

bug#42218: 28.0.50; Fix for missed item undecoration on allout-widgets-mode deactivation

[Eli Zaretskii]

> From: Ken Manheimer <ken.manheimer@gmail.com>
> Date: Tue, 21 Jul 2020 11:17:36 -0400
> Cc: 42218@debbugs.gnu.org
> 
> Ok, reverts of the emacs-27 backports are pushed to origin.

Thanks you, and sorry I didn't mention this from the get-go.