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.
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.
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