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 245014

Summary: Provide base I/O APIs
Product: platform Reporter: Jaroslav Havlin <jhavlin>
Component: Output WindowAssignee: Jaroslav Havlin <jhavlin>
Status: RESOLVED INVALID    
Severity: normal CC: apireviews, mentlicher, phejl, tstupka
Priority: P3 Keywords: API, API_REVIEW
Version: 8.0.1   
Hardware: All   
OS: All   
Issue Type: ENHANCEMENT Exception Reporter:
Bug Depends on:    
Bug Blocks: 247200    
Attachments: Patch for branch server_split
Updated patch for branch server_split
Example Usage of the base API
Updated patch for branch server_split
Updated patch for branch server_split
Example Usage of the base API
Updated patch for branch server_split

Description Jaroslav Havlin 2014-06-12 12:51:15 UTC

    
Comment 1 Jaroslav Havlin 2014-06-12 13:05:12 UTC
Created attachment 147651 [details]
Patch for branch server_split
Comment 2 Jaroslav Havlin 2014-06-12 13:17:49 UTC
Please review new module Base I/O APIs.

Goals:
 - provide a lightweight API for Output Window
 - don't break existing modules
 - make implementation in core.output2 support both APIs
 - make the API extendable, so that implementations can provide additional features
 - remove deprecated methods, add methods missing in the current API

The API is very similar to (and actually extracted from) the original API module I/O APIs (openide.io). The difference is in dependencies. The new base API depends only on Base Utilities API and Lookup API, and no Swing- or AWT-related classes are imported, so the new API can be used in lightweight JDK profiles.

The original API is untouched, so existing modules or plugins will continue to work. The main implementation in module Output Window (core.output2) can be accessed via both, the new and the original, APIs.

Comparison: The original vs the base API:

InputOutputBase

 - (common with InputOutput)
 - getOut (but returns PrintWriter instead of OutputWriter)
 - getIn
 - getErr (but returns PrintWriter instead of OutputWriter)
 - closeInputOutput
 - isClosed
 - select

 - (added)
 - reset
 - getExtendableOut
 - getExtendableErr
 - getLookup

 - (removed from InputOutput)
 - setOutputVisible
 - setErrVisible
 - setInputVisible
 - isErrSeparated
 - setErrSeparated
 - isFocusTaken
 - setFocusTaken
 - nullReader
 - nullWriter
 - flushReader 

IOProviderBase
 - static get(String)
 - static getDefault()
 - getName()
 - getIO(String, boolean)

 - (removed)
 - getIO variants with parameter of type Action

OutputEventBase
 - the same as OutputEvent

OutputListenerBase
 - The same as OutputListener

ExtendableWriter (new interface)
 - (Complement to PrintWriter that can be used for printing lines with output listeners or output tags)
 - (These methods are in class OutputWriter in the original API)
 - println(String, Lookup, boolean)
 - println(String, Lookup)

OutputTagBase (new interface)
 - getName
 - getAttribute
 - getAttributeNames


Example usage:

 final InputOutputBase io = IOProviderBase.getDefault().getIO("Test", true);
 io.getOut().println("Standard line");
 if (io.getExtendableOut().getExtendingTypes().contains(OutputListenerBase.class)) {
     OutputListenerBase ol = createOutputListener();
     Lookup lkp = Lookups.fixed(ol);
     io.getExtendableOut().println("Line with a link", lkp, true);
 }


Possibilities to consider:
 - remove method select() from InputOutputBase
 - remove methods outputLineSelected and outputLineCleared from OutputListenerBase
Comment 3 Petr Hejl 2014-06-16 13:15:33 UTC
Looks great!

[PH01] Is it really necessary to know the set of extending types? Perhaps it would be simpler for the client to use getExtendableOut().isSupported(OutputListenerBase.class) rather than getExtendableOut().getExtendingTypes().contains(OutputListenerBase.class).
[PH02] If you think it is not really required I would remove at least outputLineSelected - I have never had a use case for it. Same for outputLineCleared.
[PH03] Perhaps this is an opportunity for the InputOutputBase to be Closeable and AutoCloseable.
[PH04] Just a minor one. The lookup parameter in println seems to be just a magic bag, where just "Object... parameters" could be used. At least if you do not suppose some lookup composition and proxying. Though even lookup is ok for me.

I don't like Base suffix much as I'm used to *Base class is usually default or abstract implementation of an interface serving as base for the real impl. But I can deal with it ;)
Comment 4 Jaroslav Havlin 2014-06-18 08:34:03 UTC
Created attachment 147711 [details]
Updated patch for branch server_split

