Re: master 262a7c9ea0: New minor mode elide-head-mode

Re: master 262a7c9ea0: New minor mode elide-head-mode

[Po Lu]

Stefan Kangas <stefankangas@gmail.com> writes:

>  (defun elide-head (&optional arg)
>    "Hide header material in buffer according to `elide-head-headers-to-hide'.
> @@ -88,43 +151,17 @@ The header is made invisible with an overlay.  With a prefix arg, show
>  an elided material again.
>  
>  This is suitable as an entry on `find-file-hook' or appropriate mode hooks."
> +  (declare (obsolete elide-head-mode "29.1"))
>    (interactive "P")
>    (if arg
> -      (elide-head-show)
> -    (save-excursion
> -      (save-restriction
> -	(let ((rest elide-head-headers-to-hide)
> -	      beg end)
> -	  (widen)
> -	  (goto-char (point-min))
> -	  (while rest
> -	    (save-excursion
> -	      (when (re-search-forward (caar rest) nil t)
> -		(setq beg (point))
> -		(when (re-search-forward (cdar rest) nil t)
> -		  (setq end (point-marker)
> -			rest nil))))
> -	    (if rest (setq rest (cdr rest))))
> -	  (if (not (and beg end))
> -	      (if (called-interactively-p 'interactive)
> -		  (message "No header found"))
> -	    (goto-char beg)
> -	    (end-of-line)
> -	    (if (overlayp elide-head-overlay)
> -		(move-overlay elide-head-overlay (point-marker) end)
> -	      (setq elide-head-overlay (make-overlay (point-marker) end)))
> -	    (overlay-put elide-head-overlay 'invisible t)
> -	    (overlay-put elide-head-overlay 'evaporate t)
> -	    (overlay-put elide-head-overlay 'after-string "...")))))))
> +      (elide-head--show)
> +    (elide-head--hide)))
>  
>  (defun elide-head-show ()
>    "Show a header in the current buffer elided by \\[elide-head]."
> +  (declare (obsolete elide-head-mode "29.1"))
>    (interactive)
> -  (if (and (overlayp elide-head-overlay)
> -	   (overlay-buffer elide-head-overlay))
> -      (delete-overlay elide-head-overlay)
> -    (if (called-interactively-p 'interactive)
> -	(message "No header hidden"))))
> +  (elide-head--show))

Hmm... couldn't those two obsolete commands be made wrappers around
`elide-head-mode'?

For example:

   (defun elide-head-show ()
     "Show a header in the current buffer elided by \\[elide-head]."
     (interactive)
     (elide-head-mode 0))

And if those commands are made such simple wrappers, they could be
un-obsoleted as well, since it would be useful to explictly disable or
enable elide-head-mode inside `find-file-hook' and other similar places.

Thanks.

Re: master 262a7c9ea0: New minor mode elide-head-mode

[Stefan Kangas]

Po Lu <luangruo@yahoo.com> writes:

> For example:
>
>    (defun elide-head-show ()
>      "Show a header in the current buffer elided by \\[elide-head]."
>      (interactive)
>      (elide-head-mode 0))

I chose not to do that, in order not to change the (admittedly buggy)
behavior of the old commands.  But if people think it's a good idea, I
have no objections to such a cleanup.

> And if those commands are made such simple wrappers, they could be
> un-obsoleted as well, since it would be useful to explictly disable or
> enable elide-head-mode inside `find-file-hook' and other similar places.

We don't usually provide such commands for modes, I think?

Re: master 262a7c9ea0: New minor mode elide-head-mode

[Po Lu]

Stefan Kangas <stefan@marxist.se> writes:

> I chose not to do that, in order not to change the (admittedly buggy)
> behavior of the old commands.  But if people think it's a good idea, I
> have no objections to such a cleanup.

Hmm, let's see if anyone else chimes in.

>> And if those commands are made such simple wrappers, they could be
>> un-obsoleted as well, since it would be useful to explictly disable or
>> enable elide-head-mode inside `find-file-hook' and other similar places.

> We don't usually provide such commands for modes, I think?

Yes, but it wouldn't hurt, seeing as we already have the two old
commands.

Thanks.

Re: master 262a7c9ea0: New minor mode elide-head-mode

[Stefan Monnier]

>>> And if those commands are made such simple wrappers, they could be
>>> un-obsoleted as well, since it would be useful to explictly disable or
>>> enable elide-head-mode inside `find-file-hook' and other similar places.
>> We don't usually provide such commands for modes, I think?
> Yes, but it wouldn't hurt, seeing as we already have the two old
> commands.

`elide-head-mode` already works both to enable and to disable the minor
mode interactively.  For use on hooks, `elide-head-mode` already works
to enable the mode.  For the rare case where the users need to disable
the mode from a hook, they should use (lambda () (elide-head-mode -1)).

This should hold for all minor modes.


        Stefan

Re: master 262a7c9ea0: New minor mode elide-head-mode

[Stefan Kangas]

Po Lu <luangruo@yahoo.com> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> I chose not to do that, in order not to change the (admittedly buggy)
>> behavior of the old commands.  But if people think it's a good idea, I
>> have no objections to such a cleanup.
>
> Hmm, let's see if anyone else chimes in.

I noticed a nice side-effect from making them wrappers around the mode:
you can always say `M-x elide-head-mode' to show the header again when
it's hidden and vice versa.

So if users have `elide-head' on their mode hooks, they get this nice
thing for free.  A net win, IMO.  So I've now made the suggested change
on master.  Thanks for proposing it.

Re: master 262a7c9ea0: New minor mode elide-head-mode

[Po Lu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> `elide-head-mode` already works both to enable and to disable the minor
> mode interactively.  For use on hooks, `elide-head-mode` already works
> to enable the mode.  For the rare case where the users need to disable
> the mode from a hook, they should use (lambda () (elide-head-mode -1)).

Adding an anonymous function to a hook hurts, and IME `elide-head' is
(or at least used to be) something commonly placed in site
initialization files.

Re: master 262a7c9ea0: New minor mode elide-head-mode

[Stefan Monnier]

Po Lu [2022-01-10 20:35:28] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> `elide-head-mode` already works both to enable and to disable the minor
>> mode interactively.  For use on hooks, `elide-head-mode` already works
>> to enable the mode.  For the rare case where the users need to disable
>> the mode from a hook, they should use (lambda () (elide-head-mode -1)).
>
> Adding an anonymous function to a hook hurts,

Fully agreed.

> and IME `elide-head' is (or at least used to be) something commonly
> placed in site initialization files.

`elide-head-mode` gives the same result as `elide-head` when placed on
a hook.  The need for an anonymous function is only when you need to
disable `elide-head-mode` from the hook, which is very unusual.


        Stefan

Re: master 262a7c9ea0: New minor mode elide-head-mode

[Po Lu]

Stefan Kangas <stefan@marxist.se> writes:

> So if users have `elide-head' on their mode hooks, they get this nice
> thing for free.  A net win, IMO.  So I've now made the suggested change
> on master.  Thanks for proposing it.

Thanks for making the change as well.