Many people subclass DataEditorSupport to provide editing ability for data
objects. The great majority of them wish to provide an editor cookie only for
the primary file and with standard semantics for open, modify, save, undo,
close, reload, etc. The old EditorSupport made this easy; DataEditorSupport
makes it very complicated (a subclass and two inner classes needed), and even
the template in apisupport has gotten revised several times in response to
various problems that were found in the current recommendation.
Suggest a simpler support that most people could use. Ideally would require no
subclassing at all for common cases. See #14706 for details.
For a future API release.
From issue 14706: consider passing CookieSet into the Env constructor,
so it will add/remove SaveCookie automatically...
Set target milestone to TBD
reassigne to David K., new owner of editor
Why not add:
public static DataEditorSupport.createSimple (DataObject obj,
the code for this method is already written as part of the API support
generator I guess.
I like Yarda's suggestion. Simple, convenient.
You can call setMIMEType as a public method, I guess, so there is no
need to support that explicitly.
Created attachment 14777 [details]
Patch with implementation and basic test
With the attached patch the future of the issue could be reevaluated I
IMHO DES.create could return CloneableEditorSupport - no particular
reason to know whether the returned object actually instanceof DES. Right?
Implementing both EditCookie and OpenCookie may be a problem.
Sometimes you want to have a separate OpenCookie, e.g., and having the
editor support impl OC makes it not work reliably. Suggest a boolean
flag which lets you impl either EditCookie or OpenCookie (not both).
Should probably document that the cookie set is to be used only for
SaveCookie, unless you plan on using it for something else in the future?
Otherwise looks good to me.
I'll change the patch according to Jesse's comments and implement it
as soon as 4.1 is branched.
Any chance for promo E? I'm writing a tutorial that describes how to use
DataEditorSupport, and I'd rather have a fix than have to apologize for it. Please give it a
better name than SimpleES when it really goes into API.
Meta comment: Could we please standardize on either getLookup() for everything, or
getCookie() for everything and screw lookup, or some other reasonable semantics (making
everybody type someObject.getLookup().lookup() makes clear the wonders of lookup at
the expense of everybody having to type way too many characters) and dispense with
CookieSets, etc.? It would be much simpler if there were one thing to tell people about -
this is a mess. The fact that when talking about Nodes I should tell everyone to use
Node.getLookup().lookup(), not Node.getCookie(), but when it comes to DataObjects I have
to tell everyone to override <code>getCookie()</code> is not doing us any favors. We
need better consistency if we want people to understand this stuff.
Created attachment 20620 [details]
Ready to be integrated patch
Tim really needs this for 4.1 to greatly simplify his module writing
tutorial. It's no risk fix as no existing code is changed, just new
method added that will only be used by newly written modules or those
that decide to migrate to it (probably not in 4.1).
As the diff suggests, I plan to integrate this on Mar9,2005.
The name SimpleES is not a problem IMO. It is just a package private
implementation that is not visible. The only interface is factory method.
Maybe clearer documentation about CookieSet parameter (what should be
passed, how the saving support works?).
No chance we can make it a Lookup, not a CookieSet? Would be really nice to unify this stuff
- explaining lookup + cookies when they're the same thing just confuses people.
I am ready to integrate the diff I created on Mar 2. If anybody of you thinks
that this shall not be integrated, you can either turn this into regular review
or create a bug that would block this describing what is the request you want me
"#17081: Factory method to simplify creation of simple CloneableEditorSupport"
Checking in openide/loaders/manifest.mf;
/cvs/openide/loaders/manifest.mf,v <-- manifest.mf
new revision: 1.19; previous revision: 1.18
Checking in openide/loaders/api/apichanges.xml;
/cvs/openide/loaders/api/apichanges.xml,v <-- apichanges.xml
new revision: 1.13; previous revision: 1.12
Checking in openide/loaders/src/org/openide/text/DataEditorSupport.java;
new revision: 1.20; previous revision: 1.19
RCS file: /cvs/openide/loaders/src/org/openide/text/SimpleES.java,v
Checking in openide/loaders/src/org/openide/text/SimpleES.java;
/cvs/openide/loaders/src/org/openide/text/SimpleES.java,v <-- SimpleES.java
initial revision: 1.1
Checking in openide/loaders/test/unit/src/org/openide/text/SimpleDESTest.java;
initial revision: 1.1
Belated comment: there seems to be no way to override the content type of
text/plain? Isn't this a serious deficit? (Consider that DES replaced the much
easier to use EditorSupport, which did have an easy way to set the MIME type of
the editor.) I would suggest permitting the caller to pass in a MIME type as a
parameter to the factory.