Bug report: REPL view doesn't keep its upper and lower regions' dimensions after being restored

3 views
Skip to first unread message

Marcio Mazza

unread,
Sep 13, 2009, 10:05:18 PM9/13/09
to Cusp Development
1) observe the REPL View: it has two regions, one for input under
another for output.
Take note of their current heights (say they occupy 50% of the
space, each, f.ex.);
2) minimize the REPL View;
3) restore the REPL View;
4) observe the REPL View again: notice the heights of those two
regions has changed;

This behaviour is unexpected and inconvinient, since the size of the
output region is diminished to little more than a line.

(My environment: Cusp 1.0.414, Eclipse 3.5.0, Ubuntu Linux.)

Sergey Kolos

unread,
Sep 14, 2009, 12:50:48 PM9/14/09
to Cusp Development
This is old bug and I remember spending a lot of time on this but
couldn't resolve it - previous behavior hid one part of the REPL
completely, so the best I could do is to prevent that from happening.

Gorsal

unread,
Sep 14, 2009, 5:28:44 PM9/14/09
to Cusp Development
So this bug can be fixed by replacing the appropriate (similar) code
in ReplView.java with

sashListener = new Listener () {
public void handleEvent (Event e) {
Rectangle sashRect = sash.getBounds ();
if( sashRect.height == 0){ // sash is not displayed
return;
}
Rectangle shellRect = notButtons.getClientArea ();
int top = shellRect.height - sashRect.height - limit;
if( e == null ){
e = new Event();
e.y = (shellRect.height*6)/10;
e.x = 0;
}
if (e.y!=0) { //becomes buggy if we allow e.y=0 to get into here
int ey = Math.max(Math.min(e.y, top), limit);
/* if( ey == 0) {
if( limit <= top){
if( sashRect.y <= limit ){
ey = limit;
} else if ( sashRect.y >= top ){
ey = top;
} else {
ey = sashRect.y;
}
} else {
ey = limit;
}
} else {

} */
//sashData.top = new FormAttachment (0,)
if (ey != sashRect.y) {
sashData.top = new FormAttachment (0, ey);
sashData.bottom = new FormAttachment (0, ey + 5);
notButtons.layout ();
}
}
}
};

Gorsal

unread,
Sep 14, 2009, 5:37:22 PM9/14/09
to Cusp Development
Well, i take that back. It does fix it, but when the view is too small
only the output is resized and not the input box. Hmmm.....

Gorsal

unread,
Sep 14, 2009, 5:44:32 PM9/14/09
to Cusp Development
K, so the solution is to check if shellRect.height ==0 and if so don't
execute code. Apparently, on showing the view, it fires twice, once
with
the correct height and one with the incorrect. An eclipse bug???

Marcio Mazza

unread,
Sep 14, 2009, 10:39:51 PM9/14/09
to cusp-dev...@googlegroups.com
Well,

In fact, I think resizing the REPL view should not resize the input region.
I would almost always want to keep it just 1 or 2 lines high.

It would be nice, indeed, that the input region resized itself according to another factor: that it got always as high as needed to just accommodate all the lines typed there.

For me, it seems more natural that resizing the REPL view may affect only the output region.


2009/9/14 Gorsal <se...@tewebs.com>

Gorsal

unread,
Sep 15, 2009, 3:05:52 PM9/15/09
to Cusp Development
Yeah. But some other people might want it the way it is, or to expand
both regions at the same rate, or ..etc... Sounds like an excellent
reason
for emacs like capabilities in clojure with .init files ..(*cough*
*cough*)
On Sep 14, 9:39 pm, Marcio Mazza <marcioma...@gmail.com> wrote:
> Well,
>
> In fact, I think resizing the REPL view should not resize the input region.
> I would almost always want to keep it just 1 or 2 lines high.
>
> It would be nice, indeed, that the input region resized itself according to
> another factor: that it got always as high as needed to just accommodate all
> the lines typed there.
>
> For me, it seems more natural that resizing the REPL view may affect only
> the output region.
>
> 2009/9/14 Gorsal <s...@tewebs.com>

Marcio Mazza

unread,
Oct 20, 2009, 11:05:17 PM10/20/09
to cusp-dev...@googlegroups.com
Well, there are 2 concerns:
1) the usability issue of what's the best behaviour, and
2) how to implement it.

Let's postpone the usability issue (1) for a little later.
Supposing the desired behaviour is the one I initially described, I adapted the "Sash > implement a simple splitter" snippet from
  http://www.eclipse.org/swt/snippets/
  (http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet107.java?view=co)
in order to make a bottom section that:
 * keeps it's height between window resizes, and
 * has its height altered using the sash.

I think this can be used to adapt the REPL sections resizing behaviour.

I attach the source of the altered snippet.

(By the way... I simply don't know how to make the window bigger. No knowledge of SWT. :/ )

2009/9/15 Gorsal <se...@tewebs.com>
Snippet107WithStableBottom.java

Marcio Mazza

unread,
Oct 21, 2009, 1:34:39 PM10/21/09
to cusp-dev...@googlegroups.com
I attach a refactored version of the snippet abstracting the layout.

2009/10/21 Marcio Mazza <marci...@gmail.com>
Snippet107WithStableBottom.java

Marcio Mazza

unread,
Oct 21, 2009, 10:33:25 PM10/21/09
to cusp-dev...@googlegroups.com
Well, I now applied that construction to the ReplView.

One design decision which I think would be valuable to the whole project was to extract that layout abstraction OUT of the ReplView class. This allowed me, for example, to individually experiment its behaviour by means of an auxiliary class ReplViewLayoutExperiment (I attach that) without running the whole plugin.

I attach:
 - a patch (cummulative with the fix_debugger_focus one)
 - the ReplViewLayout (also inside the patch)
 - and ReplViewLayoutExperiment (not part of the patch, but maybe should go into inside some "manual tests" folder)


2009/10/21 Marcio Mazza <marci...@gmail.com>
fix_debugger_focus+fix_and_simplify_REPL_resize.patch
ReplViewLayoutExperiment.java
ReplViewLayout.java

Marcio Mazza

unread,
Oct 21, 2009, 10:35:10 PM10/21/09
to cusp-dev...@googlegroups.com
The patch is against:
Repository Root: http://cusp.googlecode.com/svn
Repository UUID: 230afaf4-6840-11de-89a9-0b20f3e2dceb
Revision: 428

2009/10/22 Marcio Mazza <marci...@gmail.com>
Reply all
Reply to author
Forward
0 new messages