bug#46878: 27.1; lisp-outline-level returns imprecise level number

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Howard Melman]

lisp-outline-level returns a functional but incorrect level
number.  Comments beginning with ";;; " should be at level 1
but it returns 5 because they match ";;; [^ \t\n]" and the
current code just returns that length.

The following version returns the right level following the
convention that ;;; are top level comments and each lower
level adds a ;.  As in the existing code, lines that match
an autoload tag or begin with ( return level 1000.

This fix would be helpful for code that wants to deal with
headings by level number generically (e.g., display the top
3 heading levels).  Other modes like outline, org, texinfo,
and html-mode return a level rooted at 1.  Programming modes
are more mixed, but this seems like an easy improvement for
lisp modes.

    (defun lisp-outline-level ()
      "Lisp mode `outline-level' function."
      ;; expects outline-regexp is ";;;\\(;* [^ \t\n]\\|###autoload\\)\\|("
      ;; and point is at the beginning of a matching line
      (let ((len (- (match-end 0) (match-beginning 0))))
        (cond ((looking-at "(\\|;;;###autoload")
               1000)
              ((looking-at ";;\\(;+\\) ")
               (- (match-end 1) (match-beginning 1)))
              ;; above should match everything but just in case
              (t
               len))))

In GNU Emacs 27.1 (build 1, x86_64-apple-darwin18.7.0, NS appkit-1671.60 Version 10.14.6 (Build 18G95))
of 2020-08-12 built on builder10-14.porkrind.org
Windowing system distributor 'Apple', version 10.3.1894
System Description:  Mac OS X 10.15.7

Configured using:
'configure --with-ns '--enable-locallisppath=/Library/Application
Support/Emacs/${version}/site-lisp:/Library/Application
Support/Emacs/site-lisp' --with-modules'

-- 

Howard

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Eli Zaretskii]

> From: Howard Melman <hmelman@gmail.com>
> Date: Tue, 2 Mar 2021 11:35:01 -0500
> 
> lisp-outline-level returns a functional but incorrect level
> number.  Comments beginning with ";;; " should be at level 1
> but it returns 5 because they match ";;; [^ \t\n]" and the
> current code just returns that length.

I believe these problems are fixed on the master branch (or at least
the behavior is very different), please try that.

Thanks.

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Howard Melman]

On Mar 2, 2021, at 12:05 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Howard Melman <hmelman@gmail.com>
>> Date: Tue, 2 Mar 2021 11:35:01 -0500
>> 
>> lisp-outline-level returns a functional but incorrect level
>> number.  Comments beginning with ";;; " should be at level 1
>> but it returns 5 because they match ";;; [^ \t\n]" and the
>> current code just returns that length.
> 
> I believe these problems are fixed on the master branch (or at least
> the behavior is very different), please try that.

I don't build my own emacs and can't easily test on master.  

I had previously confirmed that lisp-outline-level in 27.1 
is the same as in master as shown here:
http://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/emacs-lisp/lisp-mode.el

You can quickly test the behavior by putting point at the beginning
of a "heading" line in an emacs-lisp-mode file and running:
(progn (looking-at outline-regexp) (funcall outline-level))

Howard

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Juri Linkov]

> The following version returns the right level following the
> convention that ;;; are top level comments and each lower
> level adds a ;.

I use outline-minor-mode extensively in lisp-mode,
so after trying your version I'd say that while
the change of the level definition can be surprising
to someone, but to me your version makes more sense than
the current level definition.

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Howard Melman]

> On Mar 2, 2021, at 2:36 PM, Juri Linkov <juri@linkov.net> wrote:
> 
> I use outline-minor-mode extensively in lisp-mode,
> so after trying your version I'd say that while
> the change of the level definition can be surprising
> to someone, but to me your version makes more sense than
> the current level definition.

Thanks.  I use outline-minor-mode a bit, mostly via outline-cycle.  When you say the change can be surprising, what do you mean? Does something show the level number? AFAIK levels are just treated in relation to each other and the number isn't shown to the user.  My change I think just means levels 1-4 were previously unused and are now used.

Howard

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Juri Linkov]

>> I use outline-minor-mode extensively in lisp-mode,
>> so after trying your version I'd say that while
>> the change of the level definition can be surprising
>> to someone, but to me your version makes more sense than
>> the current level definition.
>
> Thanks.  I use outline-minor-mode a bit, mostly via outline-cycle.

bug#45147 implemented new modes for Org-like TAB cycling in outline-minor-mode.

> When you say the change can be surprising, what do you mean? Does something
> show the level number? AFAIK levels are just treated in relation to each
> other and the number isn't shown to the user.  My change I think just means
> levels 1-4 were previously unused and are now used.

Some packages like https://jurta.org/en/emacs/ee/outline
showed all 4 useless additional levels.

Now thanks to your new definition, all such visual noise is removed:

- (no-heading)
 - (no-heading)
  - (no-heading)
   - (no-heading)

