bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows

bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows

[Lawrence Mitchell]

In GNU Emacs 23.0.60.3 (i686-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2008-09-24 on lamacq.ph.ed.ac.uk
Windowing system distributor `The X.Org Foundation', version 11.0.60802000
configured using `configure  '-C' '--prefix=/scratch/s0198183/applications/emacs-trunk/' 'CFLAGS=-ggdb3 -O0' '--without-rsvg''

If an Emacs frame is split into multiple windows (two will do) of
which one is dedicated and compilation-scroll-output is t then
`compile' in the non-dedicated window will end up moving point.

Steps to reproduce:

emacs -Q --eval \
'(progn 
  (split-window-vertically)
  (set-window-dedicated-p (selected-window) t)
  (switch-to-buffer-other-window "*test*")
  (insert initial-scratch-message)
  (goto-char (point-min))
  (setq compilation-scroll-output t)
  (compile "echo \"test\"")
  (kill-buffer "*compilation*"))'

Note how point is left at the /end/ of the buffer *test* rather
than the beginning.  This appears to be a problem with a mismatch
between what `selected-window' and `current-buffer' return.

When this code in `compilation-start' is executed:

    (if (buffer-local-value 'compilation-scroll-output outbuf)
	(save-selected-window
	  (select-window outwin)
	  (goto-char (point-max))))

outwin is #<window 13 on *compilation*>
and after the select-window call (selected-window) is #<window 13
on *compilation*>.  However, at this point (current-buffer)
returns *test*, rather than *compilation*.

Cheers,
Lawrence
-- 
Lawrence Mitchell <wence@gmx.li>

bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 531 bytes --]

 > When this code in `compilation-start' is executed:
 >
 >     (if (buffer-local-value 'compilation-scroll-output outbuf)
 > 	(save-selected-window
 > 	  (select-window outwin)
 > 	  (goto-char (point-max))))
 >
 > outwin is #<window 13 on *compilation*>
 > and after the select-window call (selected-window) is #<window 13
 > on *compilation*>.  However, at this point (current-buffer)
 > returns *test*, rather than *compilation*.

This is a very, very great nuisance of `select-window'.  Would the
attached patch DTRT?

martin

[-- Attachment #2: 1073.diff --]
[-- Type: text/plain, Size: 993 bytes --]

*** progmodes/compile.el.~1.476.~	2008-06-13 18:22:16.000000000 +0200
--- progmodes/compile.el	2008-10-03 14:31:45.718750000 +0200
***************
*** 1280,1289 ****
  		(cons proc compilation-in-progress))))
        ;; Now finally cd to where the shell started make/grep/...
        (setq default-directory thisdir))
!     (if (buffer-local-value 'compilation-scroll-output outbuf)
! 	(save-selected-window
! 	  (select-window outwin)
! 	  (goto-char (point-max))))
      ;; Make it so the next C-x ` will use this buffer.
      (setq next-error-last-buffer outbuf)))
  
--- 1280,1290 ----
  		(cons proc compilation-in-progress))))
        ;; Now finally cd to where the shell started make/grep/...
        (setq default-directory thisdir))
