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 137084 - GSF Inception Review
Summary: GSF Inception Review
Status: VERIFIED WONTFIX
Alias: None
Product: editor
Classification: Unclassified
Component: CSL (API & infrastructure) (show other bugs)
Version: 6.x
Hardware: All All
: P3 blocker (vote)
Assignee: Milutin Kristofic
URL:
Keywords: API_REVIEW
Depends on:
Blocks:
 
Reported: 2008-06-12 10:01 UTC by Jan Becicka
Modified: 2012-10-12 22:10 UTC (History)
20 users (show)

See Also:
Issue Type: TASK
Exception Reporter:


Attachments
Documentation (1.05 MB, application/x-compressed)
2008-06-12 10:03 UTC, Jan Becicka
Details
Inception review write up (6.54 KB, text/plain)
2008-06-24 18:23 UTC, Vitezslav Stejskal
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Becicka 2008-06-12 10:01:37 UTC
On behalf of Tor I'd like to ask you for inception review of GSF. I propose to do it on Tue 6/24 5pm CEST, 8am Pacific
time.
Comment 1 Jan Becicka 2008-06-12 10:03:02 UTC
Created attachment 62734 [details]
Documentation
Comment 2 Jan Becicka 2008-06-12 10:05:23 UTC
The main document, overview.html (shown in index.html) will introduce GSF from a high level and then have jump off
points into other documents.   While all the documents are linking each other, it might be a bit difficult to keep track
of everything, so here's a list of the documents if you want to visit them individually without following links:

overview.html - This is the main entry point, and is rolled into the javadoc as the overview document (e.g. shown when
you open the index.html).

getting-started.html - A document for people wanting to write GSF based languages, which mostly just orders everything
you need to know and points off into other documents.

mime-resolver.html - How to set up file recognition

registration.html - How you register GSF services

lexing.html, parsing.html   - Information about lexing and parsing with GSF

indexer.html - Information on how to do indexing (and querying)

classpath.html - How to integrate GSF language support into project types (for indexing, completion scope, etc.)

embedding.html - Information about how language embedding works

unit-testing.html - How to write unit tests for GSF services

gsf-tools.html - Some info on the diagnostic tools. 
Comment 3 Jaroslav Tulach 2008-06-17 08:02:41 UTC
Y01 At the time of release, the javadoc should have standard summary page, with "what is new" section and table of
exported APIs.
Y02 Adapt to new registration scheme for DataLoaders. Find a way for GsfDataLoader not to be registered for
content/unknown, potentially use DataObject.Factory for individual mime/type.
Y03 Provide an infrastructure for declarative registrations - no classes of any language loaded until their
editor/dataobject is really used
Y04 Accompany the API with wizard. Generate code skeleton as well as testing skeleton.
Y05 Show me code coverage
Comment 4 Jan Becicka 2008-06-17 09:47:37 UTC
JB01: org.netbeans.modules.gsf.api.annotations package should not be in gsf at all. It should be integrated into
platform...?

