find-file-hook, recenter, scroll-conservatively and save-place

find-file-hook, recenter, scroll-conservatively and save-place

[Gergely Risko]

Hi,

I'm using save-place in my .emacs and I also set scroll-conservatively
to 101 with a scroll-margin of 3.

I like the smooth scrolling experience, but I would prefer to have new
files open at their previous save-place location recentered in their
window, not at the end.  This works as intended with
scroll-conservatively 0, but breaks with scroll-conservatively 101.

I did the following digging:
  - scroll-conservatively achieves its behavior in redisplay
    asynchronously to find-file-hook, therefore any kind of dynamic
    binding hackery of setting scroll-conservatively temporarily to 0
    doesn't work for me in save-place,
  - recenter refuses to run when the current window is not showing the
    current buffer (makes sense),
  - find-file-hook is ran with the new buffer as current, but the window
    is not changed to the new buffer yet.

If I have this in find-file-hook:

(defun test ()
  (message "foobar: %s %s" (current-buffer) (selected-window))
  )

And I open new-file.c while standing in old-file.c, I receive this:

Debugger entered--returning value: "foobar: old-file.c #<window 85 on new-file.c>"

My question: is there a better hook that we could use for save-place
restoration purposes?

I also looked into xref, and there the behavior is much better: the
(recenter) is included in the default configuration of
xref-after-jump-hook and it works in all the scenarios that I have
tried.

Thanks,
Gergely

Re: find-file-hook, recenter, scroll-conservatively and save-place

[Gergely Risko]

Hi,

After more digging:
  - find-file-* calls find-file-noselect
  - find-file-noselect always returns a buffer
  - find-file-noselect doesn't return the information if this is a new
    buffer just visiting a file right now or an old one that was open
  - find-file-* then selects the returned buffer and chooses a window,
    displays the window, etc.

The hook that I need (something like find-file-with-window-hook) can
be described like this: similar to find-file-hook, but called after a
window has been chosen or created as necessary and the window's buffer
has been changed to the buffer being opened.  This hook would
guarantee in its documentation that:

  current-buffer === (window-buffer (current-window))

This new hook can be run-hook'd at the end of the various find-file-*
user facing commands.

The communication between the find-file-noselect function and
the find-file-* user facing commands can be implemented in two ways:
  - return some kind of structured data from find-file-noselect (with
    the buffer and my new boolean about "is this a new buffer?")
  - setq a buffer local variable inside find-file-noselect that is
    later queried at the end of the find-file-* commands to decide
    whether to run the find-file-with-window-hook or not.

Conceptually the first sounds cleaner and better, but
find-file-noselect is a function that is documented and therefore this
would been a breaking change.

Therefore I prefer solution 2.

A third alternative would be to rename find-file-noselect to
find-file--noselect-internal, implement my API and provide a backward
compatibility find-file-noselect function.

Any opinions on the mentioned designs?  Which should I go ahead with
and prepare a patch or does anyone have a simpler/better idea or any
other suggestion?  Also, please tell me if this is already a solved
issue and I'm just being dense. :-)

Cheers,
Gergely

Re: find-file-hook, recenter, scroll-conservatively and save-place

[Eli Zaretskii]

> From: Gergely Risko <gergely@risko.hu>
> Date: Thu, 31 Jan 2019 14:46:40 +0100
> 
> The hook that I need (something like find-file-with-window-hook) can
> be described like this: similar to find-file-hook, but called after a
> window has been chosen or created as necessary and the window's buffer
> has been changed to the buffer being opened.  This hook would
> guarantee in its documentation that:
> 
>   current-buffer === (window-buffer (current-window))
> 
> This new hook can be run-hook'd at the end of the various find-file-*
> user facing commands.

Before you go further down this rabbit hole, I suggest to try setting
scroll-conservatively to a smaller value, like 30.  If that does what
you want, and doesn't cause recentering in too many other use cases,
you are done without any changes in Emacs.

Once upon a time scroll-conservatively only affected scroll commands,
such as scroll-up etc.  But then users came up and demanded that this
setting should affect any motion command, including goto-char etc.
What you see is the consequence of that: Emacs does this by popular
demand.  Any command that moves point outside of the window will,
under scroll-conservatively > 100, behave like you see, i.e. always
put point on the last or the first window line.

