Subject: Re: bug#3973: cleanup view-mode usage in vc-resynch-window

From: Dan Nicolaescu <[email-masked]>
Date: Thu, 30 Jul 2009 23:28:10 -0700 (PDT)

Stefan Monnier <[email-masked]> writes:

  > >> > I have removed that code locally for a while and didn't see any
  > >> > problem.  But I don't explicitly use view-mode, so...
  > >> 
  > >> Obiously the code only makes a difference when you use view-read-only,
  > >> so you'll need to set it to a non-nil value before doing any test.
  > 
  > > [I've never done anything with view-read-only before...]
  > > I set view-read-only to t, then played with a file under RCS.
  > > When the files is checked in view-mode is turned on, when checking it
  > > out it's turned off.
  > 
  > > If the code in question is eliminated, view-mode stays turned on after
  > > checking out.
  > 
  > > There's some code in `after-find-file' (that gets called from
  > > `revert-buffer') that looks like it does something very similar to the
  > > code in question here, but `after-find-file' is called with `nomodes' t,
  > > so that code does not trigger.
  > 
  > So it seems the code is still needed.  Maybe we could/should replace it
  > with something more clear: E.g. move that code to a new function
  > `view-refresh' (or whatever else), which we can then call from VC.

The function would look like this:

(defun view-maybe-enter-exit (file)
  (if (file-writable-p file)
      (and view-mode
         (let ((view-old-buffer-read-only nil))
              (view-mode-exit)))
    (and (not view-mode)
     (not (eq (get major-mode 'mode-class) 'special))
      (view-mode-enter))))

but the calling convention is strange: it needs to be called with a
buffer selected and pass the file name...  Not sure it's worth it.
At least the TODO comment can go and be replaced with a comment about
why changing the state of view-mode is needed there.

  > Maybe a better option in the longer term is refine the meaning of the
  > `preserve-modes' arg to revert-buffer, so that some minor modes can
  > request to be refreshed or not (the VC state is one that would also
  > benefit from such a change, as can be seen in auto-revert-mode).

That sounds like a good idea.  Unfortunately I am not familiar with that
code, it is quite complex and critical, and I don't think it's too
important to fix it now, so I'll punt.