! 
!     (with-current-buffer (window-buffer outwin)
!       (when (local-variable-p 'compilation-scroll-output)
! 	(goto-char (point-max))))
! 
      ;; Make it so the next C-x ` will use this buffer.
      (setq next-error-last-buffer outbuf)))
  

bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows

[Lawrence Mitchell]

martin rudalics wrote:

>> When this code in `compilation-start' is executed:

>>     (if (buffer-local-value 'compilation-scroll-output outbuf)
>> 	(save-selected-window
>> 	  (select-window outwin)
>> 	  (goto-char (point-max))))

>> outwin is #<window 13 on *compilation*>
>> and after the select-window call (selected-window) is #<window 13
>> on *compilation*>.  However, at this point (current-buffer)
>> returns *test*, rather than *compilation*.

> This is a very, very great nuisance of `select-window'.  Would the
> attached patch DTRT?

The attached patch fixes the observed problem, however, I do not
think that it is entirely correct:

[...]
> !
> !     (with-current-buffer (window-buffer outwin)
> !       (when (local-variable-p 'compilation-scroll-output)

local-variable-p will return t if compilation-scroll-output is
nil, but buffer-local.  I think you mean
(when (buffer-local-value 'compilation-scroll-output (current-buffer))
  ...)
?

[...]

Cheers,
Lawrence

bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows

[Lennart Borgman (gmail)]

martin rudalics wrote:
>> When this code in `compilation-start' is executed:
>>
>>     (if (buffer-local-value 'compilation-scroll-output outbuf)
>>     (save-selected-window
>>       (select-window outwin)
>>       (goto-char (point-max))))
>>
>> outwin is #<window 13 on *compilation*>
>> and after the select-window call (selected-window) is #<window 13
>> on *compilation*>.  However, at this point (current-buffer)
>> returns *test*, rather than *compilation*.
> 
> This is a very, very great nuisance of `select-window'.  Would the
> attached patch DTRT?

I did not try the patch so maybe I misunderstand it. But is not the
problem that (current-buffer) is not the same as (window-buffer)?

If so then perhaps using

  (save-selected-window
    (select-window outwin)
    (with-current-buffer (window-buffer)
      (goto-char (point-max)))

would do the expected thing?

But it looks strange to me. How does one know that this will change the
point in outwin? It would be much cleaner with something like a defmacro
`with-window-buffer' which would take a window as its first argument.
With that defmacro the code would be

  (with-window-buffer outwin
    (goto-char (point-max))

bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 555 bytes --]

 > The attached patch fixes the observed problem, however, I do not
 > think that it is entirely correct:
 >
 > [...]
 >> !
 >> !     (with-current-buffer (window-buffer outwin)
 >> !       (when (local-variable-p 'compilation-scroll-output)
 >
 > local-variable-p will return t if compilation-scroll-output is
 > nil, but buffer-local.

You're right.

 > I think you mean
 > (when (buffer-local-value 'compilation-scroll-output (current-buffer))

Not really.  This would be t in *test*, I suppose.  Try the attached one
instead, please.

Thanks, martin.

[-- Attachment #2: 1073.diff --]
[-- Type: text/plain, Size: 1030 bytes --]

*** progmodes/compile.el.~1.476.~	2008-06-13 18:22:16.000000000 +0200
--- progmodes/compile.el	2008-10-03 15:11:15.406250000 +0200
***************
*** 1280,1289 ****
  		(cons proc compilation-in-progress))))
        ;; Now finally cd to where the shell started make/grep/...
        (setq default-directory thisdir))
!     (if (buffer-local-value 'compilation-scroll-output outbuf)
! 	(save-selected-window
! 	  (select-window outwin)
! 	  (goto-char (point-max))))
      ;; Make it so the next C-x ` will use this buffer.
      (setq next-error-last-buffer outbuf)))
  
--- 1280,1291 ----
  		(cons proc compilation-in-progress))))
        ;; Now finally cd to where the shell started make/grep/...
        (setq default-directory thisdir))
! 
!     (with-current-buffer (window-buffer outwin)
!       (when (and (local-variable-p 'compilation-scroll-output)
! 		 compilation-scroll-output)
! 	(goto-char (point-max))))
! 
      ;; Make it so the next C-x ` will use this buffer.
      (setq next-error-last-buffer outbuf)))
  

bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows

[martin rudalics]

 > I did not try the patch so maybe I misunderstand it. But is not the
 > problem that (current-buffer) is not the same as (window-buffer)?

Yes.  But the real problem is that `select-window' returns immediately
(that is, without making its buffer current) when its argument is the
selected window.

martin

bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows

[Lennart Borgman (gmail)]

martin rudalics wrote:
>> I did not try the patch so maybe I misunderstand it. But is not the
>> problem that (current-buffer) is not the same as (window-buffer)?
> 
> Yes.  But the real problem is that `select-window' returns immediately
> (that is, without making its buffer current) when its argument is the
> selected window.

Thanks, that was what I thought. That is why I am suggesting the
defmacro. I think it would be useful and it is not dangerous to
introduce it. Even in a feature freeze and can be of value to fixing
problems with this strange behaviour of select-window. And the semantic
of the defmacro could be the same even if select-window is "corrected".

And the changes, but since you did not answer to that I guess I was
misunderstanding something there.

bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows

[martin rudalics]

 > And the changes, but since you did not answer to that I guess I was
 > misunderstanding something there.

I'm not sure what you mean but the change you proposed missed the
local-variable issue completely.

martin

bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows

[Lennart Borgman (gmail)]

martin rudalics wrote:
>> And the changes, but since you did not answer to that I guess I was
>> misunderstanding something there.
> 
> I'm not sure what you mean but the change you proposed missed the
> local-variable issue completely.

Ah, my bad. I was just misreading. And the macro is of course pointless
since one can always do what you did, ie

(with-current-buffer (window-buffer outwin)
  ...

bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows

[Lawrence Mitchell]

martin rudalics wrote:

>> I think you mean
>> (when (buffer-local-value 'compilation-scroll-output (current-buffer))

> Not really.  This would be t in *test*, I suppose.  Try the attached one
> instead, please.

Ah yes.  The updated patch does work correctly.

Cheers,
Lawrence

bug#1073: marked as done (23.0.60; Bad interaction between compilation-scroll-output and dedicated windows)

[Emacs bug Tracking System]

[-- Attachment #1: Type: text/plain, Size: 929 bytes --]


Your message dated Sat, 04 Oct 2008 12:07:37 +0200
with message-id <48E74069.6000809@gmx.at>
and subject line Re: bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated	windows
has caused the Emacs bug report #1073,
regarding 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact don@donarmstrong.com
immediately.)


-- 
1073: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=1073
Emacs Bug Tracking System
Contact don@donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 4205 bytes --]

From: Lawrence Mitchell <wence@gmx.li>
To: emacs-pretest-bug@gnu.org
Subject: 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows
Date: Fri, 03 Oct 2008 10:20:21 +0100
Message-ID: <?fnord?y3h6skre827e.fsf@ID-97657.user.individual.net>

In GNU Emacs 23.0.60.3 (i686-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2008-09-24 on lamacq.ph.ed.ac.uk
Windowing system distributor `The X.Org Foundation', version 11.0.60802000
configured using `configure  '-C' '--prefix=/scratch/s0198183/applications/emacs-trunk/' 'CFLAGS=-ggdb3 -O0' '--without-rsvg''

If an Emacs frame is split into multiple windows (two will do) of
which one is dedicated and compilation-scroll-output is t then
`compile' in the non-dedicated window will end up moving point.

Steps to reproduce:

emacs -Q --eval \
'(progn 
  (split-window-vertically)
  (set-window-dedicated-p (selected-window) t)
  (switch-to-buffer-other-window "*test*")
  (insert initial-scratch-message)
  (goto-char (point-min))
  (setq compilation-scroll-output t)
  (compile "echo \"test\"")
  (kill-buffer "*compilation*"))'

Note how point is left at the /end/ of the buffer *test* rather
than the beginning.  This appears to be a problem with a mismatch
between what `selected-window' and `current-buffer' return.

When this code in `compilation-start' is executed:

    (if (buffer-local-value 'compilation-scroll-output outbuf)
	(save-selected-window
	  (select-window outwin)
	  (goto-char (point-max))))

outwin is #<window 13 on *compilation*>
and after the select-window call (selected-window) is #<window 13
on *compilation*>.  However, at this point (current-buffer)
returns *test*, rather than *compilation*.

Cheers,
Lawrence
-- 
Lawrence Mitchell <wence@gmx.li>



[-- Attachment #3: Type: message/rfc822, Size: 1735 bytes --]

From: martin rudalics <rudalics@gmx.at>
To: 1073-done@emacsbugs.donarmstrong.com
Cc: Lawrence Mitchell <wence@gmx.li>
Subject: Re: bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated	windows
Date: Sat, 04 Oct 2008 12:07:37 +0200
Message-ID: <48E74069.6000809@gmx.at>

I checked in a slightly different fix.  Please have a look.

Thanks, martin.

bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows

[Stefan Monnier]

>> When this code in `compilation-start' is executed:
>> 
>> (if (buffer-local-value 'compilation-scroll-output outbuf)
>> (save-selected-window
>> (select-window outwin)
>> (goto-char (point-max))))
>> 
>> outwin is #<window 13 on *compilation*>
>> and after the select-window call (selected-window) is #<window 13
>> on *compilation*>.  However, at this point (current-buffer)
>> returns *test*, rather than *compilation*.

> This is a very, very great nuisance of `select-window'.  Would the
> attached patch DTRT?

> martin

> *** progmodes/compile.el.~1.476.~	2008-06-13 18:22:16.000000000 +0200
> --- progmodes/compile.el	2008-10-03 14:31:45.718750000 +0200
> ***************
> *** 1280,1289 ****
>   		(cons proc compilation-in-progress))))
>         ;; Now finally cd to where the shell started make/grep/...
>         (setq default-directory thisdir))
> !     (if (buffer-local-value 'compilation-scroll-output outbuf)
> ! 	(save-selected-window
> ! 	  (select-window outwin)
> ! 	  (goto-char (point-max))))
>       ;; Make it so the next C-x ` will use this buffer.
>       (setq next-error-last-buffer outbuf)))
  
> --- 1280,1290 ----
>   		(cons proc compilation-in-progress))))
>         ;; Now finally cd to where the shell started make/grep/...
>         (setq default-directory thisdir))
> ! 
> !     (with-current-buffer (window-buffer outwin)
> !       (when (local-variable-p 'compilation-scroll-output)
> ! 	(goto-char (point-max))))
> ! 
>       ;; Make it so the next C-x ` will use this buffer.
>       (setq next-error-last-buffer outbuf)))
  
I think the select-window thingy is/was needed in order to make sure we
move point in the relevant window, rather than just moving point in the
relevant buffer.
So maye something like:

    (with-selected-window outwin
      (with-current-buffer (window-buffer outwin)
        ...))

would be better.


        Stefan

bug#1073: 23.0.60; Bad interaction between compilation-scroll-output and dedicated windows

[martin rudalics]

 > I think the select-window thingy is/was needed in order to make sure we
 > move point in the relevant window, rather than just moving point in the
 > relevant buffer.
 > So maye something like:
 >
 >     (with-selected-window outwin
 >       (with-current-buffer (window-buffer outwin)
 >         ...))
 >
 > would be better.

The original code had

     (goto-char (point-max))

which moves point in all windows showing the relevant buffer.  In that
sense I didn't change the semantics of the original code.

IIUC you want something similar to the

	;; Position point as the user will see it.
	(let ((desired-visible-point
	       ;; Put it at the end if `compilation-scroll-output' is set.
	       (if compilation-scroll-output
		   (point-max)
		 ;; Normally put it at the top.
		 (point-min))))
	  (if (eq outwin (selected-window))
	      (goto-char desired-visible-point)
	    (set-window-point outwin desired-visible-point)))

stuff a few lines above?  Or am I missing something?

martin