You could perhaps do what you want by some clever one-time setting in
pre-redisplay-functions.  But it might not be easy, I'm afraid,
because you'd need to do that in a way that works only the first time
a buffer is displayed whose place was saved by save-place.

Re: find-file-hook, recenter, scroll-conservatively and save-place

[Gergely Risko]

Hey Eli,

Thanks for your answer, I was aware of the scroll-conservatively = 30
workaround and I think by now I understand the tradeoffs.

Let me tell you why I don't like it: if I scroll up by sexps and a big
one (>30) is coming, that will cause a recenter, while setting it to
101 will put the sexp to the top of the screen how I expect.

Basically I'm after getting rid of these small inconsistencies, that
scrolling through a 29 line sexp behaves differently than a 30.  I'm
not saying this is the end of the world, just saying that it breaks my
flow a little and would be interested in a fix.  Any opportunity to
learn more about Emacs and do some work I take as a positive, not as a
cost that I should avoid ;-)

> Once upon a time scroll-conservatively only affected scroll commands,
> such as scroll-up etc.  But then users came up and demanded that this
> setting should affect any motion command, including goto-char etc.
> What you see is the consequence of that: Emacs does this by popular
> demand.  Any command that moves point outside of the window will,
> under scroll-conservatively > 100, behave like you see, i.e. always
> put point on the last or the first window line.

Very interesting history, thanks for it!  I was not aware.

I can understand why users complained, and I think it was the correct
reaction of the Emacs maintainers to give in to the popular demand.
This is exactly why I love Emacs compared to some other communities,
which are tech first, user second...

I would imagine that if you interview 100 users about the save-place
situation, they will say recenter at least 90 times.  On the other
hand, they pushed for "scroll-conservatively always" in the past,
because they were simply not aware the meaning of that in the case of
save-place.

Also scroll-conservatively is set to 101 in a lot of starter kits, so
making it more compatible with save-place is a worthwhile goal in my
book, even if setting to 30 is acceptable.  Here is a user complaining
and walking away without a solution or any idea what's wrong:
https://github.com/syl20bnr/spacemacs/issues/10484

Can you maybe give me some directions and oversight to fix this in a
way that is acceptable?  I would be willing to work on this for the
fun of it.  Of course, we would leave the defaults as it is now for
other use cases, so the users with pitchforks don't return. ;)

What would be an acceptable way?  I think by now we have mentioned
three completely different approaches at least.

Very low level (C): an alternative goto-char that somehow
signals the redisplay about how to position the window? (So selectable
behavior between the historical and the current one in the API.)

Mid-level (Elisp, maybe some C): make recenter callable before we have
buffer and window correctly setup and then somehow remember this
recenter request for later? (So basically make recenter callable in a
find-file-hook)

High-level (Elisp only, sounds small and easy): providing the missing
find-file-with-window-hook, using some approach from my previous email?

> You could perhaps do what you want by some clever one-time setting in
> pre-redisplay-functions.  But it might not be easy, I'm afraid,
> because you'd need to do that in a way that works only the first time
> a buffer is displayed whose place was saved by save-place.

Yes, I already came up with a workaround:

  (defvar-local nce/flagged-for-recenter nil)
  (defun nce/flag-for-recenter ()
    (setq-local nce/flagged-for-recenter t))

  (defun nce/recenter-if-flagged ()
    (when nce/flagged-for-recenter
      (recenter)
      (setq nce/flagged-for-recenter nil)))

  (save-place-mode t)
  (add-hook 'find-file-hook 'nce/flag-for-recenter)
  (defadvice find-file                     (after nce/find-file-save-place-ff-fix activate) (nce/recenter-if-flagged))
  (defadvice find-file-other-window        (after nce/find-file-save-place-ow-fix activate) (nce/recenter-if-flagged))
  (defadvice find-file-other-frame         (after nce/find-file-save-place-of-fix activate) (nce/recenter-if-flagged))
  (defadvice find-file-literally           (after nce/find-file-save-place-fl-fix activate) (nce/recenter-if-flagged))
  ;; these 2 are required during startup (startup.el uses find-file-noselect directly)
  (defadvice switch-to-buffer              (after nce/find-file-save-place-sb-fix activate) (nce/recenter-if-flagged))
  (defadvice switch-to-buffer-other-window (after nce/find-file-save-place-so-fix activate) (nce/recenter-if-flagged))

So basically, this is solution number 3, high level, I provide myself
the find-file-with-window-hook with advices.  I think this is ugly and
I would prefer to get rid of these once I have my shiny Emacs 27. :)

