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: | getIO() not flexible enough in view of multiple impl's. | ||
---|---|---|---|
Product: | platform | Reporter: | ivan <ivan> |
Component: | Output Window | Assignee: | Jaroslav Havlin <jhavlin> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | apireviews |
Priority: | P3 | Keywords: | API, API_REVIEW_FAST |
Version: | 6.x | ||
Hardware: | All | ||
OS: | All | ||
Issue Type: | DEFECT | Exception Reporter: | |
Attachments: |
Proposed Patch
Proposed Patch Proposed Patch Fix to make TerminalIOProvider to compile Proposed Patch |
Description
ivan
2010-03-23 04:02:45 UTC
Created attachment 121222 [details]
Proposed Patch
Proposed Patch
- added method IOProvider.getIO(String, boolean, Action[], IOContainer)
- updated NbIOProvider, added NbIOProviderTest
Please review. PH01: I would suggest annotating the params with annotations from api.annotations.common. Created attachment 121225 [details]
Proposed Patch
PH01 - Arguments and return value of the new method were annotated. Thank you.
(In reply to comment #1) > Created attachment 121222 [details] > Proposed Patch > > Proposed Patch > - added method IOProvider.getIO(String, boolean, Action[], IOContainer) > - updated NbIOProvider, added NbIOProviderTest I see that the private method had become public, but I see some other changes that are not described in this IZ. It will take me a bit to give feedback on this, I'll have to go home and resync all my NB-related stuff and double-check with the trerminal io provider code. I"ll try and do that tonight. (In reply to comment #5) > I see that the private method had become public, but I see some other > changes that are not described in this IZ. The original mapping [IO name] -> [IO object] has been replaced with mapping [IO container, IO name] -> [IO object]. This mapping is used for reusing of IO objects. > It will take me a bit to give feedback on this, I'll have to go > home and resync all my NB-related stuff and double-check with > the trerminal io provider code. I"ll try and do that tonight. Please, have you tried the patch? Any comments/ideas/problems? Thank you. (In reply to comment #6) > > It will take me a bit to give feedback on this, I'll have to go > > home and resync all my NB-related stuff and double-check with > > the trerminal io provider code. I"ll try and do that tonight. > Please, have you tried the patch? Any comments/ideas/problems? Thank you. Sorry, not yet. Initially I had what I thought were hg problems. You might've seen my nbdev posts. Now I'm having trouble building the IDE. (In reply to comment #7) > Now I'm having trouble building the IDE. Please, any news? Created attachment 122306 [details]
Proposed Patch
Updated JavaDoc for the new method. Added note that returned InputOutput should be closed when not needed.
Apologies for taking so long. Paging in NB development stuff after 1.5 years of inactivity wasn't easy. IS01: Making IOProvider.getIO(<all args>) public will break the build because TerminalIOProvider also has a private method getIO() with an identical signature. But given that there are just two implementations of the IOProvider SPI you can just adjust TerminalIOProvider. It's trivial. See appended diff (sorry I'm not very handy with patch stuff). After adjusting TerminalIOProvider I verified that the <all args> implementation of TerminalIOProvider.getIO() works as expected. See below for details. IS02: The new semantics of having each container have it's own "name" space makes a lot of sense. I've filed IZ 215913 to ensure that TerminalIOProvider provides identical semantics although I'm not sure when I'll get to it. module 'terminal' which is part of the cnd cluster has a second, after output2, implementation of IOProvider. You can run it's junit test to witness the motivation for this bug. The junit test runs in batches where a standalone TC is put up, exercised and then brought down. Pay attention to the first batch. Everything will happen in one TerminalIOProvider provided TC except that another standalone TC will be created with the message "Hello to io2" in it. The relevant junit code is in T1_Close_Test.testStreamClose(). Search for 182538. A bit above it is a line io2 = ioProvider.getIO("io1", false); This is the line that causes io2 to show up in the default as opposed to the given ioContainer. If you replace this line with io2 = ioProvider.getIO("io1", false, null, ioContainer); then the standalone TC with "Hello to io2" won't show up. I verified this in my workdir. By the way, thanks for fixing this. If you're going to be working more with "output" ... - There are a coupla readmes that, while written for Terminal, might be useful in the context of output2: terminal/README.attributes terminal/README.close_semantics # especially this one - The terminal module extends the IO interface using the standard static method + Lookup technique with classes like IOConnect, IOEmulation, IOREsizable, IOTErm, IOVisibility, etc. These, by rights should be moved to openide.io and if possible implemented in output2. Created attachment 122330 [details]
Fix to make TerminalIOProvider to compile
Created attachment 122351 [details] Proposed Patch The two patches were merged. Thank you very much for your great help, Ivan. IS01 - TerminalIOProvider adjusted. > If you're going to be working more with "output" ... I've created separate bug 215940 (enhancement) with your suggestions. Thanks again for your effort, explanation and ideas! If no objections are filed, I'll integrate on Friday. Integrated into 'main-golden', will be available in build *201207280002* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-golden/rev/6cb43817877c User: Jaroslav Havlin <jhavlin@netbeans.org> Log: #182538: getIO() not flexible enough in view of multiple impl's Integrated into 'main-golden', will be available in build *201207290002* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main-golden/rev/334fea338aa7 User: ivan@netbeans.org Log: nobugid - adjust Terminal unit tests now that BZ #182538 is fixed |