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 83347 - [65cat] Stabilize Editor Hints API
Summary: [65cat] Stabilize Editor Hints API
Status: RESOLVED FIXED
Alias: None
Product: editor
Classification: Unclassified
Component: Hints & Annotations (show other bugs)
Version: 6.x
Hardware: All All
: P2 blocker with 1 vote (vote)
Assignee: David Strupl
URL:
Keywords: API
: 132206 135077 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-08-24 09:49 UTC by Jan Lahoda
Modified: 2009-04-15 08:51 UTC (History)
15 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Generated javadoc. (103.72 KB, application/octet-stream)
2006-08-24 09:50 UTC, Jan Lahoda
Details
First version of a patch (29.46 KB, patch)
2009-04-01 13:04 UTC, David Strupl
Details | Diff
Current version of javadoc (115.64 KB, application/x-compressed)
2009-04-02 07:47 UTC, David Strupl
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Lahoda 2006-08-24 09:49:13 UTC
I would like to request an inception review of a new editor hints API.

The usecases and open issues are available here:
http://editor.netbeans.org/hints/arch.html

Prototype implementation is in the NetBeans CVS, under contrib/editorhints/spi.

I will attach generated javadoc - still very imperfect though.
Comment 1 Jan Lahoda 2006-08-24 09:50:11 UTC
Created attachment 33230 [details]
Generated javadoc.
Comment 2 Martin Entlicher 2006-08-28 16:27:53 UTC
I've just question about DefaultHighlight. Why does it take Position objects in
the constructor? Shouldn't there be just int startOffset and int endOffset
instead? Not every client might have Position objects in hand and the ints seems
to be enough...
Comment 3 Petr Nejedly 2006-08-28 18:16:21 UTC
Why does the infrastructure officially prefer push model, moreover explicitely
stating:

  1. The error provider finds out that a document changed and it is necessary
     to scan it for errors.
  2. The error provider starts to scan the document for errors, using its own
     infrastructure and thread.

While I understand it is desirable to support asynchronous push of errors (e.g.
to support external, slow, on-demand scanning tool), I believe it should at
least suggest, better yet implement common infrastructure to use, to prevent
zillions plugins independently listening and parallely parsing the file. 

Generally, it seems to me that the infrastructure complexity is not mandated by
the set of use cases yet.
Comment 4 Jan Lahoda 2006-08-29 08:56:39 UTC
Hi,
    thanks for all the questions so far.

Regarding the DefaultHighlight - the DefaultHighlight is part of the Editor
Highlights API, which should not be part of this API review. It will actualy be
replaced by a different approach. The DefaultHighlight is only a support class -
nothing prevents clients to implement Highlight interface with whatever approach
they need. The DefaultHighlight takes Position to assure consistency - if the DH
took ints, how it would be assured that the document was not changed during
the constructor invocation? Moreover, the constructor would need to take
Document reference, because it would need to create Positions (of some form)
under the hood anyway.

pnejedly: a very good question. First, why the push model is preffered (applies
only to editor hints, I do not want to generalize here):
-building a pull layer on top of a push layer is IMO relatively straightforward
and should not cause performance degradation. Building a push layer on top of
pull layer will IMO be always cumbersome and degrade performance a bit at least.
-if the pull model is used, the infrastructure needs to decide when it will ask
for errors/hints for every client. Consider (imaginary) Java and (imaginary)
C/C++ support: the Java support wants to parse for errors 500ms after last
document modification. It does not want to parse on save unless the document has
been modified since the last parse. The C/C++ support wants to parse for errors
on save (for whatever reason). So the infrastructure needs to ask for
errors/hints 500ms after last modification and on save and the plugins
need to ignore some of these requests. If a plugin detects (in a domain-spefic
way) that something is changed and it needs to reparse, it needs to use some
callback into the infrastructure and wait. So I think the push model gives more
freedom to the clients.
-the push model is in no way suited only for slow error providers - it is also
suited for the new planned Java model (which should provide errors quite fast).

But, I am also concerned about a lot of plugins listening somewhere, etc. - this
is open issue "Support for Simple Error Providers". Actually, there was a pull
layer built on top of the editor hints, but I removed it for the review to see
if we need it or not. It is in contrib/editorhints/spisupport (please look only
at the API, ignore the implementation itself).