and only one top heading is displayed:

- (no-heading)
   ;;; ee-outline.el --- manipulate outlines collected from outline-mode
   ;;; Commentary:
 - ;;; Code:

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Howard Melman]

> On Mar 3, 2021, at 2:04 PM, Juri Linkov <juri@linkov.net> wrote:
> 
> Some packages like https://jurta.org/en/emacs/ee/outline
> showed all 4 useless additional levels.
> 
> Now thanks to your new definition, all such visual noise is removed:
> 
> - (no-heading)
> - (no-heading)
>  - (no-heading)
>   - (no-heading)
> 
> and only one top heading is displayed:
> 
> - (no-heading)
>   ;;; ee-outline.el --- manipulate outlines collected from outline-mode
>   ;;; Commentary:
> - ;;; Code:

Great.  I came about it because I hope consult-outline adds an argument 
to let the user choose headings from the top N levels, so that's why the 
numbering became important for me.

Hopefully no reports a downside which is why I asked about any visible
changes in what is shipped with emacs.

Howard

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Juri Linkov]

> Hopefully no reports a downside which is why I asked about any visible
> changes in what is shipped with emacs.

I tried to use your new definition, but noticed that
it adds a new cycling state: while using outline-cycle-buffer
previously it used to cycle between only two states
(thus it was convenient to collapse/expand all headings
with only one keypress S-TAB).

But with your change, it requires typing S-TAB twice
to get the state where all headings are shown and
their bodies are collapsed, because it cycles
between three states.

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Howard Melman]

On Mar 7, 2021, at 1:45 PM, Juri Linkov <juri@linkov.net> wrote:

> I tried to use your new definition, but noticed that
> it adds a new cycling state: while using outline-cycle-buffer
> previously it used to cycle between only two states
> (thus it was convenient to collapse/expand all headings
> with only one keypress S-TAB).
> 
> But with your change, it requires typing S-TAB twice
> to get the state where all headings are shown and
> their bodies are collapsed, because it cycles
> between three states.

Hmmm. Yes I have that, but I always had that in emacs-lisp-mode
even before this change.  The cycle commands cycle between
three states (top level, all headings and everything) and as I 
understand it, it's just a function of how many levels are defined, 
not what numbers they are called.

Howard

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Juri Linkov]

>> I tried to use your new definition, but noticed that
>> it adds a new cycling state: while using outline-cycle-buffer
>> previously it used to cycle between only two states
>> (thus it was convenient to collapse/expand all headings
>> with only one keypress S-TAB).
>>
>> But with your change, it requires typing S-TAB twice
>> to get the state where all headings are shown and
>> their bodies are collapsed, because it cycles
>> between three states.
>
> Hmmm. Yes I have that, but I always had that in emacs-lisp-mode
> even before this change.  The cycle commands cycle between
> three states (top level, all headings and everything) and as I
> understand it, it's just a function of how many levels are defined,
> not what numbers they are called.

This is strange.  When trying this in emacs -Q and visiting
a Emacs-Lisp file from the emacs repository and typing:

  M-x outline-minor-mode RET

Then using repeatedly:

  M-x outline-cycle-buffer RET

It consistently alternates between only two states:

  Show all
  All headings

But after evaluating your version of lisp-outline-level,
it alternates between three states:

  Show all
  Top level headings
  All headings

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Howard Melman]

On Mar 8, 2021, at 12:23 PM, Juri Linkov <juri@linkov.net> wrote:

> This is strange.  When trying this in emacs -Q and visiting
> a Emacs-Lisp file from the emacs repository and typing:
> 
> M-x outline-minor-mode RET
> 
> Then using repeatedly:
> 
> M-x outline-cycle-buffer RET
> 
> It consistently alternates between only two states:
> 
> Show all
> All headings
> 
> But after evaluating your version of lisp-outline-level,
> it alternates between three states:
> 
> Show all
> Top level headings
> All headings

That is strange.  Is outline-regexp changing?  (I'm not on Emacs 28 so I can't check.)

Howard

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Juri Linkov]

>> M-x outline-cycle-buffer RET
>>
>> It consistently alternates between only two states:
>>
>> Show all
>> All headings
>>
>> But after evaluating your version of lisp-outline-level,
>> it alternates between three states:
>>
>> Show all
>> Top level headings
>> All headings
>
> That is strange.  Is outline-regexp changing?  (I'm not on Emacs 28 so I can't check.)

I don't think that outline-regexp is changing.  But I can't check
in Emacs 27 because outline-cycle-buffer is only in Emacs 28.

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Gabriel]

Juri Linkov <juri@linkov.net> writes:

>>> M-x outline-cycle-buffer RET
>>>
>>> It consistently alternates between only two states:
>>>
>>> Show all
>>> All headings
>>>
>>> But after evaluating your version of lisp-outline-level,
>>> it alternates between three states:
>>>
>>> Show all
>>> Top level headings
>>> All headings
>>
>> That is strange.  Is outline-regexp changing?  (I'm not on Emacs 28 so I can't check.)
>
> I don't think that outline-regexp is changing.  But I can't check
> in Emacs 27 because outline-cycle-buffer is only in Emacs 28.

