This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 82647 - Support programmatically scrolling output window to a line with an output listener
Summary: Support programmatically scrolling output window to a line with an output lis...
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Output Window (show other bugs)
Version: 6.x
Hardware: All All
: P1 blocker (vote)
Assignee: t_h
URL:
Keywords: API
Depends on:
Blocks:
 
Reported: 2006-08-14 20:49 UTC by _ tboudreau
Modified: 2009-03-09 15:07 UTC (History)
5 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments
Zip of openide-io with scrolling API added (46.59 KB, application/octet-stream)
2006-08-14 20:50 UTC, _ tboudreau
Details
Patch to core/output2 to implement a handler for calls to OutputScroller.requestScroll() w/ tests (13.51 KB, patch)
2006-08-14 20:51 UTC, _ tboudreau
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description _ tboudreau 2006-08-14 20:49:20 UTC
Along with separate output components, to really do the JUnit window well, it
should be possible to use an output window instance as the detail part of a
master-detail view.

To do that requires the ability to programmatically change where the output
window is scrolled to.

The patch I will attach adds an API for this to openide/io (I even used Jarda's
inscrutable "accessor" pattern) and implementation to core/output2 + tests for
it.  What it does:

Adds an abstract class OutputScroller to the IO API.  This can be the parent
class for implementations of OutputListener.  It has one final method,
"requestScroll()".  When called, the associated output window will become the
selected tab, and scroll to the line associated with that listener into view,
placing the caret there.

I'm attaching a zip of openide/io rather than a patch, since I didn't want to
create a bunch of directories for in CVS for org.netbeans.modules.openide.io for
the private part of the API.
Comment 1 _ tboudreau 2006-08-14 20:50:27 UTC
Created attachment 32891 [details]
Zip of openide-io with scrolling API added
Comment 2 _ tboudreau 2006-08-14 20:51:43 UTC
Created attachment 32892 [details]
Patch to core/output2 to implement a handler for calls to OutputScroller.requestScroll() w/ tests
Comment 3 Jesse Glick 2006-08-16 18:01:15 UTC
Tim get a newer dev build so you don't make core/output2/nbproject/project.xml
invalid acc. to /3 schema when editing proj props from GUI.


I find the proposed API rather weird. IMHO we should not be offering abstract
parent classes for an existing interface to add functionality that cannot be
accessed some other way. What if you wanted to add another such API? You would
be screwed.

Perhaps we should instead add methods to OutputWriter:

public int currentLine() throws UnsupportedOperationException;
public void scrollToLine(int line) throws UnsupportedOperationException;

You could then keep track of what line # corresponded to a given print, and
later scroll to it. This seems like a much clearer API to me. It would also work
just as well for non-hyperlinked output.

The impl would also be simpler, I guess, as you would just need to impl these
methods in OW to throw an exception (for compat w/ e.g. core/output), and add an
impl of the methods to NbWriter.
Comment 4 ivan 2006-08-16 19:19:24 UTC
Jesse,
output line numbers become meaningless if you add line wrapping and history.
I understand the current output module provides the illusionof infinite
storage, but other providers might choose to tradeoff that feature
for others. You shouldn't preclude that.
Comment 5 Jesse Glick 2006-08-16 19:45:32 UTC
Re. line numbers vs. alternate OW impls: all I proposed was the two methods you
see, which can uniquely identify a given printed line. If an OW has discarded
lines before a certain point, then it can just ignore requests to scroll to
older lines.

And line wrapping in the display is irrelevant; the int's in the API refer to
logical lines as counted by the impl. In fact you can name them "markers"
instead of "lines" and use some opaque data type if you prefer; there is no
important correspondence between these numbers and actual line numbers. The same
API usage should work fine regardless of impl:

OutputWriter ow = ...;
...
ow.println("foo", new Hyperlink(...));
int fooPrintedHere = ow.currentLine();
...
someJButton.addActionListener(...
  ow.scrollToLine(fooPrintedHere);
});

Similar API without reference to "lines":

OutputWriter ow = ...;
...
ow.println("foo", new Hyperlink(...));
OutputPosition fooPrintedHere = ow.currentPosition();
...
someJButton.addActionListener(...
  fooPrintedHere.show();
});

where

public interface OutputPosition {
  void show();
}
Comment 6 _ tboudreau 2006-08-17 07:49:00 UTC
> public int currentLine() throws UnsupportedOperationException;
> public void scrollToLine(int line) throws UnsupportedOperationException;

Actually I'd prefer to do that too;  I was trying to do the most minimally
invasive thing possible with respect to the current API and implementation. 
Easy enough to do with some caveats:

currentLine() - This is meaningful even in the case of wrapping.  Certainly
output2 does wrapping, but lines are quite meaningful even when wrapping (the OW
keeps an int : int map of lines to logical lines - so, if this looks like:
23 : 5
25 : 10
and I want to know where to draw line 26, I only have to do a binary search for
the nearest key int to the line I'm painting, and its associated value will be
the cumulative number of wrapped lines - so current line + that == logical line,
and you only use memory to store data about lines that are wrapped.

That digression aside, currentLine() will need to be documented as possibly
being inaccurate - there's no guarantee that another thread won't change its
value between the time an interesting write is detected and the call to
currentLine() - so it's value is only guaranteed to be valid if only one thread
is writing to the stream and currentLine() is called from that thread.  

If you can live with that, fine.  The one advantage to passing the arg w/ the
println is that it's guaranteed to always be accurate.

The alternative would be to return a value akin to a Swing text Position object
from the write call, which can subsequently be passed.  Or provide an alternate
write method that returns an int - actually that's clearer.

Any preference?

I think the weirdness in the way I did it here is that the argument is an
OutputListener - ideally I'd want some more general object (who knows what you
would call it though), which can express decoration of the line, possibly react
to clicks, etc.
Comment 7 Milos Kleint 2006-08-17 08:07:03 UTC
currentLine() should in my undestanding mean that the OW gets back to the
"sticky end" mode and keeps scrolling. No reason for it to be inaccurate. Maybe
a different name shall be picked to avoid the double meaning?
Comment 8 Antonin Nebuzelsky 2006-08-17 10:43:56 UTC
Is this really meant to 5.5? (version is set to 5.5)

Who requested this enhancement in 5.5?

Note that all modules except for J2EE/Web are in high-resistence mode. We should
not be doing enhancements to 5.5 just for fun.
Comment 9 _ tboudreau 2006-08-17 15:46:26 UTC
Not 5.5.  I didn't know we were using "Dev" to refer to the trunk - we always
avoided things like Dev and "Current" when I was on the core team.

Re Milos' comments:  I can see the place for a method that will programmatically
reset sticky-caret behavior in the output window (although I don't quite see the
need - how should code that's writing to the output window know that the user
wants it to auto-scroll again?  Getting this wrong could be a cure that's worse
than the disease).

My understanding of Jesse's request for "currentLine()" was a method that would
return an int identifying the current line - i.e. write an interesting line to
the output window and then save that int and use it later to tell the OW to
scroll to it.  Jesse, if that's not what you meant, let me know.  Probably
"getCurrentLine()" would be preferable.
Comment 10 Jesse Glick 2006-08-17 17:16:44 UTC
I am tending toward a preference for the OutputPosition style. It makes it
clearer that the position is just an abstract object with no relevant
association with line numbers etc. Also, you can put the scroll method on that
object, rather than on the OutputWriter, which is better OO style.

Re. race condition if another thread prints to the stream before you get the
position - not an issue. Multithreaded printing is not usual (possible but very
rare in Ant, probably never occurs in other clients). And scrolling to the wrong
text position is hardly a big deal.

Adding a new method like

public OutputPosition printlnAt(String, OutputListener/*|null*/);

is possible but seems a bit clumsy to me. (You cannot change the return type of
an existing method from void without breaking binary compatibility.)
Comment 11 ivan 2006-08-17 19:12:36 UTC
Re lines.
Heh, heh ... take a look at the main comment for
org.netbeans.lib.terminalemulator.Term
and its discussion of absolute coordinates. With an
unbounded buffer an 'int' can wrap rather quickly.
While both output2 and Term use line numbers some _other_
implementation might not.

Comment 12 Milos Kleint 2006-08-30 07:36:26 UTC
there's some other related output window api enhancements like #60862 and #58633
which all try to get around the fact that current IO api is not easily extendible. 

I suggest we pull all of them from review and create a new api that would handle
the new usecases in a consistent manner.
Comment 13 _ rkubacki 2007-01-04 20:08:31 UTC
Is this review finished? Accepted/rejected?
Comment 14 _ tboudreau 2007-03-28 04:11:20 UTC
Silence == assent?
Comment 15 Milos Kleint 2007-03-28 07:19:58 UTC
AFAIK
silence == "we agreed on pulling 3 separate, non-compatible API changes related
to OW and get complete, consistent rewrite later".

not sure why it's still assigned to apireviews -> reassigning
Comment 16 Antonin Nebuzelsky 2008-02-07 15:52:56 UTC
Reassigning to new module owner Tomas Holy.
Comment 17 t_h 2009-03-09 15:07:25 UTC
Fixed in core-main #b846993e69a9