Activity monitor in the eclipse embedding

5 views
Skip to first unread message

Matt Hillsdon

unread,
Jun 7, 2009, 7:01:00 PM6/7/09
to terminat...@googlegroups.com
Attached is a patch that moves the ownership of the
JAsynchronousProgressIndicator from the JTerminalPane to the
TerminatorTabbedPane, which now registers itself directly with the
TerminalControl as an implementation of the new ActivityMonitor interface.

This allows me to implement activity notification differently in the
Eclipse embedding. My UI options are more limited because I'm not
responsible for rendering tabs but I can make the tab title italic so
I've gone with that.

Are you happy with moving the spinner in this way? It seems to make
things clearer - it was a bit confusing having it owned by a component
that wasn't its UI parent.

Thanks,

Matt.

activityMonitor.patch

Martin Dorey

unread,
Jun 7, 2009, 7:29:12 PM6/7/09
to terminat...@googlegroups.com
The use of == false rather than ! will have been deliberate, to avoid
the ! getting missed next to the parenthesis.

- if (pane.isShowing() == false) {
+ if (!terminalPane.isShowing()) {

Being a C++ programmer, I was a bit surprised by the name here, though
the idiom, of a default listener that does nothing, was familiar:

+ ActivityMonitor NULL = new ActivityMonitor() {

The first extant example of that idiom that I found was NoOpRunnable.
Of course, that has be defined outside the interface that we don't own.
I see we have a NoOpAction and a NoOpIndenter too. Some precedent
there, then.

Well, it looked fine to me and in a good cause. I'd commit it but
Elliott might have more useful comments.

Elliott Hughes

unread,
Jun 7, 2009, 9:09:28 PM6/7/09
to terminator-users
changed to use the usual add/remove/fire pattern, and the built-in
ChangeListener interface. committed as r1543.

--elliott
>  activityMonitor.patch
> 5KViewDownload

Matt Hillsdon

unread,
Jun 8, 2009, 3:46:30 AM6/8/09
to terminat...@googlegroups.com
Martin Dorey wrote:
> The use of == false rather than ! will have been deliberate, to avoid
> the ! getting missed next to the parenthesis.
>
> - if (pane.isShowing() == false) {
> + if (!terminalPane.isShowing()) {
>

I'd wondered that when removing the line but by the time I came to add
it back in it had slipped my mind.

> Being a C++ programmer, I was a bit surprised by the name here, though
> the idiom, of a default listener that does nothing, was familiar:
>
> + ActivityMonitor NULL = new ActivityMonitor() {
>
> The first extant example of that idiom that I found was NoOpRunnable.
> Of course, that has be defined outside the interface that we don't own.
> I see we have a NoOpAction and a NoOpIndenter too. Some precedent
> there, then.

Yeah, I can see how that would look weird from a C/C++ POV. I think
it's pretty common in Java. Anyway, not needed with Elliott's switch to
using ChangeListener.

Thanks,

Matt.

Matt Hillsdon

unread,
Jun 8, 2009, 3:54:23 AM6/8/09
to terminat...@googlegroups.com
Elliott Hughes wrote:
> changed to use the usual add/remove/fire pattern, and the built-in
> ChangeListener interface. committed as r1543.
>

Thanks, that works great.

Matt.

Reply all
Reply to author
Forward
0 new messages