all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#3973: cleanup view-mode usage in vc-resynch-window
@ 2009-07-30  7:44 Dan Nicolaescu
  2009-07-30 21:14 ` Stefan Monnier
  0 siblings, 1 reply; 8+ messages in thread
From: Dan Nicolaescu @ 2009-07-30  7:44 UTC (permalink / raw)
  To: bug-gnu-emacs


vc-resynch-window contains this code:

             ;; TODO: Adjusting view mode might no longer be necessary
             ;; after RMS change to files.el of 1999-08-08.  Investigate
             ;; this when we install the new VC.
             (and view-read-only
                  (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))))

not sure what it's supposed to do... 
But the comment (added in Apr 2000!) seems to say that it can be removed.
Should it?






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#3973: cleanup view-mode usage in vc-resynch-window
  2009-07-30  7:44 bug#3973: cleanup view-mode usage in vc-resynch-window Dan Nicolaescu
@ 2009-07-30 21:14 ` Stefan Monnier
  2009-07-30 22:06   ` Dan Nicolaescu
  0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2009-07-30 21:14 UTC (permalink / raw)
  To: Dan Nicolaescu; +Cc: 3973, bug-gnu-emacs

> not sure what it's supposed to do... 
> But the comment (added in Apr 2000!) seems to say that it can be removed.
> Should it?

I don't know.  Someone needs to try it,


        Stefan







^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#3973: cleanup view-mode usage in vc-resynch-window
  2009-07-30 21:14 ` Stefan Monnier
@ 2009-07-30 22:06   ` Dan Nicolaescu
       [not found]     ` <jwv1vnxbowd.fsf-monnier+emacsbugreports@gnu.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Dan Nicolaescu @ 2009-07-30 22:06 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 3973

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

  > > not sure what it's supposed to do... 
  > > But the comment (added in Apr 2000!) seems to say that it can be removed.
  > > Should it?
  > 
  > I don't know.  Someone needs to try it,

I have removed that code locally for a while and didn't see any
problem. But I don't explicitly use view-mode, so...
How about remove the code and see if someone sees a problem?





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#3973: cleanup view-mode usage in vc-resynch-window
       [not found]     ` <jwv1vnxbowd.fsf-monnier+emacsbugreports@gnu.org>
@ 2009-07-31  3:22       ` Dan Nicolaescu
  2009-07-31  3:43         ` Stefan Monnier
  0 siblings, 1 reply; 8+ messages in thread
From: Dan Nicolaescu @ 2009-07-31  3:22 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 3973

Stefan Monnier <monnier@iro.umontreal.ca> 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.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#3973: cleanup view-mode usage in vc-resynch-window
  2009-07-31  3:22       ` Dan Nicolaescu
@ 2009-07-31  3:43         ` Stefan Monnier
  2009-07-31  6:28           ` Dan Nicolaescu
  0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2009-07-31  3:43 UTC (permalink / raw)
  To: Dan Nicolaescu; +Cc: 3973

>> > 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.

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).


        Stefan





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#3973: cleanup view-mode usage in vc-resynch-window
  2009-07-31  3:43         ` Stefan Monnier
@ 2009-07-31  6:28           ` Dan Nicolaescu
  2009-07-31 19:19             ` Stefan Monnier
  0 siblings, 1 reply; 8+ messages in thread
From: Dan Nicolaescu @ 2009-07-31  6:28 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 3973

Stefan Monnier <monnier@iro.umontreal.ca> 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.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#3973: cleanup view-mode usage in vc-resynch-window
  2009-07-31  6:28           ` Dan Nicolaescu
@ 2009-07-31 19:19             ` Stefan Monnier
  2011-07-12 20:17               ` Lars Magne Ingebrigtsen
  0 siblings, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2009-07-31 19:19 UTC (permalink / raw)
  To: Dan Nicolaescu; +Cc: 3973

> 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.

Why do we need to pass `file'?
Can't we (assert (equal file buffer-file-name))?

> At least the TODO comment can go and be replaced with a comment about
> why changing the state of view-mode is needed there.

Yes, please.

>> 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.

Wise choice,


        Stefan





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#3973: cleanup view-mode usage in vc-resynch-window
  2009-07-31 19:19             ` Stefan Monnier
@ 2011-07-12 20:17               ` Lars Magne Ingebrigtsen
  0 siblings, 0 replies; 8+ messages in thread
From: Lars Magne Ingebrigtsen @ 2011-07-12 20:17 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 3973, Dan Nicolaescu

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

>> At least the TODO comment can go and be replaced with a comment about
>> why changing the state of view-mode is needed there.
>
> Yes, please.

Apparently this was done, so I'm closing this report.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2011-07-12 20:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-30  7:44 bug#3973: cleanup view-mode usage in vc-resynch-window Dan Nicolaescu
2009-07-30 21:14 ` Stefan Monnier
2009-07-30 22:06   ` Dan Nicolaescu
     [not found]     ` <jwv1vnxbowd.fsf-monnier+emacsbugreports@gnu.org>
2009-07-31  3:22       ` Dan Nicolaescu
2009-07-31  3:43         ` Stefan Monnier
2009-07-31  6:28           ` Dan Nicolaescu
2009-07-31 19:19             ` Stefan Monnier
2011-07-12 20:17               ` Lars Magne Ingebrigtsen

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.