GNU bug report logs - #19892
25.0.50; hideshow: hs-hide-all-non-comment-function example infloop

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 19892 in the body.
You can then email your comments to 19892 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: bug-gnu-emacs <at> gnu.org
Cc: ttn <at> gnu.org, dann <at> ics.uci.edu
Subject: 25.0.50; hideshow: hs-hide-all-non-comment-function example infloop
Date: Tue, 17 Feb 2015 23:45:15 +0100

Hello,

  (CCing the authors of hideshow specified in the file header)

in the header of hideshow.el, we have the following paragraph:

--8<---------------cut here---------------start------------->8---
;; Some languages (e.g., Java) are deeply nested, so the normal behavior
;; of `hs-hide-all' (hiding all but top-level blocks) results in very
;; little information shown, which is not very useful.  You can use the
;; variable `hs-hide-all-non-comment-function' to implement your idea of
;; what is more useful.  For example, the following code shows the next
;; nested level in addition to the top-level:
;;
;;   (defun ttn-hs-hide-level-1 ()
;;     (hs-hide-level 1)
;;     (forward-sexp 1))
;;   (setq hs-hide-all-non-comment-function 'ttn-hs-hide-level-1)
--8<---------------cut here---------------end--------------->8---

But this doesn't always work.  For example, eval the above example in
emacs -Q, open "files.el", M-x hs-minor-mode, M-x hs-hide-all.  You get
an infloop.

AFAICT the definition should be (or at least it works with that)

(defun ttn-hs-hide-level-1 ()
  (when (hs-looking-at-block-start-p)
    (hs-hide-level 1))
  (forward-sexp 1))


Secondly, there is this comment in `hs-hide-all' which confuses me a bit:

;; Go to end of matched data to prevent from getting stuck
;; with an endless loop.

Which match data is meant there?  It is either match data from before
hiding the block - then it should be documented that
`hs-hide-all-non-comment-function' must not change match data, I guess,
or the call should be wrapped into `save-match-data'.  Or it is even the
case that `hs-hide-all-non-comment-function' must set the match data
(how?), which then should probably be documented.


Thanks,

Michael.






In GNU Emacs 25.0.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.14.5)
 of 2015-02-15 on drachen