I know this sounds like a small problem, but it works for xref, I'm
sure we can fix save-place in an acceptable way.

Cheers,
Gergely

Re: find-file-hook, recenter, scroll-conservatively and save-place

[martin rudalics]

 > My question: is there a better hook that we could use for save-place
 > restoration purposes?
 >
 > I also looked into xref, and there the behavior is much better: the
 > (recenter) is included in the default configuration of
 > xref-after-jump-hook and it works in all the scenarios that I have
 > tried.

Juri, should we handle this somehow?  That is, provide 'window-point'
and 'window-start' action alist entries with the former allowing the
'switch-to-buffer-preserve-window-point' logic (among others) and the
latter optionally allowing to recenter.  I don't know how 'save-place'
could pass these on to the 'pop-to-buffer' call in 'find-file' though.
Can you think of any other use cases where these would be helpful?

martin

Re: find-file-hook, recenter, scroll-conservatively and save-place

[Eli Zaretskii]

> Date: Thu, 31 Jan 2019 14:49:17 +0100
> From: martin rudalics <rudalics@gmx.at>
> Cc: Juri Linkov <juri@jurta.org>
> 
> Juri, should we handle this somehow?  That is, provide 'window-point'
> and 'window-start' action alist entries with the former allowing the
> 'switch-to-buffer-preserve-window-point' logic (among others) and the
> latter optionally allowing to recenter.

How would you compute window-start in this case so that point is
centered?

> I don't know how 'save-place' could pass these on to the
> 'pop-to-buffer' call in 'find-file' though.  Can you think of any
> other use cases where these would be helpful?

save-place installs a find-file-hook, which simply calls goto-char.
All this happens before pop-to-buffer is called.

Re: find-file-hook, recenter, scroll-conservatively and save-place

[martin rudalics]

 >> Juri, should we handle this somehow?  That is, provide 'window-point'
 >> and 'window-start' action alist entries with the former allowing the
 >> 'switch-to-buffer-preserve-window-point' logic (among others) and the
 >> latter optionally allowing to recenter.
 >
 > How would you compute window-start in this case so that point is
 > centered?

When 'display-buffer' finds a '(window-start . recenter) ALIST entry
it would call 'recenter' after assigning the window buffer.

 >> I don't know how 'save-place' could pass these on to the
 >> 'pop-to-buffer' call in 'find-file' though.  Can you think of any
 >> other use cases where these would be helpful?
 >
 > save-place installs a find-file-hook, which simply calls goto-char.
 > All this happens before pop-to-buffer is called.

Which probably means that no 'window-point' entry is needed -
'(window-point . nil) would mean to use the buffer's point as
window-point.

martin

Re: find-file-hook, recenter, scroll-conservatively and save-place

[Gergely Risko]

On 2019-01-31 19:44 (Thursday), martin rudalics <rudalics@gmx.at> writes:
> When 'display-buffer' finds a '(window-start . recenter) ALIST entry
> it would call 'recenter' after assigning the window buffer.

I reproduced this behavior with advices and hooks in current Emacs, so
we can play with it.  This is a self-contained ~/.emacs (that I'm
testing with --no-site-file --no-site-lisp --no-splash):

-=-=-=-=-=-=-=-=-=-=-
(save-place-mode 1)
(setq vc-follow-symlinks t)
(setq scroll-margin 3)
(setq scroll-conservatively 101)

(defvar-local nce/flagged-for-recenter nil)
(defun nce/flag-for-recenter ()
  (setq-local nce/flagged-for-recenter t))

