lisp-outline-level.

lisp-outline-level.

[Lute Kamstra]

In (Emacs) Lisp mode, outline-regexp is ";;;;* [^ \t\n]\\|(" and
outline-level is lisp-outline-level:

(defun lisp-outline-level ()
  "Lisp mode `outline-level' function."
  (if (looking-at "(\\|;;;###autoload")
      1000
    (looking-at outline-regexp)
    (- (match-end 0) (match-beginning 0))))

This is a bit strange as outline-regexp doesn't match
";;;###autoload".  Shall I commit the patch below?

Lute.


*** lisp/emacs-lisp/lisp-mode.el	1 Feb 2005 15:48:50 -0000	1.171
--- lisp/emacs-lisp/lisp-mode.el	13 Feb 2005 11:03:02 -0000
***************
*** 212,223 ****
  
  (defun lisp-outline-level ()
    "Lisp mode `outline-level' function."
!   (if (looking-at "(\\|;;;###autoload")
        1000
-     (looking-at outline-regexp)
      (- (match-end 0) (match-beginning 0))))
  
- 
  (defvar lisp-mode-shared-map
    (let ((map (make-sparse-keymap)))
      (define-key map "\t" 'lisp-indent-line)
--- 212,221 ----
  
  (defun lisp-outline-level ()
    "Lisp mode `outline-level' function."
!   (if (eq (following-char) ?\()
        1000
      (- (match-end 0) (match-beginning 0))))
  
  (defvar lisp-mode-shared-map
    (let ((map (make-sparse-keymap)))
      (define-key map "\t" 'lisp-indent-line)

Re: lisp-outline-level.

[David Kastrup]

Lute Kamstra <Lute.Kamstra.lists@xs4all.nl> writes:

> In (Emacs) Lisp mode, outline-regexp is ";;;;* [^ \t\n]\\|(" and
> outline-level is lisp-outline-level:
>
> (defun lisp-outline-level ()
>   "Lisp mode `outline-level' function."
>   (if (looking-at "(\\|;;;###autoload")
>       1000
>     (looking-at outline-regexp)
>     (- (match-end 0) (match-beginning 0))))
>
> This is a bit strange as outline-regexp doesn't match
> ";;;###autoload".

Why is that strange?  outline-regexp is not even consulted when
;###autoload is found, so I don't see how it would come into play
here.

> Shall I commit the patch below?

> *** lisp/emacs-lisp/lisp-mode.el	1 Feb 2005 15:48:50 -0000	1.171
> --- lisp/emacs-lisp/lisp-mode.el	13 Feb 2005 11:03:02 -0000
> ***************
> *** 212,223 ****
>   
>   (defun lisp-outline-level ()
>     "Lisp mode `outline-level' function."
> !   (if (looking-at "(\\|;;;###autoload")
>         1000
> -     (looking-at outline-regexp)
>       (- (match-end 0) (match-beginning 0))))
>   
> - 
> --- 212,221 ----
>   
>   (defun lisp-outline-level ()
>     "Lisp mode `outline-level' function."
> !   (if (eq (following-char) ?\()
>         1000
>       (- (match-end 0) (match-beginning 0))))

The patch is completely nonsensical.  It returns rubbish in almost all
cases since it is the "looking-at" that established match-end and
match-beginning in the first place.

However, the original code also is buggy since it returns nonsense in
case outline-regexp does not match at point.  So one should probably
rather write

        (and (looking-at outline-regexp)
             (- (match-end 0) (match-beginning 0)))

as the last lines.  This will return "nil" instead of a nonsensical
value in case there is no match at point.  I don't know how and where
lisp-outline-level is used, so maybe some other value (1000?) would be
more appropriate than nil.  But returning some random value (which
includes a segmentation violation, by the way) in case that none of
the two looking-at expressions succeeded does not seem like a good
idea in any case.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: lisp-outline-level.

[Lute Kamstra]

David Kastrup <dak@gnu.org> writes:

> Lute Kamstra <Lute.Kamstra.lists@xs4all.nl> writes:
>
>> In (Emacs) Lisp mode, outline-regexp is ";;;;* [^ \t\n]\\|(" and
>> outline-level is lisp-outline-level:
>>
>> (defun lisp-outline-level ()
>>   "Lisp mode `outline-level' function."
>>   (if (looking-at "(\\|;;;###autoload")
>>       1000
>>     (looking-at outline-regexp)
>>     (- (match-end 0) (match-beginning 0))))
>>
>> This is a bit strange as outline-regexp doesn't match
>> ";;;###autoload".
>
> Why is that strange?  outline-regexp is not even consulted when
> ;###autoload is found, so I don't see how it would come into play
> here.
>
>> Shall I commit the patch below?
>
>> *** lisp/emacs-lisp/lisp-mode.el	1 Feb 2005 15:48:50 -0000	1.171
>> --- lisp/emacs-lisp/lisp-mode.el	13 Feb 2005 11:03:02 -0000
>> ***************
>> *** 212,223 ****
>>   
>>   (defun lisp-outline-level ()
>>     "Lisp mode `outline-level' function."
>> !   (if (looking-at "(\\|;;;###autoload")
>>         1000
>> -     (looking-at outline-regexp)
>>       (- (match-end 0) (match-beginning 0))))
>>   
>> - 
>> --- 212,221 ----
>>   
>>   (defun lisp-outline-level ()
>>     "Lisp mode `outline-level' function."
>> !   (if (eq (following-char) ?\()
>>         1000
>>       (- (match-end 0) (match-beginning 0))))
>
> The patch is completely nonsensical.  It returns rubbish in almost all
> cases since it is the "looking-at" that established match-end and
> match-beginning in the first place.

The variables outline-regexp and outline-level are used by Outline
minor mode.  Did you take a look at their docstrings?  Outline minor
mode searches for matches of outline-regexp and at each match the
function in outline-level is called to determine the nesting level:

,----[ C-h v outline-level RET ]
| outline-level's value is outline-level
| 
| *Function of no args to compute a header's nesting level in an outline.
| It can assume point is at the beginning of a header line and that the match
| data reflects the `outline-regexp'.
| 
| Defined in `outline'.
`----

Does my patch make more sense now?

Lute.

Re: lisp-outline-level.

[David Kastrup]

Lute Kamstra <Lute.Kamstra.lists@xs4all.nl> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> Lute Kamstra <Lute.Kamstra.lists@xs4all.nl> writes:
>>
>>> In (Emacs) Lisp mode, outline-regexp is ";;;;* [^ \t\n]\\|(" and
>>> outline-level is lisp-outline-level:
>>>
>>> (defun lisp-outline-level ()
>>>   "Lisp mode `outline-level' function."
>>>   (if (looking-at "(\\|;;;###autoload")
>>>       1000
>>>     (looking-at outline-regexp)
>>>     (- (match-end 0) (match-beginning 0))))
>>>
>>> This is a bit strange as outline-regexp doesn't match
>>> ";;;###autoload".
>>
>> Why is that strange?  outline-regexp is not even consulted when
>> ;###autoload is found, so I don't see how it would come into play
>> here.
>>
>>> Shall I commit the patch below?
>>
>>> *** lisp/emacs-lisp/lisp-mode.el	1 Feb 2005 15:48:50 -0000	1.171
>>> --- lisp/emacs-lisp/lisp-mode.el	13 Feb 2005 11:03:02 -0000
>>> ***************
>>> *** 212,223 ****
>>>   
>>>   (defun lisp-outline-level ()
>>>     "Lisp mode `outline-level' function."
>>> !   (if (looking-at "(\\|;;;###autoload")
>>>         1000
>>> -     (looking-at outline-regexp)
>>>       (- (match-end 0) (match-beginning 0))))
>>>   
>>> - 
>>> --- 212,221 ----
>>>   
>>>   (defun lisp-outline-level ()
>>>     "Lisp mode `outline-level' function."
>>> !   (if (eq (following-char) ?\()
>>>         1000
>>>       (- (match-end 0) (match-beginning 0))))
>>
>> The patch is completely nonsensical.  It returns rubbish in almost all
>> cases since it is the "looking-at" that established match-end and
>> match-beginning in the first place.
>
> The variables outline-regexp and outline-level are used by Outline
> minor mode.  Did you take a look at their docstrings?  Outline minor
> mode searches for matches of outline-regexp and at each match the
> function in outline-level is called to determine the nesting level:
>
> ,----[ C-h v outline-level RET ]
> | outline-level's value is outline-level
> | 
> | *Function of no args to compute a header's nesting level in an outline.
> | It can assume point is at the beginning of a header line and that the match
> | data reflects the `outline-regexp'.
> | 
> | Defined in `outline'.
> `----
>
> Does my patch make more sense now?

Ok, I did not realize that those functions were essentially hook
functions, sorry.  However, I find that within a Lisp buffer I have

outline-regexp's value is ";;;;* [^ 	\n]\\|("
Local in buffer loaddefs.el; global value is "[*\f]+"

*Regular expression to match the beginning of a heading.
Any line whose beginning matches this regexp is considered to start a heading.
Note that Outline mode only checks this regexp at the start of a line,
so the regexp need not (and usually does not) start with `^'.
The recommended way to set this is with a Local Variables: list
in the file it applies to.  See also `outline-heading-end-regexp'.

You can customize this variable.

Defined in `outline'.

[back]

and this just barely manages to avoid matching ;;;###autoload.  I am
not sure whether this really is intentional.  Especially in light of
the lisp-outline-level definition.

So while I have no clue about the matter at hand, I would just want to
suggest that it might be possible from the look of the previous code
that outline-regexp was intended to also match ;;;###autoload, or at
least that it was considered a valid possibility for a user
customization of outline-regexp in Lisp mode.  In either case it would
not be wrong to keep the check.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: lisp-outline-level.

[Lute Kamstra]

David Kastrup <dak@gnu.org> writes:

>>> Lute Kamstra <Lute.Kamstra.lists@xs4all.nl> writes:
>>>
>>>> In (Emacs) Lisp mode, outline-regexp is ";;;;* [^ \t\n]\\|(" and
>>>> outline-level is lisp-outline-level:

[...]

> Ok, I did not realize that those functions were essentially hook
> functions, sorry.  However, I find that within a Lisp buffer I have
>
> outline-regexp's value is ";;;;* [^ 	\n]\\|("
> Local in buffer loaddefs.el; global value is "[*\f]+"

[...]

> and this just barely manages to avoid matching ;;;###autoload.  I am
> not sure whether this really is intentional.  Especially in light of
> the lisp-outline-level definition.
>
> So while I have no clue about the matter at hand, I would just want to
> suggest that it might be possible from the look of the previous code
> that outline-regexp was intended to also match ;;;###autoload, 

Yup, that's what Stefan hinted at.  My bad.

> or at least that it was considered a valid possibility for a user
> customization of outline-regexp in Lisp mode.  In either case it
> would not be wrong to keep the check.

Well, if you change outline-regexp, you should also check
outline-level as those variables work in concert.

Lute.

Re: lisp-outline-level.

[Stefan Monnier]

> (defun lisp-outline-level ()
>   "Lisp mode `outline-level' function."
>   (if (looking-at "(\\|;;;###autoload")
>       1000
>     (looking-at outline-regexp)
>     (- (match-end 0) (match-beginning 0))))

> This is a bit strange as outline-regexp doesn't match
> ";;;###autoload".  Shall I commit the patch below?

Why did you assume that the ;;;###autoload thingy got there by mistake?
Wouldn't it be better to fix outline-regexp?


        Stefan

Re: lisp-outline-level.

[Lute Kamstra]

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

>> (defun lisp-outline-level ()
>>   "Lisp mode `outline-level' function."
>>   (if (looking-at "(\\|;;;###autoload")
>>       1000
>>     (looking-at outline-regexp)
>>     (- (match-end 0) (match-beginning 0))))
>
>> This is a bit strange as outline-regexp doesn't match
>> ";;;###autoload".  Shall I commit the patch below?
>
> Why did you assume that the ;;;###autoload thingy got there by
> mistake?

Ah, I see you put it in.

> Wouldn't it be better to fix outline-regexp?

I figured that an ;;;###autoload line is not a heading.  If you do
make it a heading, then a thus autoloaded function would become a
two-headed monster.  But maybe I was too rigid in my thinking.  I
suppose you want it added as a heading so that hiding the body of the
previous function doesn't hide the fact that the current function is
autoloaded?  Should I change outline-regexp into 
";;;;*\\( [^ \t\n]\\|###autoload\\)\\|("?


Lute.

Re: lisp-outline-level.

[David Kastrup]

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

>> (defun lisp-outline-level ()
>>   "Lisp mode `outline-level' function."
>>   (if (looking-at "(\\|;;;###autoload")
>>       1000
>>     (looking-at outline-regexp)
>>     (- (match-end 0) (match-beginning 0))))
>
>> This is a bit strange as outline-regexp doesn't match
>> ";;;###autoload".  Shall I commit the patch below?
>
> Why did you assume that the ;;;###autoload thingy got there by mistake?
> Wouldn't it be better to fix outline-regexp?

Anyway, I find the original butt-ugly and inefficient.  Wouldn't it be
better to write

(defun lisp-outline-level ()
  "Lisp mode `outline-level' function."
  (if (save-match-data (looking-at "(\\|;;;###autoload")))
      1000
    (- (match-end 0) (match-beginning 0))))

or even (if preservation of match data is definitely not required)

(defun lisp-outline-level
  (let ((len (- (match-end 0) (match-beginning 0))))
    (if  (looking-at ...)
       1000
     len)))


-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

Re: lisp-outline-level.

[Lute Kamstra]

David Kastrup <dak@gnu.org> writes:

[...]

> or even (if preservation of match data is definitely not required)
>
> (defun lisp-outline-level
>   (let ((len (- (match-end 0) (match-beginning 0))))
>     (if  (looking-at ...)
>        1000
>      len)))

A quick look at lisp/outline.el gave me the impression that calls to
outline-level functions are embedded in (save-match-data ...) if the
match data should be preserved.

Lute.

Re: lisp-outline-level.

[Stefan Monnier]

> (defun lisp-outline-level
>   (let ((len (- (match-end 0) (match-beginning 0))))
>     (if  (looking-at ...)
>        1000
>      len)))

Yes, that's probably preferable.


        Stefan

Re: lisp-outline-level.

[Richard Stallman]

    This is a bit strange as outline-regexp doesn't match
    ";;;###autoload".  Shall I commit the patch below?

Does this change give improved behavior in any particular case?
If so, which case?

What happens if you add ;;;###autoload to outline-regexp?

Re: lisp-outline-level.

[Lute Kamstra]

Richard Stallman <rms@gnu.org> writes:

>     This is a bit strange as outline-regexp doesn't match
>     ";;;###autoload".  Shall I commit the patch below?
>
> Does this change give improved behavior in any particular case?
> If so, which case?
>
> What happens if you add ;;;###autoload to outline-regexp?

As Stefan Monnier suggested, I added ;;;###autoload to outline-regexp.
I also improved the efficiency of lisp-outline-level as suggested by
David Kastrup.

Lute.

Re: lisp-outline-level.

[Richard Stallman]

    As Stefan Monnier suggested, I added ;;;###autoload to outline-regexp.

Does this in fact give good results?  I can't anticipate whether
they would be good or bad.

Re: lisp-outline-level.

[Lute Kamstra]

Richard Stallman <rms@gnu.org> writes:

>     As Stefan Monnier suggested, I added ;;;###autoload to outline-regexp.
>
> Does this in fact give good results? 

Yes, this has the advantage that Outline minor mode doesn't consider
an autoload cookie as part of the previous top-level sexp anymore.
(Hiding the body of such a preceding top-level sexp would also hide
the autoload cookie.)

Lute.

Re: lisp-outline-level.

[Richard Stallman]

    Yes, this has the advantage that Outline minor mode doesn't consider
    an autoload cookie as part of the previous top-level sexp anymore.
    (Hiding the body of such a preceding top-level sexp would also hide
    the autoload cookie.)

If you like the results, then this is a good change.  Thanks.