Repository revision: ba5bc0ee7c81f2122072bee162fcf1dbd8b2a8f2
Windowing system distributor `The X.Org Foundation', version 11.0.11602901
System Description:	Debian GNU/Linux 8.0 (jessie)

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND DBUS GSETTINGS NOTIFY
LIBXML2 FREETYPE XFT ZLIB

Important settings:
  value of $LC_ALL: de_DE.utf8
  value of $LC_COLLATE: C
  value of $LC_TIME: C
  value of $LANG: de_DE.utf8
  locale-coding-system: utf-8-unix

Major mode: Emacs-Lisp

Minor modes in effect:
  magit-auto-revert-mode: t
  rainbow-delimiters-mode: t
  paredit-mode: t
  on-screen-global-mode: t
  ml-scale-mode: t
  interaction-log-mode: t
  highlight-defined-mode: t
  helm-descbinds-mode: t
  helm-mode: t
  shell-dirtrack-mode: t
  helm-occur-match-plugin-mode: t
  helm-match-plugin-mode: t
  helm-autoresize-mode: t
  global-diff-hl-mode: t
  diff-hl-mode: t
  diff-auto-refine-mode: t
  recentf-mode: t
  which-function-mode: t
  winner-mode: t
  show-paren-mode: t
  auto-image-file-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  mouse-wheel-mode: t
  prettify-symbols-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
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  hs-minor-mode: t

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: ttn <at> gnu.org, dann <at> ics.uci.edu, 19892 <at> debbugs.gnu.org
Subject: Re: bug#19892: 25.0.50; hideshow: hs-hide-all-non-comment-function
 example infloop
Date: Fri, 02 Aug 2019 21:08:54 +0200

Michael Heerdegen <michael_heerdegen <at> web.de> writes:

> in the header of hideshow.el, we have the following paragraph:

[...]

> ;;   (defun ttn-hs-hide-level-1 ()
> ;;     (hs-hide-level 1)
> ;;     (forward-sexp 1))
> ;;   (setq hs-hide-all-non-comment-function 'ttn-hs-hide-level-1)
>
> But this doesn't always work.  For example, eval the above example in
> emacs -Q, open "files.el", M-x hs-minor-mode, M-x hs-hide-all.  You get
> an infloop.
>
> AFAICT the definition should be (or at least it works with that)
>
> (defun ttn-hs-hide-level-1 ()
>   (when (hs-looking-at-block-start-p)
>     (hs-hide-level 1))
>   (forward-sexp 1))

I've confirmed that the example still infloops in Emacs 27, and that
your example doesn't, so I've committed it to the Emacs trunk.

> Secondly, there is this comment in `hs-hide-all' which confuses me a bit:
>
> ;; Go to end of matched data to prevent from getting stuck
> ;; with an endless loop.
>
> Which match data is meant there?  It is either match data from before
> hiding the block - then it should be documented that
> `hs-hide-all-non-comment-function' must not change match data, I guess,
> or the call should be wrapped into `save-match-data'.  Or it is even the
> case that `hs-hide-all-non-comment-function' must set the match data
> (how?), which then should probably be documented.

Here's the code:

             (progn
               (goto-char (match-beginning 1))
	       (unless (if hs-hide-all-non-comment-function
			   (funcall hs-hide-all-non-comment-function)
			 (hs-hide-block-at-point t))
		 ;; Go to end of matched data to prevent from getting stuck
		 ;; with an endless loop.
		 (goto-char (match-end 0))))

`hs-hide-block-at-point' calls a lot of functions that change match
data, so presumably `hs-hide-all-non-comment-function' is allowed, too.
But this all seems nonsensical -- what `match-end' points to here seems
pretty random.  So it looks like a bug to me.

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

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: ttn <at> gnu.org, dann <at> ics.uci.edu, 19892 <at> debbugs.gnu.org
Subject: Re: bug#19892: 25.0.50; hideshow: hs-hide-all-non-comment-function
 example infloop
Date: Fri, 02 Aug 2019 21:17:00 +0200

This was introduced (by me applying the following patch) seven years
ago.

Hm...  Oh, I see what the brittle logic here really is.  The code (by
default) calls this function:

(defun hs-hide-block-at-point (&optional end comment-reg)
[...]
    (when (hs-looking-at-block-start-p)

And:

(defun hs-looking-at-block-start-p ()
  "Return non-nil if the point is at the block start."
  (and (looking-at hs-block-start-regexp)
       (save-match-data (not (nth 8 (syntax-ppss))))))

So if the match is successful, but whatever (nth 8 (syntax-ppss)) means
doesn't, then we go to the end of that match data.

So it kinda works...  But not if `hs-hide-all-non-comment-function' is
set.  I'll try to rework this in a more logical fashion.

commit 43956923c00921499e0582f5da4cd739bc005240
Author: Sébastien Gross <seb <at> chezwam.org>
Date:   Wed Apr 11 01:34:25 2012 +0200

    (hs-hide-all): Don't infloop on comments that start in the middle of the line.
    
    Fixes: debbugs:10496

diff --git a/lisp/progmodes/hideshow.el b/lisp/progmodes/hideshow.el
--- a/lisp/progmodes/hideshow.el
+++ b/lisp/progmodes/hideshow.el
@@ -804,7 +804,10 @@
          (if (match-beginning 1)
-             ;; we have found a block beginning
+             ;; We have found a block beginning.
              (progn
                (goto-char (match-beginning 1))
-               (if hs-hide-all-non-comment-function
-                   (funcall hs-hide-all-non-comment-function)
-                 (hs-hide-block-at-point t)))
+	       (unless (if hs-hide-all-non-comment-function
+			   (funcall hs-hide-all-non-comment-function)
+			 (hs-hide-block-at-point t))
+		 ;; Go to end of matched data to prevent from getting stuck
+		 ;; with an endless loop.
+		 (goto-char (match-end 0))))



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

Previous Next

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