Regarding the too complex infrastructure, could you be please more specific? Are
there some particular things that are/seem to be useless? Or is there some more
simple API that would seem to suffice? I will be glad to either extend the
usecases or schedule removal of the useless parts.
Comment 5 Vitezslav Stejskal 2007-11-08 10:46:51 UTC
The history of Editor Hints API was not very bright, let's make its future brighter. There was an inception review for
the API, which approved implementing a friend API, details in http://www.netbeans.org/issues/show_bug.cgi?id=60843. This
I believe had happened somewhere around 5.x. In 6.0 the API was 'retouched' and made completely private with quite a few
modules impl-depending on it (impl-deps.txt@deadlock):

MODULE org.netbeans.api.gsf (ruby)
MODULE org.netbeans.modules.editor.codetemplates (ide)
MODULE org.netbeans.modules.gsf (ruby)
MODULE org.netbeans.modules.j2ee.ejbverification (j2ee)
MODULE org.netbeans.modules.j2ee.jpa.verification (java)
MODULE org.netbeans.modules.java.editor (java)
MODULE org.netbeans.modules.java.hints (java)
MODULE org.netbeans.modules.javadoc (java)
MODULE org.netbeans.modules.mobility.editor (mobility)
MODULE org.netbeans.modules.ruby.hints (ruby)
MODULE org.netbeans.modules.websvc.core (j2ee)
MODULE org.netbeans.modules.websvc.editor.hints (j2ee)

Reassigning back to editor.
Comment 6 Vitezslav Stejskal 2008-04-09 09:14:28 UTC
*** Issue 132206 has been marked as a duplicate of this issue. ***
Comment 7 Vitezslav Stejskal 2008-05-19 09:44:36 UTC
*** Issue 135077 has been marked as a duplicate of this issue. ***
Comment 8 David Strupl 2009-03-05 08:41:47 UTC
This is API defect.
Comment 9 David Strupl 2009-03-12 15:37:54 UTC
Temporarily making this P3. I plan to do this after M3 before beta.
Comment 10 David Strupl 2009-03-31 12:38:06 UTC
Starting to work on this one. Making it back to P2 priority.
Comment 11 David Strupl 2009-03-31 13:17:54 UTC
The new list of impl dependencies is here:

MODULE org.netbeans.modules.cnd.highlight (cnd)
MODULE org.netbeans.modules.cnd.refactoring (cnd)
MODULE org.netbeans.modules.csl.api (ide)
MODULE org.netbeans.modules.editor.codetemplates (ide)
MODULE org.netbeans.modules.gsf (ide)
MODULE org.netbeans.modules.gsf.api (ide)
MODULE org.netbeans.modules.j2ee.ejbverification (enterprise)
MODULE org.netbeans.modules.j2ee.jpa.verification (java)
MODULE org.netbeans.modules.java.editor (java)
MODULE org.netbeans.modules.java.hints (java)
MODULE org.netbeans.modules.java.hints.analyzer (java)
MODULE org.netbeans.modules.javadoc (java)
MODULE org.netbeans.modules.javascript.hints (ide)
MODULE org.netbeans.modules.maven.hints (java)
MODULE org.netbeans.modules.mobility.editor (mobility)
MODULE org.netbeans.modules.ruby.hints (ruby)
MODULE org.netbeans.modules.websvc.core (enterprise)
MODULE org.netbeans.modules.websvc.editor.hints (enterprise)
Comment 12 David Strupl 2009-03-31 13:50:15 UTC
+ the stuff from extra that I did not have built locally:

MODULE org.netbeans.modules.editor.hints.i18n (extra)
MODULE org.netbeans.modules.javahints (extra)
MODULE org.netbeans.modules.spellchecker (extra)
Comment 13 David Strupl 2009-04-01 13:04:36 UTC
Created attachment 79221 [details]
First version of a patch
Comment 14 David Strupl 2009-04-01 13:17:06 UTC
I would like to ask 

mmetelka
vstejskal
mfukala
jtulach

