Subject: Re: bug#51809: 29.0.50; [PATCH] Support for outline default state in Diff buffers

From: Matthias Meulien <[email-masked]>
Date: Mon, 13 Dec 2021 08:55:45 +0100

Juri Linkov <[email-masked]> writes:

>> I've this simple patch for the outline.el part.
>
> Thanks, this is a good starting point to add just the basic functionality
> like org-mode initial visibility supported by ‘org-startup-folded’ and
> per-file settings:
>
>    #+STARTUP: fold              (or ‘overview’, this is equivalent)
>    #+STARTUP: nofold            (or ‘showall’, this is equivalent)
>
>    #+STARTUP: content
>    #+STARTUP: show<n>levels (<n> = 2..5)
>
>    #+STARTUP: showeverything
>
>> @@ -1058,13 +1060,16 @@ outline-show-heading
>> -(defun outline-hide-sublevels (levels)
>> +(defun outline-hide-sublevels (levels &optional fun)
>
> It seems you don't use this argument in this patch?

It will be necessary for `diff-mode' to provide its own default state
functions that decide of the visibility of each heading
individually. You're right, it's not needed right now so I'll remove it
and reintroduce it when working on `diff-mode'.

>> +  :local t
>> +  :safe t)
>> +;; TODO fix variable being set through file local variable
>
>> One thing that bothers me is that I am not able to store the wanted
>> default visibility state as a local variable... Any suggestion welcome!
>
> For example, `outline-minor-mode-cycle' and `outline-minor-mode-highlight'
> have no `:local t', but when visiting a file that sets these file local variables,
> then automatically become local.
>
>> +(defun outline-apply-default-state ()
>> +  "Apply the outline state defined by `outline-default-state'."
>> +  (interactive)
>> +  (cond
>> +   ((not outline-default-state) (outline-show-all))
>
> It seems this change doesn't keep the current default behavior.
> Maybe the result will look like it currently works, maybe not.
> Who knows what effect will have calling `outline-show-all'
> by default in some user configurations.

Good point. I can remove that line but one must be aware that calling
`outline-apply-default-state' interactively after changing some headings
visibility won't restore the default state. Not very intuitive...

>
>> +   ((eq outline-default-state 'only-headings)
>> +    (outline-show-all)
>> +    (outline-hide-region-body (point-min) (point-max)))
>> +   ((integerp outline-default-state)
>> +    (outline-hide-sublevels outline-default-state))
>> +   ((when (functionp outline-default-state)
>> +      (funcall outline-default-state)))))
>
> Maybe some other values from org-mode could be supported too?

Sure.

Let me first fix handling of local values. I saw that `outline-mode' is
set *before* local variables when I visit a file whose major mode
default to `outline-mode'. 

> I think the direction is right: first outline could support
> the initial visibility feature, then later various modes
> could use it: in diff-mode, xref, etc.

That's my plan.

For xref mode I am wondering whether "regexp" based outline mode is the
right thing to use. Should there be an outline mode based on
text-properties? The buffer content is built internally and we know
where "headings" are inserted... But it's another topic.

Thanks for reading the patch! I'll send another one, hopefully taking
your remarks into account, when support of local variables is fixed.
-- 
Matthias