JB02: DefaultParseListener, DefaultParserFile, CancellableTask, ParseListener, Parser, ParserFile,
ParserResult.AstTreeNode and similar classes interfaces should be replaced by Parsing API
(http://wiki.netbeans.org/ParsingAPI)

JB03: Indexing functionality should be part of Parsing API
Comment 5 Jaroslav Tulach 2008-06-17 12:37:29 UTC
Petr Hejl is expert on JB01
Comment 6 Petr Hejl 2008-06-17 14:44:26 UTC
PH01: I don't think these annotations are stable enough to be included in this form as an API.
-definitely it should not be part of gsf (as mentioned in JB01)
-these annotations have class level retention (I'm not sure if this is desired)
-although there is explicit reference to findbugs in javadoc, these won't work with findbugs 1.3.x

I filed a separate issue for this: issue 137437.
Comment 7 Torbjorn Norbye 2008-06-19 01:05:17 UTC
JB01, PH01 (the nullness annotations in GSF): 
Agreed, the annotations in GSF should not be there; they are there until NetBeans gets standard annotations for these.
As soon as they become available, I will immediately switch over to using them.  (I spoke to Petr and Yarda about this
last time I was in Prague, and my recollection from our meeting was that I had an action item to look into JSR 305
annotations, but since this wasn't a pressing need for 6.1 and we were already close to code freeze, I didn't work on it.)

As a general comment however, note that for this inception review, individual APIs are not really ready for review;
there are many remaining problems at the individual class level. Many javadocs are incomplete as well. (By the way, the
wiki page http://wiki.netbeans.org/GsfIssues lists some of the remaining problems ).

For this -inception- review, I'd like to focus on all the higher level documentation; this is located in gsf.api/doc/ in
the Mercurial repository. This will cover the high level architecture of GSF, the principle of providing UI and
infrastructure in the gsf implementation module, and language specific lexing/parse tree inspection in language plugins.

It's better if you read the documentation out of your own gsf.api module (e.g. run ant javadoc there) instead of the
attachment since I have and plan to keep updating the documentation.


JB02, JB03: (Parsing/Indexing APIs)
Agreed. GSF will switch to the Parsing and Indexing API before the GSF APIs themselves are reviewed.


YT01: (javadoc summary page)
Ok, will do. 


YT02: (Use data loader factory)
Yes, I should do this. If I can properly solve the registration issue (which already requires me to populate files into
the system file system, for the navigator, tasklist and editor features), I should be able to dynamically register a
factory as well.  I had hoped to use your dynamic file system facility (issue 26338) to support this, but I tried
implementing it and couldn't get it to work; it appeared that the dynamic file system is added too late. Another
approach somebody suggested was to use code generation at build time to modify language client layer files to register
services.   I plan to look into both approaches in more depth; this is part of the registration issues that are still open.

YT03 (Declarative registrations)
I believe this is already the case. The blacklist unit test ensures that this is the case.

YT04 (Apisupport wizards)
Yes, good idea (though it won't really save a lot of work; the whole point of GSF is to remove as much boiler plate as
possible, thus nearly every class in say the Ruby support or the JavaScript support contains details specific to
Ruby/JavaScript, not generic registration stuff). I can definitely help as a starting point though, getting the basic
registration in place; a language config object, a layer registration of it, and probably a mime resolver and actions
registration. These latter parts aren't GSF specific, but on the other hand it's very likely anybody trying to write a
new language support will need to start there.

YT05 (Code coverage)
I've discussed unit testing in more detail in the doc/unit-testing.html document. The story for language clients is
pretty good; GSF adds testing infrastructure such that language clients have to do very little to test their features
(keystroke handlers, code completion, etc) - typically just a single method call pointing to a golden file, and a caret
(or selection) position. Golden files are generated on demand so to add new tests, just add a new golden file and caret
position, run the test, and inspect the newly created golden file. Therefore, language support modules like Ruby and
JavaScript have a lot of detailed tests (438 for javascript editing and hints, 718 for ruby editing and hints).

The GSF implementation module itself has very very poor test coverage. It's nearly all UI, which is harder to test. I've
had a lot of difficulties with the test infrastructure (for example, I still haven't gotten my own GsfDataLoader to show
up and own files in unit tests; despite adding every test-dependency etc. I could find, all my Documents end up being
PlainDocuments instead of BaseDocuments). Thus, I have some workarounds in GSF where it loads and populates
BaseDocuments manually. This is adequate for testing the relatively standalone language clients, which talk through
simple APIs like "give me all your completion alternatives at offset X". For GSF however it's all deep in the guts of
NetBeans and I need to make it act like a real live IDE instance. I would highly appreciate help in this area to improve
the unit testing of the infrastructure itself. 
Comment 8 Torbjorn Norbye 2008-06-19 03:21:46 UTC
By the way, regarding service registration, Vita had a third idea: Use entities in language plugins to "pull in" all the
stuff needed by GSF at XML parse time. Something like
  <file name="Editors">
    <file name="mime-main">
      <file name="mime-minor">
         &gsf-hooks;
      </file>
      ...
Comment 9 Jaroslav Tulach 2008-06-19 12:50:11 UTC
Y03, additional comments: the GSF shall support declarative registration of "everything". I am not sure how to define 
everything, but I know issue 131401 is not currently supported and imho it should be as it is common usecase and 
workaround is ugly.

Y02+Y04, additional comments: Imho, the wizard shall generate all the necessary layer entries. I can imagine there is 
many of them - another reason to use wizard, imho. Using the dynamic registration is possible, but not really 
effective, as that is not cached and needs to be recomputed every start. 

Y05: Imho the wizard shall also generate the test, if applicable to prepare testing environment. That is what I did 
for "File Type" in recent builds (btw. that means your newly generated loader shall be testable). At worse, use 
NbModuleSuite to wrap your test, then it will be executed in real NetBeans environment.
Comment 10 Miloslav Metelka 2008-06-24 15:36:39 UTC
MM01: I understand that one of the benefits of GSF is that you have everything on one pile. The main benefit of GSF from
my point of view is an interconnection of APIs such as completion or folds with nodes in ASTs. Still I think that GSF
API should not duplicate one-to-one an information that can be found by using other APIs. For example EditorOptions
could be discussed in this way. IMHO there can be links in javadocs for GSF classes (or links in GSF tutorials) that
point the developers to the appropriate primary API that contains the particular information.

MM02: Just a nitpick - IMHO the org.netbeans.modules.gsf.spi with all the Default* impls should rather be named
org.netbeans.modules.gsf.spi.support according to existing NB conventions.
Also are all the things in org.netbeans.modules.gsf.api necessary to be seen by clients? I.e. shouldn't they be rather
an SPI if only implementors would use them?
Comment 11 Torbjorn Norbye 2008-06-24 15:55:59 UTC
MM01: EditorOptions is a class that was there in 6.0/6.1 to try to help the situation with editor options, but I'm
hoping I can completely remove it now that Vita has rewritten the editor settings infrastructure.  I agree with your
general point; there shouldn't be duplication unless there's value add. I think this will be one of the key areas to
review when we get to the actual API review of GSF.

MM02: Regarding the .support package - thanks for the suggestion, I will change it to that.

"Also are all the things in org.netbeans.modules.gsf.api necessary to be seen by clients? I.e. shouldn't they be rather
an SPI if only implementors would use them?"

Pretty much all the APIs in GSF are intended to be implemenented by language plugins, so I suppose you could argue it's
-all- SPI. (There are a couple of interfaces that are implemented by GSF instead but that's the exception to the rule.)
Comment 12 Vitezslav Stejskal 2008-06-24 18:22:55 UTC
The inception review took place on 24-jun-2008. The API was accepted by all the voting reviewers for further development
with the following TCRs:

1. move annotations package outside of GSF api
2. remove parsing related classes from GSF and use the new parsing API
3. create indexing API as part of the parsing API
4. remove GSF copy of the classpath API and use the standard classpath API
5. no unneccessary classes loaded on startup, especially no language specific classes
6. use XML layers for registrations

Please see the attached meeting write up for details (I'm sorry for any misunderstandings or confusion).
Comment 13 Vitezslav Stejskal 2008-06-24 18:23:49 UTC
Created attachment 63352 [details]
Inception review write up
Comment 14 Jesse Glick 2011-11-28 18:11:01 UTC
Obsolete?
Comment 15 Tomas Zezula 2011-11-28 18:15:43 UTC
Yes, obsolete.
The GSF was deprecated and replaced by CSL.