So I checked this "apparent strange behavior" on master branch and it
seems to be working as expected. The commit
5a823a2a0c3eca60dd3939fba67df1bbe5a1b715 (Lars on 24 Nov 2020) added a
new condition ('has-top-level') to 'outline-cycle-buffer' to check,
well, if there is a top level heading, which is used to decide to hide
or not sublevels, in case they exist. For emacs-lisp-mode, it fails to
identify a top-level because 'outline-level' for a line starting with
";;; " returns a number greater than 1, which the code thinks its a
child level:

(when (= (funcall outline-level) 1)
  (setq has-top-level t))

Even though the outline level is used to compare relatively the headings
to decide which ones are parents and which ones are children (level 5 is
parent of level 6 and subsequent levels), there are cases (like the one
above) where it's expected to have an ordered levelling starting from
1. It's more intuitive, and otherwise it's impossible to know what is a
top-level or to set faces according to is level (level 1 with face
outline-1 and so on).

A simpler approach to avoid rewriting the existing functions and regexps
would be to create a new variable called 'outline-level-offset', which
could be used inside 'outline-level' to subtract from the original
value. For example, if today the outline level for ";;; " in emacs-lisp
returns 3, the elisp-mode.el (or lisp.mode.el) could set
'outline-level-offset' to 2, so 3 - 2 = 1, the expected value.

Regards,
Gabriel

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Juri Linkov]

> So I checked this "apparent strange behavior" on master branch and it
> seems to be working as expected. The commit
> 5a823a2a0c3eca60dd3939fba67df1bbe5a1b715 (Lars on 24 Nov 2020) added a
> new condition ('has-top-level') to 'outline-cycle-buffer' to check,
> well, if there is a top level heading, which is used to decide to hide
> or not sublevels, in case they exist. For emacs-lisp-mode, it fails to
> identify a top-level because 'outline-level' for a line starting with
> ";;; " returns a number greater than 1, which the code thinks its a
> child level:
>
> (when (= (funcall outline-level) 1)
>   (setq has-top-level t))
>
> Even though the outline level is used to compare relatively the headings
> to decide which ones are parents and which ones are children (level 5 is
> parent of level 6 and subsequent levels), there are cases (like the one
> above) where it's expected to have an ordered levelling starting from
> 1. It's more intuitive, and otherwise it's impossible to know what is a
> top-level or to set faces according to is level (level 1 with face
> outline-1 and so on).

Thanks for the analysis.  After replacing the hard-coded 1 with 5
it works as expected in emacs-lisp-mode.

> A simpler approach to avoid rewriting the existing functions and regexps
> would be to create a new variable called 'outline-level-offset', which
> could be used inside 'outline-level' to subtract from the original
> value. For example, if today the outline level for ";;; " in emacs-lisp
> returns 3, the elisp-mode.el (or lisp.mode.el) could set
> 'outline-level-offset' to 2, so 3 - 2 = 1, the expected value.

Maybe a better name for a new buffer-local variable would be 'outline-top-level',
to make it similar to the internal variable 'has-top-level'.

Or maybe better just to apply the patch that Howard sent in the top message
of this bug report to change 'outline-level' in emacs-lisp-mode to count
from 1.

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Howard Melman]

Juri Linkov <juri@linkov.net> writes:

> Or maybe better just to apply the patch that Howard sent in the top message
> of this bug report to change 'outline-level' in emacs-lisp-mode to count
> from 1.

FWIW, this seems cleaner to me :) 

-- 

Howard

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Howard Melman]

Howard Melman <hmelman@gmail.com> writes:

> Juri Linkov <juri@linkov.net> writes:
>
>> Or maybe better just to apply the patch that Howard sent in the top message
>> of this bug report to change 'outline-level' in emacs-lisp-mode to count
>> from 1.
>
> FWIW, this seems cleaner to me :) 

So two months later, will this fix get applied?

-- 

Howard

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Juri Linkov]

tags 46878 fixed
close 46878 28.0.50
thanks

>>> Or maybe better just to apply the patch that Howard sent in the top message
>>> of this bug report to change 'outline-level' in emacs-lisp-mode to count
>>> from 1.
>>
>> FWIW, this seems cleaner to me :)
>
> So two months later, will this fix get applied?

Sorry for the delay, I waited for more possible objections.
Now pushed to master in the commit 502e3ce614.

bug#46878: 27.1; lisp-outline-level returns imprecise level number

[Howard Melman]

Juri Linkov <juri@linkov.net> writes:

>> So two months later, will this fix get applied?
>
> Sorry for the delay, I waited for more possible objections.
> Now pushed to master in the commit 502e3ce614.

Thanks very much.

-- 

Howard