to be the reviewers for this API.
Comment 15 Jaroslav Tulach 2009-04-01 17:03:56 UTC
Y01 Can you generate and publish javadoc documentation, please? Just running 
spi.editor.hints$ ant javadoc
failed.
Comment 16 David Strupl 2009-04-02 07:47:09 UTC
Created attachment 79259 [details]
Current version of javadoc
Comment 17 David Strupl 2009-04-02 07:48:21 UTC
Y01: ant javadoc worked for me when I have the patch applied. I have attached the resulting zip to the issue.
Comment 18 Jaroslav Tulach 2009-04-02 08:35:05 UTC
Y02: Create apichanges.xml, put there initial note: Making API stable, so it appear in the list of API changes for 6.7
Y03: Think up something for "XXX no answer for arch-usecases" <usecase id="..." name="...">blablabla link to 
javadoc</usecase>. Btw. the "blablabla" shall probably describe what one has to do to show own hint in text/something 
mimetype, I cannot find the info anywhere in the javadoc.
Y04: Referencing index.html in <api/> tag nests frames into frames. Probably refer to 
@TOP@/org/netbeans/spi/editor/hints/package-summary.html
Y05: Create package.html in src/org/netbeans/spi/editor/hints/
Y06: Provide some javadoc for Severity, HintsController, ErrorDescription(Factory), Fix, EnhancedFix.
Comment 19 misterm 2009-04-06 16:58:54 UTC
There is an unspecified behaviour in the way hints work today that should be changed.

Currently, hints are not invoked in a guarded section. It is a good default, as guarded sections shouldn't be changed 
directly by hints, but there are valid use cases for evaluating a hint in a guarded section.

First, one might have written a specialized hint that knows what to do to fix errors in a guarded section - changing 
the .form file for Matisse, for instance, or applying the fix correctly in a editor-generated BeanInfo file. 

Also a hint might be displayed next to the element it refers, but the fix will actually change code somewhere else - in 
the same file or even in a related file.

It's possible to work around these limitations by writing a hint that runs on COMPILATION_UNIT, but it's a hack and 
makes hints analyze files that don't actually contain the trees they are interested in.

Therefore, an annotation, marker interface or separate layer folder should be made available, since most hints 
shouldn't run on guarded blocks, but there must be a valid way for the ones who need to do so to state that.
Comment 20 Miloslav Metelka 2009-04-08 12:39:47 UTC
MM01:
HintsController.setErrors() could possibly mention an example of a real value for "layer" variable.

MM02:
ErrorDescription would deserve javadoc. For example ErrorDescription.getDescription() is just plain string or can it contain html as well? Similar for 
Fix.getText().

MM03:
Although Honza already explained push/pull model advantages maybe we could discuss it on the review for some clarifications.



Comment 21 Geertjan Wielenga 2009-04-08 12:42:36 UTC
Please make sure to provide a complete example that illustrates how all/most things relating to this API work.
Comment 22 David Strupl 2009-04-08 14:22:05 UTC
Review meeting was hold today: present dstrupl, vstejskal, mmetelka, jlahoda, jtulach

Vote 1: "under development" vs. "stable" API: vote 3:1 --> Under Development

Vote 0: integrate before NB 6.7 Beta: vote  4:0 --> Approved

Additional recommendations:

a. do not delete anything from the API now, possibly deprecate methods that should not be called by the clients
     (voted 4:0)
Comment 23 David Strupl 2009-04-14 09:59:57 UTC
Most of the raised concerns should be resolved after 62b3cb48b9bf (jet-main).

Please file individial bugs/enhancements for further improvments in the spi.editor.hints.
Comment 24 Geertjan Wielenga 2009-04-14 12:13:23 UTC
Where is a sample that can be used for the tutorial? 
Comment 25 David Strupl 2009-04-14 12:55:49 UTC
I have put 2 examples into arch.xml. They contain only calls related to this SPI - not the other ones. Do you need some
more context or will these be ok?
Comment 26 Geertjan Wielenga 2009-04-14 13:02:58 UTC
I will have a look and let you know if it is sufficient.

Can you look at this and tell me what needs to be changed: http://platform.netbeans.org/tutorials/nbm-java-hint.html
Comment 27 Geertjan Wielenga 2009-04-14 19:34:56 UTC
And should the "Under Development" indicator be removed from the Javadoc for these APIs?
Comment 28 Quality Engineering 2009-04-15 07:48:09 UTC
Integrated into 'main-golden', will be available in build *200904150201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/62b3cb48b9bf
User: David Strupl <dstrupl@netbeans.org>
Log: Issue #83347: Javadoc for spi.editor.hints updated.
Comment 29 David Strupl 2009-04-15 08:51:41 UTC
> And should the "Under Development" indicator be removed from the Javadoc for these APIs?

No. As was voted on the review this API will stay in the "Under Development" category for 6.7. That means that its
javadoc will not be part of the release JavaDoc - it will be only in the Dev javadoc.

The situation is still different from the previous state: now you can declare normal dependency on the module instead of
the implementation dependency.