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.
Summary: | Support programmatically scrolling output window to a line with an output listener | ||
---|---|---|---|
Product: | platform | Reporter: | _ tboudreau <tboudreau> |
Component: | Output Window | Assignee: | t_h <t_h> |
Status: | RESOLVED FIXED | ||
Severity: | blocker | CC: | ivan, jtulach, mkleint, mpetras, tboudreau |
Priority: | P1 | Keywords: | API |
Version: | 6.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | ENHANCEMENT | Exception Reporter: | |
Attachments: |
Zip of openide-io with scrolling API added
Patch to core/output2 to implement a handler for calls to OutputScroller.requestScroll() w/ tests |
Description
_ tboudreau
2006-08-14 20:49:20 UTC
Created attachment 32891 [details]
Zip of openide-io with scrolling API added
Created attachment 32892 [details]
Patch to core/output2 to implement a handler for calls to OutputScroller.requestScroll() w/ tests
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. 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. 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(); } > 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.
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? 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. 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. 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.) 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. 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. Is this review finished? Accepted/rejected? Silence == assent? 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 Reassigning to new module owner Tomas Holy. Fixed in core-main #b846993e69a9 |