The existing IOProvider.getIO() functions, in union,
take the following parameters:
Yet none cover all the possible parameters.
In view of multiple possible implementations of IOProviders
and IOContainer.Providers this lack of flexibility can be awkward.
For example, consider, ...
IOProvider myIOProvider = IOProvider.get("Terminal");
IOContainer myContainer = (new MyContainer()).ioContainer();
InputOutput io1 = myIOProvider.getIO("io1", null, myContainer);
// at this point io1 is not reusable because it's getOut() hasn't been closed
boolean reuse = true;
InputOutput io2 = myIOProvider.getIO("io1", reuse);
Even though 'reuse' is true this call to getIO() will not
find an available "io1" and will have to create a new one.
The question is, which container will it go into?
It might pick IOCOntainer.default(), but that's probably not
what the caller would expect. They would expect that io2 would
appear in the same container as io1.
Perhaps the "reuse" variation of getIO() should use the default
container associated with the IOProvider being used, i.e. MyContainer?
That might address the above example, but will not work in general.
The problem is that MyContainer might not be associated with a
TopComponent so the first time IOPRovider has to get one ... it can't.
Perhaps IOProvider should remember that "io1" was in myContainer?
But what if myContainer is "gone"?
The natural solution seems to be a variation ongetIO() that takes
all 4 parameters. That way someone can use
InputOutput io2 = myIOProvider.getIO("io1", null, reuse, myContainer);
Ironically all three getIO()'s call a common getIO() like the above which
is unfortunately private.
So perhaps the simplest solution is to make that common getIO() public?
Created attachment 121222 [details]
- added method IOProvider.getIO(String, boolean, Action, IOContainer)
- updated NbIOProvider, added NbIOProviderTest
PH01: I would suggest annotating the params with annotations from api.annotations.common.
Created attachment 121225 [details]
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]
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
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.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]
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)
User: Jaroslav Havlin <firstname.lastname@example.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)
Log: nobugid - adjust Terminal unit tests now that BZ #182538 is fixed