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 17081 - Suggest easier subclass of DataEditorSupport
Summary: Suggest easier subclass of DataEditorSupport
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Text (show other bugs)
Version: 3.x
Hardware: PC Linux
: P2 blocker (vote)
Assignee: Jaroslav Tulach
URL:
Keywords: API, API_REVIEW_FAST, ARCH
Depends on:
Blocks: 21748 14706
  Show dependency tree
 
Reported: 2001-10-29 11:57 UTC by Jesse Glick
Modified: 2008-12-22 21:03 UTC (History)
5 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments
Patch with implementation and basic test (11.87 KB, patch)
2004-05-10 07:44 UTC, Jaroslav Tulach
Details | Diff
Ready to be integrated patch (15.63 KB, patch)
2005-03-02 14:48 UTC, Jaroslav Tulach
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2001-10-29 11:57:17 UTC
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.
Comment 1 Jesse Glick 2001-10-29 11:57:54 UTC
For a future API release.
Comment 2 Jaroslav Tulach 2002-01-30 09:01:22 UTC
From issue 14706: consider passing CookieSet into the Env constructor,
so it will add/remove SaveCookie automatically...
Comment 3 Marek Grummich 2002-07-22 11:30:11 UTC
Set target milestone to TBD
Comment 4 Marek Grummich 2002-07-22 11:32:12 UTC
Set target milestone to TBD
Comment 5 Marian Mirilovic 2002-12-06 17:18:13 UTC
reassigne to David K., new owner of editor
Comment 6 Jaroslav Tulach 2004-04-23 17:43:59 UTC
Why not add:

public static DataEditorSupport.createSimple (DataObject obj,
CookieSet set);

the code for this method is already written as part of the API support
generator I guess.
Comment 7 Jesse Glick 2004-04-23 18:24:54 UTC
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.
Comment 8 Jaroslav Tulach 2004-05-10 07:44:36 UTC
Created attachment 14777 [details]
Patch with implementation and basic test
Comment 9 Jaroslav Tulach 2004-05-10 07:45:20 UTC
With the attached patch the future of the issue could be reevaluated I
guess.
Comment 10 Jesse Glick 2004-05-10 17:21:07 UTC
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.
Comment 11 Jaroslav Tulach 2005-02-24 08:54:16 UTC
I'll change the patch according to Jesse's comments and implement it
as soon as 4.1 is branched.
Comment 12 _ tboudreau 2005-03-01 09:41:19 UTC
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.
Comment 13 Jaroslav Tulach 2005-03-02 14:48:08 UTC
Created attachment 20620 [details]
Ready to be integrated patch
Comment 14 Jaroslav Tulach 2005-03-02 14:51:44 UTC
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.
Comment 15 _ rkubacki 2005-03-07 09:37:15 UTC
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?).
Comment 16 _ tboudreau 2005-03-07 10:24:14 UTC
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.
Comment 17 Jaroslav Tulach 2005-03-10 07:32:50 UTC
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
to implement.
Comment 18 Jaroslav Tulach 2005-03-11 18:37:55 UTC
"#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 
done 
Checking in openide/loaders/api/apichanges.xml; 
/cvs/openide/loaders/api/apichanges.xml,v  <--  apichanges.xml 
new revision: 1.13; previous revision: 1.12 
done 
Checking in openide/loaders/src/org/openide/text/DataEditorSupport.java; 
/cvs/openide/loaders/src/org/openide/text/DataEditorSupport.java,v  <--  
DataEditorSupport.java 
new revision: 1.20; previous revision: 1.19 
done 
RCS file: /cvs/openide/loaders/src/org/openide/text/SimpleES.java,v 
done 
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 
done 
RCS 
file: /cvs/openide/loaders/test/unit/src/org/openide/text/SimpleDESTest.java,v 
done 
Checking in openide/loaders/test/unit/src/org/openide/text/SimpleDESTest.java; 
/cvs/openide/loaders/test/unit/src/org/openide/text/SimpleDESTest.java,v  <--  
SimpleDESTest.java 
initial revision: 1.1 
 
Comment 19 Jesse Glick 2005-03-30 20:24:20 UTC
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.