Re: [Emacs-diffs] master abd1825: Ensure that we can find definitions when buffer is narrowed

Re: [Emacs-diffs] master abd1825: Ensure that we can find definitions when buffer is narrowed

[Charles A. Roelli]

> Date: Sun, 10 Dec 2017 09:54:23 -0500 (EST)
> From: me@wilfred.me.uk (Wilfred Hughes)
> 
> branch: master
> commit abd18254aec76b26e86ae27e91d2c916ec20cc46
> Author: Wilfred Hughes <me@wilfred.me.uk>
> Commit: Wilfred Hughes <me@wilfred.me.uk>
> 
>     Ensure that we can find definitions when buffer is narrowed
>     
>     find-function-search-for-symbol will reuse the existing buffer if
>     we've already opened the file that contains this symbol. However, if
>     the user has narrowed that buffer, we can't find definitions outside
>     the narrowed area.
>     
>     Instead, search the whole file to find definitions, and teach the help
>     buttons to widen if necessary.
>     
>     * lisp/emacs-lisp/find-func.el (find-function-search-for-symbol):
>       Search the whole buffer for the target symbol.
>     
>     * lisp/help-mode.el: Help buttons now widen the target buffer, if
>       narrowing is in effect and the target position is not in that range.
> ---
>  lisp/emacs-lisp/find-func.el | 46 +++++++++++++++++++++++---------------------
>  lisp/help-mode.el            | 45 ++++++++++++++++++++++++++++++-------------
>  2 files changed, 56 insertions(+), 35 deletions(-)

Thanks for fixing this.  If I may suggest it, it might also be useful
for these functions to respect `widen-automatically':

  widen-automatically is a variable defined in ‘simple.el’.
  Its value is t

  Documentation:
  Non-nil means it is ok for commands to call ‘widen’ when they want to.
  Some commands will do this in order to go to positions outside
  the current accessible part of the buffer.

  If ‘widen-automatically’ is nil, these commands will do something else
  as a fallback, and won’t change the buffer bounds.

Also, I wonder if it would be worth the trouble to define a
buffer-local ring of narrowings, so that a user could return to a
previous narrowing after executing a command that changes to a
different one.

Re: [Emacs-diffs] master abd1825: Ensure that we can find definitions when buffer is narrowed

[Dmitry Gutov]

On 12/11/17 10:23 PM, Charles A. Roelli wrote:

> Thanks for fixing this.  If I may suggest it, it might also be useful
> for these functions to respect `widen-automatically':

Probably not find-function-search-for-symbol though?

>    widen-automatically is a variable defined in ‘simple.el’.
>    Its value is t
> 
>    Documentation:
>    Non-nil means it is ok for commands to call ‘widen’ when they want to.
>    Some commands will do this in order to go to positions outside
>    the current accessible part of the buffer.
> 
>    If ‘widen-automatically’ is nil, these commands will do something else
>    as a fallback, and won’t change the buffer bounds.

I wonder if we're using this variable in all the places we should.

A recent, heated discussion comes to mind.

Re: [Emacs-diffs] master abd1825: Ensure that we can find definitions when buffer is narrowed

[Charles A. Roelli]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 12 Dec 2017 17:11:29 +0200
> 
> On 12/11/17 10:23 PM, Charles A. Roelli wrote:
> 
> > Thanks for fixing this.  If I may suggest it, it might also be useful
> > for these functions to respect `widen-automatically':
> 
> Probably not find-function-search-for-symbol though?

Yes, you are right.

> >    widen-automatically is a variable defined in ‘simple.el’.
> >    Its value is t
> > 
> >    Documentation:
> >    Non-nil means it is ok for commands to call ‘widen’ when they want to.
> >    Some commands will do this in order to go to positions outside
> >    the current accessible part of the buffer.
> > 
> >    If ‘widen-automatically’ is nil, these commands will do something else
> >    as a fallback, and won’t change the buffer bounds.
> 
> I wonder if we're using this variable in all the places we should.

Not sure.  Maybe the first step would be to document it and make it
customizable, so that people can find it.  Then we can add support for
it in places that lack it.

Re: [Emacs-diffs] master abd1825: Ensure that we can find definitions when buffer is narrowed

[Stefan Monnier]

>> Non-nil means it is ok for commands to call ‘widen’ when they want to.
>> Some commands will do this in order to go to positions outside
>> the current accessible part of the buffer.
>> If ‘widen-automatically’ is nil, these commands will do something else
>> as a fallback, and won’t change the buffer bounds.

> I wonder if we're using this variable in all the places we should.
> A recent, heated discussion comes to mind.

Its docstring also doesn't really make it clear what it means and how to
use it.  Looking at the current users, a slightly different var might
work better.  E.g.

    (defvar widen-to-pos-function
       (lambda (pos)
         (if (not (<= (point-min) pos (point-max)))
             (widen))))

this would simplify the two places I could find which check the value of
this var (xref and pop-global-mark).  And it would let the only user
who sets it (to nil) set it to a more useful value (a function which
jumps to the page in which that position is visible).

Also I think the docstring should clarify that it is meant for "user
commands" (i.e. not to replace things like font-lock-dont-widen).


        Stefan

Re: [Emacs-diffs] master abd1825: Ensure that we can find definitions when buffer is narrowed

[Charles A. Roelli]

> Its docstring also doesn't really make it clear what it means and how to
> use it.  Looking at the current users, a slightly different var might
> work better.  E.g.

>     (defvar widen-to-pos-function
>        (lambda (pos)
>          (if (not (<= (point-min) pos (point-max)))
>              (widen))))

Sounds good to me.  What do you think would be better, to supplement
widen-automatically with widen-to-pos-function, or remove
widen-automatically and only offer widen-to-pos-function?  From a
user's perspective, it might be useful to keep both around (allowing
widen-automatically to easily be set to nil in some buffers where it's
unwanted).

Re: [Emacs-diffs] master abd1825: Ensure that we can find definitions when buffer is narrowed

[Stefan Monnier]

> From a user's perspective, it might be useful to keep both around
> (allowing widen-automatically to easily be set to nil in some buffers
> where it's unwanted).

(setq-local widen-to-pos-function #'ignore)

would work just as well.


        Stefan