> [PH01] Is it really necessary to know the set of extending types? Perhaps it
> would be simpler for the client to use
> getExtendableOut().isSupported(OutputListenerBase.class) [...]
I agree, good idea. I've separated support for hyperlinks into a new extension type (see below). The code would now be: BaseIOHyperlink.isSupported(io, OutputListenerBase.class).


> [PH02] If you think it is not really required I would remove at least
> outputLineSelected - I have never had a use case for it. Same for
> outputLineCleared.
OK, I've removed both methods and also BaseInputOutput.select(), BaseIOSelect extension type can be used instead.

> [PH03] Perhaps this is an opportunity for the InputOutputBase to be
> Closeable and AutoCloseable.
The close() method on BaseInputOutput can be a shortcut for io.getOut().close(), and can be convenient to use in try-with-resources block.
Is it what you meant? (It's semantics is different from io.closeInputOutput()).

> [PH04] Just a minor one. The lookup parameter in println seems to be just a
> magic bag, where just "Object... parameters" could be used. [...]
Sure, it will be more comfortable for the clients. Having a marker interface BaseIOLinkInfo would be more descriptive than simple Object type.

> I don't like Base suffix much as I'm used to *Base class is usually default
> or abstract implementation of an interface serving as base for the real
> impl. But I can deal with it ;)
Good point, I've refactored the classes, the "Base" is now the prefix.

Thank you very much for your comments, Petr.

All the extension classes from the original I/O APIs now have it's counterparts in the base APIs (with dependencies on AWT and Swing classes removed). For color values, there is a new class BaseColor, which currently supports only the RGB model. For hyperlink support, there's a new extension type BaseIOHyperlink.


Pending:
Support for I/O objects with actions, e.g. alternative for IOProvider.getIO(String, Action[]) in the original I/O APIs.
Comment 5 Jaroslav Havlin 2014-06-18 08:36:43 UTC
Created attachment 147712 [details]
Example Usage of the base API
Comment 6 Jaroslav Havlin 2014-06-18 13:09:48 UTC
Created attachment 147714 [details]
Updated patch for branch server_split

> Pending:
> Support for I/O objects with actions, e.g. alternative for
> IOProvider.getIO(String, Action[]) in the original I/O APIs.
New parameter in BaseIOProvider.getIO(String, boolean, EventListener...) that accepts array of action objects, and new accompanying method BaseIOProvider.isActionTypeSupported(Class<? extends EventListener>).

NbIOProviderBase logs a warning when an unsupported action is passed to the getIO method.
Comment 7 Petr Hejl 2014-06-23 13:22:49 UTC
(In reply to Jaroslav Havlin from comment #4)
> > [PH03] Perhaps this is an opportunity for the InputOutputBase to be
> > Closeable and AutoCloseable.
> The close() method on BaseInputOutput can be a shortcut for
> io.getOut().close(), and can be convenient to use in try-with-resources
> block.
> Is it what you meant? (It's semantics is different from
> io.closeInputOutput()).

Actually I meant the io.closeInputOutput(). Now I see it has a different semantic - in that case I would not make it a shortcut as it introduces second close method. Sorry for the confusion.

Otherwise the API is ok for me.
Comment 8 Jaroslav Havlin 2014-06-24 08:15:40 UTC
Created attachment 147751 [details]
Updated patch for branch server_split

> [PH03] Actually I meant the io.closeInputOutput(). Now I see it has a different
> semantic - in that case I would not make it a shortcut as it introduces second
> close method. Sorry for the confusion.
OK, I've removed the close() method from BaseInputOutput, which doesn't implement Closeable and AutoCloseable any more.

Thank you, Petr.
Comment 9 Jaroslav Havlin 2014-06-24 10:49:48 UTC
Created attachment 147759 [details]
Example Usage of the base API
Comment 10 Jaroslav Havlin 2014-09-17 15:19:11 UTC
Created attachment 149307 [details]
Updated patch for branch server_split

BaseIOProvider.getIO now takes also parameter "context" (Lookup with various info required by implementations).
Comment 11 Jaroslav Havlin 2014-09-18 15:24:38 UTC
http://hg.netbeans.org/jet-main/rev/55c3536c7148
Patch integrated into server_split branch.
Comment 12 Jaroslav Havlin 2014-09-25 07:16:15 UTC
http://hg.netbeans.org/jet-main/rev/6eeb572034db
I've removed the base API from server_split branch, it needs redesigning.
The API review has been moved to bug 247404, so I'm closing this issue.
Comment 13 Quality Engineering 2014-10-18 05:12:07 UTC
Integrated into 'main-silver', will be available in build *201410180001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)

Changeset: http://hg.netbeans.org/main-silver/rev/55c3536c7148
User: Jaroslav Havlin <jhavlin@netbeans.org>
Log: #245014: Provide base I/O APIs