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.
| Summary: | Generic Languages Support | ||
|---|---|---|---|
| Product: | obsolete | Reporter: | Jan Jancura <jjancura> |
| Component: | languages | Assignee: | Jan Jancura <jjancura> |
| Status: | RESOLVED FIXED | ||
| Severity: | blocker | CC: | epdv, jtulach, pbuzek, phrebejk, pjiricka, vstejskal |
| Priority: | P2 | Keywords: | API, API_REVIEW |
| Version: | 6.x | ||
| Hardware: | All | ||
| OS: | All | ||
| URL: | http://wiki.netbeans.org/wiki/view/SchliemannReview1 | ||
| Issue Type: | DEFECT | Exception Reporter: | |
| Bug Depends on: | 133900 | ||
| Bug Blocks: | |||
|
Description
Jan Jancura
2006-12-07 09:56:36 UTC
An explanation of the change in architecture or API: ---------------------------------------------------- - New module languages/engine will be added to ide cluster. This module reads .nbs files from module layers and provides languages support for them (code completion, syntax coloring...) see languages/engine/arch.xml or http://saturn.czech.sun.com/wiki/view/Schliemann for more info. Y01 The arch.xml document does not list any usecases. Y02 Are really all the classes in JavaDoc meant to be public? Y03 What are the entry points to the API? Y04 Why you are using org.jdesktop.layout, this is non-visual API, is it not? Y05 More on Y04 note: The engine shall be split out of visual part of the module. Y06 Do not use org.openide.options Y07 Can you move the wiki page to some public place? wiki.netbeans.org? Y08 I see no exported layer interfaces, please describe what you read from system filesystem using <api name="folder-name"/> tag. Y09 I want to see description - a usecase - how to register some new language - like diff - what to register and where. Y01: fixed Y02: GLF API has been created - feel free to review it, please. Y03: fixed, see arch.xml Y04: fixed Y05: fixed, languages.studio module has been created and some part of visual classes has been move there. Y06: I am not sure I understand it. There are some options accessible through this api only (setCodeFoldingEnable). I do not know how to workaround it now. Y07: done Y08-9: in progress. Some other comments? [JG01] I find it pretty hard to review the Javadoc when there are nearly no Javadoc comments. Please try to write at least a draft of API documentation before asking for a review. That means that every package, class, and method has a Javadoc description which states what it does and what it is intended to be used for. [JG02] Please rename org.netbeans.api.languages.Cookie, we do not want to use the term "cookie" any more for anything. "Context" perhaps? Ditto SyntaxCookie. But see JG09. [JG03] Please use proper generic signatures on all applicable methods, at least in the public API if not in the implementation. E.g. ASTNode.getChildren (which could I guess use Union2, though it's not critical). [JG04] The new wiki page is I guess http://wiki.netbeans.org/wiki/view/Schliemann I see that http://wiki.netbeans.org/wiki/view/SchliemannNBSLanguageDescription is incomplete and has not been updated for a month and a half. Where is the formal description of the NBS syntax? [JG05] CompletionSupport looks to contain nothing Schliemann-specific at all. Shouldn't it rather be moved into the Completion API? [JG06] Probably the constructor and factory method for Cookie need not be public, since I assume only the engine itself makes instances. Again see JG09. [JG07] What is a "PTPath"? The acronym is meaningless to me. [JG08] Missing any description of which context-free grammars are accepted by the engine. (I am assuming that there are some restrictions, or at least that some grammars are much more efficiently parsed than others.) [JG09] Description of calling convention for Java method calls is vague and doesn't sound very friendly. I assume you need to have a public static method with one parameter of type Cookie? When will a SyntaxCookie be passed instead? (Can you specify the parameter to be SyntaxCookie if the method only makes sense to be called with a specific caret position?) If SyntaxCookie just adds the getPTPath() method, why not just move that into Cookie and return null if it is inapplicable? If Cookie/SyntaxCookie is just a struct holding a few objects, why not permit the Java method to just accept those objects it needs directly? E.g. public static void handleCodeCompletion(Document doc, PTPath path) {...} [JG10] Who would call LanguagesManager.getSupportedMimeTypes and why? I am missing use cases for the various accessible classes. For example, TokenInput is not referenced by any other class in the API, so it probably shouldn't be there at all. [JG11] ParserManager.* constants should be an enum. [JG12] ParserManager.get may not accept a parameter of type org.netbeans.modules.editor.NbEditorDocument, as this is not an API-visible class. Similarly, TokenInput.create may not take a org.netbeans.modules.languages.parser.Parser. Thanks Jesse for your comments. I am working hard to fix them. On the other hand API is still not done. But I want to add my modules to standard build (QA...). So, is it possible to do it with unfinished APIs? Do I need some different kind of review? As far as I'm concerned it's no problem to have the modules in the standard build so long as you schedule and plan for a fuller API review before it's too late (i.e. feature freeze). If this issue is considered an inception review of some kind, i.e. a sanity check on the basic idea, then it looks good to me overall. Anyone else? [JG02] fixed [JG03] fixed [JG07] renamed to ASTPath (PT=Parse Tree) [JG10] LanguagesManager.getSupportedMimeTypes () is used in languages/studio module. There is dialog which allows you to list all Schlieamnn based languages and open and edit .nbs file for it. TokenInput has been removed. [JG11] fixed [JG12] fixed Yarda has agreed to create initial review now (before adding to standard build), and some API review later. Y11 Usecases (Y01) - ok example is there, but where on system filesystem it shall be put? Please fix Y08. Y12 Entry points (Y03) - the usecases use obsolete class names - please fix and replace all class names with link to the actual javadoc - 1st it will be more readable, 2nd it will enforce these two things be in sync. Y13 There is dependency on non-public API editor/settings/storage, please justify why in the arch.xml Y14 No SystemOption (Y06) - do not depend on org.openide.options module. If there is an option you need to read, ask editor guys to find other way to expose it, they shall get rid of SystemOption usage as well. Y15 Layer interface (Y08) - in the usecases please say - you are supposed to create this language file here, here here and enclose this into <api/> tag. Y16 I've noticed a lot of test classes. goog. however the tests do not compile. Make this compilable and pass. Y17 create apichanges and write there XY/AB/2007 - the initial version of the API created. Y18 export stability of your java APIs using <api/> Y19 remove obsoleted classes (DbManager) Y20 use generified interfaces (LanguageManager) Y21 (JG05) explain what is purpose of CompletionSupport? Inception review scheduled: Agharta, 7.3 16:00 CET (7:00 PST) [Y11] fixed [Y12] class names updated, but not changed to links yet [Y15] fixed reviewed see:http://wiki.netbeans.org/wiki/view/SchliemannReview1 no end-user impact, not planned for M9 Not needed for beta, lowering priority to P2. Based on a discussion with Hanz, the current outlook is: - some incompatible changes in this API will be needed after NetBeans 6, it is not possible to commit to API compatibility after NB 6 - the goal is to turn this API to Stable by NetBeans 7 Reviewers, what approach do you suggest for NB 6? You need to keep the /0 major release version as a sign that it is not stable yet. The previous review (http://wiki.netbeans.org/wiki/view/SchliemannReview1) ended with resolution "There will be next review 2-3 weeks before feature freeze (API review).". That did not happen. As such I think this API cannot be considered official neither a candidate to be an official review: Y30 Repackage all API classes into org.netbeans.modules.languages.api where they can be under development for 6.0 and future releases. I do not understand you, Yarda. Why repackage? Schliemann API is under development until the next release. We have already agreed on that, there is no change. I would like to do final API review later, because there are still some important changes in APIs (mostly performance reasons). Doing API changes in "under development" API is allowed, so what is the problem? I do not think we should repackage the API now. I'd do what Jesse suggests (add /0) and make sure we clarify the stability as Under Development in arch document for now and move this task to the next release. The problem is that by definition anything in org.netbeans.api{spi}.* packages that appears in an official Netbeans
release is supposed to be an official API. In other words modules are not allowed to declare stability level of their
APIs lower than stable/official in official releases unless they use different packages. Hence the repackaging request.
Repackaging to something and later back seems like really "good" thing to do. I guess we should rather focus on fixing real bugs. Well, there is no need to repackage it back of course. Any API in any package can be declared stable/official. I don't see what the purpose of repackaging would be. Nobody who is not subscribed to apireviews pays attention to org.netbeans.api.* vs. org.netbeans.modules.*.api anyway. It is clear that there are already plenty of users of this API even before 6.0 has been released - it has been popularized through tutorials etc. and there is broad interest in its functionality. Regardless of what package name we use or what stability level we put in the corner of the Javadoc frame, people who need it will use it. If we have to break the API after 6.0 to make it better and there is no reasonable compatible alternative then we will do that and people will just have to adapt. Putting it in o.n.m.*.api and then renaming it back to o.n.api.* would just guarantee that _everyone_ who has used it will have their code broken. Just IMHO. "Schliemann API is under development until the next release" - next release is 6.1! Are you sure that it is the release you are talking about? If not, then it'd be better to repackage. "Repackaging to something and later back" & "Putting it in o.n.m.*.api and then renaming it back to o.n.api.*" - who was the first one to propose renaming back? Definitely not me. Unless you are ready to develop compatibly with in 6.0 (with ~99% confidence from majority of reviewers) get out of official namespace and stay there. The right to release something marked as under development in official namespace is an exception (http://openide.netbeans.org/tutorial/api.html#earlyaddopt), not common case. This was first used for project APIs - well reviewed, documented, tested and lead by Jesse for whom "no reasonable compatible alternative" is not a empty sentence without meaning. As a result, the project APIs never in fact needed to do anything incompatible. Giving the same exception to Schliemann API which did not even finish its review before first release is not acceptable from my point of view[1]. I see this as TCR. If some reviewers disagree, let's call a meeting and vote. [1] Too bad, the API is already in trunk in the official namespace. Otherwise there would be nothing to talk about. I've once again trusted a given word when we did the review before integration and were told that "There will be next review 2-3 weeks before feature freeze (API review)" - I learned my lesson. Thanks to everyone for their opinions - sounds like the majority prefers to keep the current package. Let's do it that way. Is there anything else that needs to be done so this API can be declared as Under Development for 6.0? Please post comments to this issue; we will call a review meeting only if there is a major dispute. If not, I suggest to waive this issue for 6.0 and fix it (i.e. declare the API stable) for the next feature release. Thanks. I do not think there was any vote. I would prefer a conf call, but if other reviewers want to do it over email, I am find with that. I've updated the wiki: http://wiki.netbeans.org/wiki/Diff.jsp?page=SchliemannReview1&r1=9&r2=4 to contain two different votes. Please vote there. If you want vote3 or vote4, add it by Oct19. Deadline for voting is Oct21, 2007. So what is the status of this issue? It is assigned to apireviews. What are reviewers required to do? Is it still bug? This might sound a little bit picky, but what is the future of 'Generic Languages Support' project in general? Maybe we don't need to be reviewing this API now. And actually I would prefer this not to be answered by Hanz (jjancura). I'm not a reviewer - just waiting for improvements of NBS support; seems that nothings has happened since more than
three months :-(
However, I've seen sth. like a possible spec leak in TOKEN definition for Java method calls:
As the parsed token might probably be of different types, it'd probably be a good idea to declare
all these possible types, so they could be used in grammar definition, e.g.:
TOKEN:(token1,token2,token3):my.package.MyClass.myMethod;
And, please, so. extend the wiki docs.
This API will not be used by PhP and in future it will not be used by groovy, HTML, CSS and JavaScript either. Suppprts for those languages will require a real parser. There is probably value in closing the review to finalize the parts of API that will be used for support of .bat, .manifest, .sh, etc. Can you clarify what do mean by "real parser", please? I'm not a reviewer too. Erlang and Scala supports all heavily use language/engines NBS and parser, and I think with a bit more enhancements (for example: status stack, lookahead(https://javacc.dev.java.net/doc/lookahead.html)), language/engines parser can reach the same level of JavaCC. First, everybody can comment here, not only reviewers. Thanks for feedback! This issue is mainly about APIs, but I am happy to see any comments / requirements. Lookahead is on the top of my stack. THis is must have... But what do you mean by "status stack"? Sorry for the typo, "status stack" should be "state stack", please see http://www.netbeans.org/issues/show_bug.cgi?id=124419 jjancura: I admit that "real parser" is very vague, sorry. I mean to say that the languages I mentioned do not plan to use this API. They plan to use custom parsers, not generated by this API. JavaScript will use an existing parser (rhino), php will likely generate a parser using JavaCUP using an existing available grammer, etc. I propose not to go through api review in 6.1 timeframe. We plan to consolidate parsing support in future NB release. This api is still under development. We don't want to freeze this API now. The API is in official packages and they require an API to become stable/official at least after second release - e.g. in 6.1. I said that in http://wiki.netbeans.org/SchliemannReview1 and also expressed my worry that the Schlieman API does not seem to be able to fulfill that requirement. And here we go. Anyway, imho, the result from the review http://wiki.netbeans.org/SchliemannReview1 says: "Make it official for 6.1". Imho, there is no need to go though any review, just say it is official. We can try to make it final as soon as Parsing API will be final OK. It is official. just passing away from your site and found this blog. I must say you have guts that you write this article. <a href="http://www.facebook.com">FB</a> you can solve this bug easily search it again.... http://www.doneessay.co.uk |