Bug 91665

Created attachment 36966 [details]
Proposed patch in core to include Lookup.getDefault().lookupResult(DataLoader.class) in the pool

Before integration I'll add a note to openide/loaders/apichanges.xml

Could you please use diff -u next time? Meaning, currently, do a diff from the
command line, not the IDE.


Need docs changes. (**/apichanges.xml, **/doc-files/*)


apisupport's File Type wizard should be modified to generate M-I/s registration
rather than manifest registration, for 6.0+ platforms.


Do you plan to change the registration of existing loaders? Or deprecate
manifest-based registration?


Can we deprecate serialization to loaders.ser? Or is that part of a separate change?

Is this still active?

Well, yes, someone could implement this. I do not thing I handle that for 6.1

Here is completely new attempt how to solve the problem with registration. Let's also tackle few performance aspects 
and make the whole infrastructure more scalable. Here is the proposal:

http://wiki.netbeans.org/FitnessAgainstForgetfulness

I'd like this to be review properly. So please comment on the page or here or on the mailing list. And let's have a 
conf call to resolve the outstanding issue next week.

> > Dne Friday 02 May 2008 21:03:56 Ivan Soleimanipour napsal(a):
> > > We'll end up eliminating these:
> The tags.

These tags will no longer be needed. Instead we will use regular positioning of files on layer: <file 
name="x.instance> <attr name="position" intvalue="9"/></file>, etc.

> But conflicts/mistakes in position ordering of loaders is
> going to have more impact than UI items.
> The ui item ordering with the "/" syntax was poorly conceived,
> but loader dependency is a bit cleaner ... more DAG-ish why
> not keep it?

I see. You are proposing to use relative ordering for loaders. Fine with me, it still works, we will just need to 
convince Jesse that it is the right thing.


Ivan sent me his comment via email, so I guess I need to formalize it somehow:

I01 Use relative ordering for positioning loaders

I am in favor of this effort. I guess the idea is that the loader pool would do something to the effect of

String mimeType = fo.getMIMEType();
for (DataLoader dl : Lookups.proxy(Lookups.forPath("Loaders/" + mimeType + "/Factories").lookupAll(DataLoader.class),
Lookup.getDefault()) {
  // check dl on fo, use it if recognized
}

[JG01 vs. I01] Use numeric positioning as with everything else. It will be a relief to get away from the evil relative
positioning of Install-Before/After. Never worked very reliably.

[JG02, though perhaps not appropriate here?] The GUI in
http://ui.netbeans.org/docs/ui/OptionMenuDialogues/#FileTypesredesign looks a little dubious. Not clear how this will
map to declarative MIME resolvers. E.g. for Ant files, the default resolver uses a mixture of various techniques; no GUI
will really capture this adequately or allow it to be meaningfully edited.

[JG03] Would be nice if DataLoader did not extend SharedClassObject. Then we could have a factory to create loaders
whereby you would just need to specify the DataObject class (a constructor taking a FileObject parameter would be
called). But not very important.

[JG04] Ensure that so long as MIMEResolver's are correctly registered, DataObject recognition works identically inside
unit tests as in the full platform.

Here is answer to some of your questions:
http://wiki.netbeans.org/Diff.jsp?page=FitnessAgainstForgetfulness&r1=13&r2=11

Ondřeji, please attach the new UI spec, so JG02 is addressed. Thanks.

The review will be on Thursday, May 22, 2008, at 17.30CET, 7.30am pacific time, I'll send dial in note to everyone 
interested before the meeting.

New UI spec linked from wiki.

I do not understand the "Open with" dialog, it will allow me to open unrecognized file as some mimetype but what if I
have two editors(Loaders) for the same mimetype? Is that supported? I will have two image viewers and then I want easy
way to switch between them. 

maybe I'm not getting something...

6c6fea40955e

http://hg.netbeans.org/core-main/rev/6c6fea40955e
http://hg.netbeans.org/main/rev/6c6fea40955e

deniss: You would have to go to option panel to do this, which should be still way easier than current solution, AFAIK.
Can you clarify the case when you need this?

it's ok, I understand

Since the loaders now implicitly specify the MIME types they handle, I wanted to remove the ExtensionList initialization
from SQLDataLoader:

    ExtensionList extensions = new ExtensionList();
    extensions.addMimeType("text/x-sql");
    setExtensions(extensions);

However, apparently this code is still needed: UniFileLoader.findPrimaryFile() still checks ExtensionList. So then I
tried to remove SQLDataLoader entirely and replace it with an impl of DataObject.Factory. But I couldn't do that either,
since I didn't see an easy way to assign the layer-specified node actions ("Loaders/text/x-sql/Actions") to the SQL
nodes. At least not as easy as overriding DataLoader.actionsContext(). It seems such support is needed if we want to
attract people toward DataObject.Factory and away from DataLoader.

To abadea: there are several options (I don't have a strong opinion as to which is preferable):

1. Fix UniFileLoader to recognize all files in case no extensions have been registered. E.g. change

return getExtensions().isRegistered(fo) ? fo : null;

to

ExtensionList l = (ExtensionList) getProperty(PROP_EXTENSIONS);
return l == null || l.isRegistered(fo) ? fo : null;

2. Change your loader to say

protected @Override FileObject findPrimaryFile(FileObject fo) {
  return fo;
}

3. Use DataObject.Factory and in your DataNode use Lookups.forPath to determine the action list. You need to look for
both Action and JSeparator instances (mapping the latter to null); probably this common idiom (repeated in a number of
places in NB) should be factored into a utility method somewhere.


BTW [JG05] The usefulness of the new DataObject.Factory is questionable given that the DataObject constructor requires
you to pass in a DataLoader! DataLoaderInLayerTest cheats by making SimpleFactory actually delegate to a DataLoader,
which seems to defeat the purpose. When I said during the review "check that DataObject.Factory actually works", I meant
actually try to use it in a realistic module and see if it works as expected (i.e. no DataLoader in sight), not just
write some unit test.

Created attachment 62332 [details]
Changes to support loaderless creation of DataObjects

I see the previous patch as an answer to JG05. Unless there are big objections, I'll use these API changes to create 
the new wizard for "File Type". It will not generate DataLoader and its BeanInfo anymore. It is possible not to 
generate a subclass of DataNode as well, I do not if I should generate it or not.

Commenting on the new proposed API: where is MIMEFactory.java? Seems missing from patch.


[JG06] DLP.factory Javadoc messed up. Missing @param image. SFS.lI values should be URLs, e.g.
urlvalue="nbresloc:/.../foo.png". "&attr" should be "<attr".


[JG07] What is the image param to DLP.factory for? If it's only for the DataNode icon, I think it should be deleted.

Created attachment 62481 [details]
Another improved patch: factory method and new file type wizard that uses it

Looks like you ignored the comment about SFS.lI in JG06.