(defun nce/recenter-if-flagged (ad-do-it buffer &rest args)
  (let ((window (apply ad-do-it buffer args)))
    (when (buffer-local-value 'nce/flagged-for-recenter buffer)
      (setq-local nce/flagged-for-recenter nil)
      (with-selected-window window
        (with-current-buffer buffer
          (condition-case nil (recenter) ('error t)))))
    window))

(add-hook 'find-file-hook 'nce/flag-for-recenter)
(advice-add 'display-buffer :around 'nce/recenter-if-flagged)
-=-=-=-=-=-=-=-=-=-=-

It works quite well and it's a lot better than my previous find-file
based solution (sent to Eli), because we only need one advice on
display-buffer and not on all the various find file stuff.

I only discovered one issue, startup.el contains this regarding
command line parsing:
-=-=-=-=-=-=-=-=-=-=-
    (let ((displayable-buffers-len (length displayable-buffers))
          ;; `nondisplayed-buffers-p' is true if there exist buffers
          ;; in `displayable-buffers' that were not displayed to the
          ;; user.
          (nondisplayed-buffers-p nil))
      (when (> displayable-buffers-len 0)
        (switch-to-buffer (car displayable-buffers)))   <------
      (when (> displayable-buffers-len 1)
        (switch-to-buffer-other-window (car (cdr displayable-buffers)))
        ;; Focus on the first buffer.
        (other-window -1))
      (when (> displayable-buffers-len 2)
-=-=-=-=-=-=-=-=-=-=-

Here, the second file is properly recenterd, because it uses
switch-to-buffer-other-window (so our display-buffer is called), but
the first file is simply displayed with switch-to-buffer.  So I guess
we would have to put a recenter here in startup.el conditional on the
buffer-local variable that was meant to display-buffer by the
save-place hook.  Or maybe replace the marked switch-to-buffer call
with: (display-buffer (...) '(display-buffer-same-window))

Gergely

Re: find-file-hook, recenter, scroll-conservatively and save-place

[martin rudalics]

 > ..., but
 > the first file is simply displayed with switch-to-buffer.  So I guess
 > we would have to put a recenter here in startup.el conditional on the
 > buffer-local variable that was meant to display-buffer by the
 > save-place hook.  Or maybe replace the marked switch-to-buffer call
 > with: (display-buffer (...) '(display-buffer-same-window))

Emacs 27 has 'switch-to-buffer-obey-display-actions'.  Can you try
that?

martin

Re: find-file-hook, recenter, scroll-conservatively and save-place

[Juri Linkov]

>> My question: is there a better hook that we could use for save-place
>> restoration purposes?
>>
>> I also looked into xref, and there the behavior is much better: the
>> (recenter) is included in the default configuration of
>> xref-after-jump-hook and it works in all the scenarios that I have
>> tried.
>
> Juri, should we handle this somehow?

I don't like the default recentering too.  I had to fix it with
a lot of customization, e.g.

(add-hook 'xref-after-jump-hook 'reposition-window)
(add-hook 'xref-after-return-hook 'reposition-window)
(add-hook 'find-function-after-hook 'reposition-window)

;; Let `C-M-a' (beginning-of-defun) not scroll the window
;; when after jumping point stays within current window bounds
(advice-add 'beginning-of-defun :around
	    (lambda (orig-fun &rest args)
	      (let ((w-s (window-start))
		    (w-e (window-end)))
		(apply orig-fun args)
		(when (and
		       ;; Only when used interactively
		       (eq this-command 'beginning-of-defun)
		       ;; And only when jumping outside of window
		       ;; to the center of the window
		       (or (< (point) w-s) (> (point) w-e)))
		  (recenter 11))))
	    '((name . recenter)))

;; Save and restore window start positions on returning to previous search
(setq isearch-push-state-function
      (lambda ()
	;; Recenter new search hits outside of window boundaries
        (when (and isearch-success (not (pos-visible-in-window-p)))
	  ;; reposition-window takes too much time in large buffers
          (if (or (eq major-mode 'fundamental-mode)
                  (> (buffer-size) 1000000))
	      (recenter 11)
	    (condition-case nil
		;; Prevent errors from reposition-window
		(reposition-window)
	      (error nil))))
        `(lambda (cmd)
           (when isearch-success
             (set-window-start nil ,(window-start))))))

> That is, provide 'window-point' and 'window-start' action alist entries
> with the former allowing the 'switch-to-buffer-preserve-window-point'
> logic (among others) and the latter optionally allowing to recenter.

Like switch-to-buffer-preserve-window-point used in dired-find-file?

> I don't know how 'save-place' could pass these on to the
> 'pop-to-buffer' call in 'find-file' though.  Can you think
> of any other use cases where these would be helpful?

Interesting question.  Maybe introduce two new buffer-local variables
'window-point' and 'window-start' that a hook could set and then
display-buffer could read and call functions window-point and window-start?
These buffer-local values should be used only once and should be reverted
to nil after the first use.

BTW, a related question: should save-place save window-start as well?
It should be easy to implement after this problem is solved.

Re: find-file-hook, recenter, scroll-conservatively and save-place

[Gergely Risko]

On 2019-01-31 22:57 (Thursday), Juri Linkov <juri@jurta.org> writes:
> Interesting question.  Maybe introduce two new buffer-local variables
> 'window-point' and 'window-start' that a hook could set and then
> display-buffer could read and call functions window-point and window-start?
> These buffer-local values should be used only once and should be reverted
> to nil after the first use.

I start to understand the approach proposed by you and Martin.  This
buffer-local variable approach feels natural and OK to me.

Martin also said this:

>> When 'display-buffer' finds a '(window-start . recenter) ALIST entry
>> it would call 'recenter' after assigning the window buffer.

Just one point to this: should we have '(window-start . (recenter 10))
also, where 10 is the ARG for the future recenter call?

> BTW, a related question: should save-place save window-start as well?
> It should be easy to implement after this problem is solved.

I think that would be nice and there is only one corner case I think
we have to take care of: recentf is a long term operation compared to
switch-to-buffer-preserve-window-point in the sense that when the user
comes back after days maybe he is sitting in front of a different
sized screen with differently configured frames and windows.
Therefore we have to give precedence to the point and take the
window-start recommendation of save-place with a grain of salt.  As
far as I can understand it now, fortunately this is done for us
already by redisplay for free.

Gergely

Re: find-file-hook, recenter, scroll-conservatively and save-place

[martin rudalics]

 > Just one point to this: should we have '(window-start . (recenter 10))
 > also, where 10 is the ARG for the future recenter call?

We need a convention for alist entries about how to specify functions
and their arguments.

 >> BTW, a related question: should save-place save window-start as well?
 >> It should be easy to implement after this problem is solved.
 >
 > I think that would be nice and there is only one corner case I think
 > we have to take care of: recentf is a long term operation compared to
 > switch-to-buffer-preserve-window-point in the sense that when the user
 > comes back after days maybe he is sitting in front of a different
 > sized screen with differently configured frames and windows.
 > Therefore we have to give precedence to the point and take the
 > window-start recommendation of save-place with a grain of salt.  As
 > far as I can understand it now, fortunately this is done for us
 > already by redisplay for free.

The restoration would call 'set-window-start' with NOFORCE non-nil so
point would be preserved.

martin

Re: find-file-hook, recenter, scroll-conservatively and save-place

[Juri Linkov]

>> Interesting question.  Maybe introduce two new buffer-local variables
>> 'window-point' and 'window-start' that a hook could set and then
>> display-buffer could read and call functions window-point and window-start?
>> These buffer-local values should be used only once and should be reverted
>> to nil after the first use.
>
> I start to understand the approach proposed by you and Martin.  This
> buffer-local variable approach feels natural and OK to me.
>
> Martin also said this:
>
>>> When 'display-buffer' finds a '(window-start . recenter) ALIST entry
>>> it would call 'recenter' after assigning the window buffer.
>
> Just one point to this: should we have '(window-start . (recenter 10))
> also, where 10 is the ARG for the future recenter call?

Using one-off buffer-local variables will allow standard hooks
like adding window-start-hook.

Re: find-file-hook, recenter, scroll-conservatively and save-place

[Juri Linkov]

>>> Interesting question.  Maybe introduce two new buffer-local variables
>>> 'window-point' and 'window-start' that a hook could set and then
>>> display-buffer could read and call functions window-point and window-start?
>>> These buffer-local values should be used only once and should be reverted
>>> to nil after the first use.
>>
>> I start to understand the approach proposed by you and Martin.  This
>> buffer-local variable approach feels natural and OK to me.
>>
>> Martin also said this:
>>
>>>> When 'display-buffer' finds a '(window-start . recenter) ALIST entry
>>>> it would call 'recenter' after assigning the window buffer.
>>
>> Just one point to this: should we have '(window-start . (recenter 10))
>> also, where 10 is the ARG for the future recenter call?
>
> Using one-off buffer-local variables will allow standard hooks
> like adding window-start-hook.

Alternatively, we could use a solution like in windmove-display-in-direction
that sets display-buffer-overriding-action with a lambda (in this case,
with some recentering) until the end of the current command.

Re: find-file-hook, recenter, scroll-conservatively and save-place

[martin rudalics]

 > I don't like the default recentering too.  I had to fix it with
 > a lot of customization, e.g.
 >
 > (add-hook 'xref-after-jump-hook 'reposition-window)
 > (add-hook 'xref-after-return-hook 'reposition-window)
 > (add-hook 'find-function-after-hook 'reposition-window)

Once you get used to recentering you probably like it.  I never
managed to like it.  It's a consequence of our insistence to keep
point on-screen.

 >> That is, provide 'window-point' and 'window-start' action alist entries
 >> with the former allowing the 'switch-to-buffer-preserve-window-point'
 >> logic (among others) and the latter optionally allowing to recenter.
 >
 > Like switch-to-buffer-preserve-window-point used in dired-find-file?

More like we handle 'display-buffer-mark-dedicated' now.  That is,
'switch-to-buffer-preserve-window-point' would be respected by
'display-buffer' unless explicitly overridden by a 'window-point'
entry.  But I dislike the idea of adding yet another variable that
will be eventually handled like an alist entry.

 > Interesting question.  Maybe introduce two new buffer-local variables
 > 'window-point' and 'window-start' that a hook could set and then
 > display-buffer could read and call functions window-point and window-start?
 > These buffer-local values should be used only once and should be reverted
 > to nil after the first use.

Basically, anyone who wants to customize the behavior may bind
'display-buffer-alist' appropriately around the 'find-file' call.  But
this is awkward.  Alists are not very suitable to "affect just this
'display-buffer' call".

 > BTW, a related question: should save-place save window-start as well?
 > It should be easy to implement after this problem is solved.

I have no practice with 'save-place-mode' so I can't tell.  From my
experience I can only tell that it's disorienting when reverting a
buffer doesn't preserve window start.

martin

Re: find-file-hook, recenter, scroll-conservatively and save-place

[Gergely Risko]

On 2019-02-01 10:04 (Friday), martin rudalics <rudalics@gmx.at> writes:
> I have no practice with 'save-place-mode' so I can't tell.  From my
> experience I can only tell that it's disorienting when reverting a
> buffer doesn't preserve window start.

Thanks for catching this, the current proposal toggles the flag in
save-place's hook even on revert-buffer.  Fortunately we have
revert-buffer-in-progress-p, so save-place could do like this:

(defvar-local nce/flagged-for-recenter nil)
(defun nce/flag-for-recenter ()
  (when (not revert-buffer-in-progress-p)
    (setq-local nce/flagged-for-recenter t)))

BTW, my previous fake implementation had some other bugs too :-(

So if anyone is interested here is a better version (still supposed to
be tested with --no-site-file --no-site-lisp --no-splash):
-=-=-=-=-=-
(save-place-mode 1)

(setq vc-follow-symlinks t)
(setq scroll-margin 3)
(setq scroll-conservatively 101)

(defvar-local nce/flagged-for-recenter nil)
(defun nce/flag-for-recenter ()
  (when (not revert-buffer-in-progress-p)
    ;; (message "setting %s %s" (current-buffer) (selected-window))
    (setq-local nce/flagged-for-recenter t)))

(defun nce/recenter-if-flagged (ad-do-it buffer &rest args)
  (let ((window (apply ad-do-it buffer args)))
    (when (buffer-local-value 'nce/flagged-for-recenter buffer)
      (with-selected-window window
	(with-current-buffer buffer
	  ;; (message "unsetting %s" (current-buffer))
	  (setq-local nce/flagged-for-recenter nil)
	  (condition-case nil (recenter) ('error t)))))
    window))

(add-hook 'find-file-hook 'nce/flag-for-recenter)
(advice-add 'display-buffer :around 'nce/recenter-if-flagged)
-=-=-=-=-=-

Martin, I will change save-place to use
`switch-to-buffer-obey-display-actions', as you suggested and will
report back as soon as I have it.

Should I prepare a patch based on these examples, so we have some
existing code to talk about or should we try to come up with something
better than buffer-local variables for passing the info?

Cheers,
Gergely

Re: find-file-hook, recenter, scroll-conservatively and save-place

[martin rudalics]

 > Should I prepare a patch based on these examples, so we have some
 > existing code to talk about or should we try to come up with something
 > better than buffer-local variables for passing the info?

Note that a final implementation should not use advice.  As soon as
Juri has a definite proposal for the values we can install the
'window-start' and 'window-point' alist entry handlings (the easy
part).  The link between 'save-place' and the 'pop-to-buffer' in
'find-file' will be still missing then (the harder part).  I'm not
entirely sure about using one-shot variables.

martin