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 218312

Summary: Annotation to register keywords for panels in Options dialog
Product: platform Reporter: Theofanis Oikonomou <theofanis>
Component: Options&SettingsAssignee: Theofanis Oikonomou <theofanis>
Status: RESOLVED FIXED    
Severity: normal CC: apireviews, mkristofic, ovrabec, tmysik, tzezula
Priority: P2 Keywords: API_REVIEW_FAST
Version: 7.3   
Hardware: All   
OS: All   
Issue Type: DEFECT Exception Reporter:
Bug Depends on:    
Bug Blocks: 216474, 217166    
Attachments: new annotation
OptionsPanel getting the registered keywords
Editor and F&C panels using the new annotation
new annotation
OptionsPanel getting the registered keywords
new annotation
OptionsPanel getting the registered keywords
General, Editor, F&C, Keymap, Java and Miscellaneous panels using the new annotation
new annotation
OptionsPanel getting the registered keywords
General, Editor, F&C, Keymap, Java and Miscellaneous panels using the new annotation
PHP and C/C++ panels using the new annotation

Description Theofanis Oikonomou 2012-09-13 17:46:45 UTC
There should be a way to declare keywords for panels in the Options dialog without needing to load the actual panel, as this makes the search in options really slow. 

I am attaching the new annotation in order to do this, the way it is used from OptionsPanel in order to get the registered keywords and how the new annotation can be used from every panel (Editor/General, Editor/Hints, Editor/Highlighting, F&C/Syntax, F&C/Highlighting, F&C/Annotations)
Comment 1 Theofanis Oikonomou 2012-09-13 17:50:42 UTC
Created attachment 124324 [details]
new annotation
Comment 2 Theofanis Oikonomou 2012-09-13 17:51:36 UTC
Created attachment 124325 [details]
OptionsPanel getting the registered keywords
Comment 3 Theofanis Oikonomou 2012-09-13 17:52:40 UTC
Created attachment 124326 [details]
Editor and F&C panels using the new annotation
Comment 4 Theofanis Oikonomou 2012-09-13 17:55:15 UTC
Please comment on the new annotation. Thank you
Comment 5 Theofanis Oikonomou 2012-09-14 07:13:02 UTC
The new annotation will have to be used by everyone that registers some panel in the options dialog in order for the search to function properly.
Comment 6 Tomas Zezula 2012-09-17 12:40:35 UTC
tz01: Why the annotation attributes are written to the layer by the OptionsPanelControllerProcessor while the runtime uses the annotation values from classes? Who uses the FileObject attribute "keyword-\d+"?
Comment 7 Theofanis Oikonomou 2012-09-19 11:31:26 UTC
Created attachment 124572 [details]
new annotation
Comment 8 Theofanis Oikonomou 2012-09-19 11:32:18 UTC
Created attachment 124573 [details]
OptionsPanel getting the registered keywords
Comment 9 Theofanis Oikonomou 2012-09-19 11:39:45 UTC
(In reply to comment #6)
> tz01: Why the annotation attributes are written to the layer by the
> OptionsPanelControllerProcessor while the runtime uses the annotation values
> from classes? 

I updated the patches that introduce the new annotation (now has RetentionPolicy.SOURCE) and get the registered keywords (see especially handlePanel(FileObject keywordPanel) method). Jarda is this the correct way to get the registered keywords without loading the actual panels?

>Who uses the FileObject attribute "keyword-\d+"?

If there are more than one keywords (the expected case) then only the last one is written to the layer file, if I just use "keyword" as the name for the attribute. So, I am writing attributes with name values "keyword-1", "keyword-2", etc to actually write all the available keywords from some Panel. Is there some other way I could do this?

Thank you
Comment 10 Jaroslav Tulach 2012-09-19 13:17:39 UTC
Y01 I can't envision proper usage of the annoation and why it is better than existing http://bits.netbeans.org/dev/javadoc/org-netbeans-modules-options-api/org/netbeans/spi/options/OptionsPanelController.ContainerRegistration.html#keywords%28%29 ? Maybe you want to attach a patch modifying some existing panel to use your annotation.

Y02 There already is some support for handling keywords and storing them in a layer. I don't think we need two ways of doing the same thing. So the old and new support should store data on the disk in the same format.

Y03 KeywordPanel name misleads me a bit. I tend to think it is a JPanel subclass for editing keywords. Why not just @Keywords or @KeywordRegistration?

Y04 As keywords have to be localized, I was hoping for some interaction with @NbBundle.Messages. For example people could just use

@Keywords(location=...)
@NbBundle.Messages({
  "CTL_Label=...",
  "CTL_Text=...."
})

and the processor would register all the bundle keys as keywords. This would save users a lot of typing, I think.

Y04 No tests? Please write a test to show the interaction between the annotation and the consumer of the annotation. E.g. if somebody asks a keyword appropriate place in editor is shown.

Y05 You should not assume attributes of a file object are sorted when reading them.

Y06 Can one panel by registered with two KeywordPanels? I can't imagine the usecase. 

Y07 Can you use "bundlevalue" instead of if (id.startsWith("#")) {
id = NbBundle.getMessage(keywordPanel.getClass(), id.substring(1)); Btw. as keywordPanel is FileObject, the previous code is invalid anyway. Another reason to write some tests (see Y04).

Y08 I am not sure I understand "index" javadoc. It says something about alphabetic sort. Then why do I need an int!?
Comment 11 Theofanis Oikonomou 2012-09-19 15:38:35 UTC
(In reply to comment #10)
> Y01 I can't envision proper usage of the annoation and why it is better than
> existing
> http://bits.netbeans.org/dev/javadoc/org-netbeans-modules-options-api/org/netbeans/spi/options/OptionsPanelController.ContainerRegistration.html#keywords%28%29

This only works for top level options category (General, Editor, Java, ...). When I tried to use http://bits.netbeans.org/dev/javadoc/org-netbeans-modules-options-api/org/netbeans/spi/options/OptionsPanelController.SubRegistration.html#keywords%28%29 instead I could not access panels registered under Fonts&Colors category as they do not use this annotation. So, I decided to introduce the new one.

> ? Maybe you want to attach a patch modifying some existing panel to use your
> annotation.

you mean something like the "Editor and F&C panels using the new annotation" attachment?

> 
> Y02 There already is some support for handling keywords and storing them in a
> layer. I don't think we need two ways of doing the same thing. So the old and
> new support should store data on the disk in the same format.

no strong opinion against that. I will change it.

> 
> Y03 KeywordPanel name misleads me a bit. I tend to think it is a JPanel
> subclass for editing keywords. Why not just @Keywords or @KeywordRegistration?

sure, @Keywords is better.

> 
> Y04 As keywords have to be localized, I was hoping for some interaction with
> @NbBundle.Messages. For example people could just use
> 
> @Keywords(location=...)
> @NbBundle.Messages({
>   "CTL_Label=...",
>   "CTL_Text=...."
> })
> 
> and the processor would register all the bundle keys as keywords. This would
> save users a lot of typing, I think.

My idea was that users should pick ~5-10 keywords (or more if they choose to) that best describe the panel they are registering and not use every bundle text. I do not know what users think about this though.

> 
> Y04 No tests? Please write a test to show the interaction between the
> annotation and the consumer of the annotation. E.g. if somebody asks a keyword
> appropriate place in editor is shown.

sorry for that. I will.

> 
> Y05 You should not assume attributes of a file object are sorted when reading
> them.

I do not think I am.

> 
> Y06 Can one panel by registered with two KeywordPanels? I can't imagine the
> usecase. 

for example have a look at org.netbeans.modules.options.editor.FolderBasedController
it creates two org.netbeans.modules.options.editor.FolderBasedOptionPanel

> 
> Y07 Can you use "bundlevalue" instead of if (id.startsWith("#")) {
> id = NbBundle.getMessage(keywordPanel.getClass(), id.substring(1)); Btw. as
> keywordPanel is FileObject, the previous code is invalid anyway. Another reason
> to write some tests (see Y04).

I will write the test.

> 
> Y08 I am not sure I understand "index" javadoc. It says something about
> alphabetic sort. Then why do I need an int!?

This is similar to the tab index in any tabbedpane. In Miscellaneous category the tabs are sorted alphabetically. This means that even if the user specifies an index value it will not be taken into consideration.
Comment 12 Theofanis Oikonomou 2012-09-25 14:48:57 UTC
Created attachment 124882 [details]
new annotation
Comment 13 Theofanis Oikonomou 2012-09-25 14:49:34 UTC
Created attachment 124883 [details]
OptionsPanel getting the registered keywords
Comment 14 Theofanis Oikonomou 2012-09-25 14:50:45 UTC
Created attachment 124884 [details]
General, Editor, F&C, Keymap, Java and Miscellaneous panels using the new annotation
Comment 15 Theofanis Oikonomou 2012-09-25 15:03:52 UTC
Updated all the patches based on jtulach's comments.

Simplified the use of the annotation:
1) renamed id to tabTitle
2) tabtitle only needs to be filled in the case some panel is registered under the Miscellaneous category
3) index is made optional and only needs to be filled in the case some panel is registered under the Editor, F&C, Java, PHP and C/C++ categories
4) keywords are now saved the same way as keywords for global Quick Search were saved
5) bundle and Bundle.Messages keys can be used
6) renamed KeywordPanels/KeywordPanel to KeywordsRegistration/Keywords

Used the new annotation to all (or so I think, please correct me if I am wrong) the panels registered in the General, Editor, F&C, Keymap, Java and Miscellaneous categories.

Included a test that checks that the correct categories/tabs are enabled/selected while performing various searches.

Please comment as I would like to integrate on Monday. Thank you
Comment 16 Jaroslav Tulach 2012-09-27 09:42:14 UTC
Y11  @org.junit.Test is meaningless in a NbTestCase subclass (which is using 3.8 conventions).

Y12 Many of the annotations in the sample patch don't use localized values.

Y13 location() is often localized (which I found strange), but its javadoc says "reference to container's ID" - that is probably not localized, right? So what is happening?
Comment 17 Theofanis Oikonomou 2012-09-27 11:08:10 UTC
(In reply to comment #16)
> Y11  @org.junit.Test is meaningless in a NbTestCase subclass (which is using
> 3.8 conventions).

true. I will change this

> 
> Y12 Many of the annotations in the sample patch don't use localized values.

it was not my intend to actually fill in the real values. I will file P2 bugs so that each owner adds the appropriate keywords.

> 
> Y13 location() is often localized (which I found strange), but its javadoc says
> "reference to container's ID" - that is probably not localized, right? So what
> is happening?

using localized values for location() is a mistake (sorry for that). The javadoc states it correctly. I will update the javadoc and also introduce compile-time constants similar to OptionsDisplayer.ADVANCED

I will update the patches shortly. Thank you
Comment 18 Theofanis Oikonomou 2012-09-27 15:59:59 UTC
Created attachment 125012 [details]
new annotation

handling [Y11] and [Y13], also introduced four compile-time constants: OptionsDisplayer.GENERAL, OptionsDisplayer.EDITOR, OptionsDisplayer.FONTSANDCOLORS and OptionsDisplayer.KEYMAPS
Comment 19 Theofanis Oikonomou 2012-09-27 16:00:42 UTC
Created attachment 125013 [details]
OptionsPanel getting the registered keywords
Comment 20 Theofanis Oikonomou 2012-09-27 16:03:40 UTC
Created attachment 125014 [details]
General, Editor, F&C, Keymap, Java and Miscellaneous panels using the new annotation

updated all panels to use the newly introduced compile-time constants.

updated all modules to require the correct version of options.api module and increased their Specification Version
Comment 21 Theofanis Oikonomou 2012-10-01 09:46:51 UTC
If there are no more comments I will integrate later today and file P2 so that each owner adds appropriate keywords and/or use the new annotation to other panels that I was not able to locate or handle (such as options for PHP and C/C++). Thank you
Comment 22 Tomas Mysik 2012-10-01 09:48:12 UTC
(In reply to comment #21)
> I was not able to locate or handle (such as options for PHP and
> C/C++)

Just curious - why?
Comment 23 Theofanis Oikonomou 2012-10-01 10:05:14 UTC
(In reply to comment #22)
> Just curious - why?

no particular reason. Good point. I will at least add the annotation and update the dependency and spec version for PHP and C/C++ panels. Thanks
Comment 24 Tomas Mysik 2012-10-01 11:17:11 UTC
(In reply to comment #23)
> I will at least add the annotation and update
> the dependency and spec version for PHP and C/C++ panels. Thanks

That would be great, thanks.
Comment 25 Theofanis Oikonomou 2012-10-01 11:28:27 UTC
Created attachment 125141 [details]
PHP and C/C++ panels using the new annotation
Comment 26 Tomas Mysik 2012-10-01 13:24:38 UTC
(In reply to comment #25)
> Created attachment 125141 [details]
> PHP and C/C++ panels using the new annotation

Thanks a lot!
Comment 27 Theofanis Oikonomou 2012-10-02 09:06:56 UTC
Thank you for all your comments.

Integrated into core-main: http://hg.netbeans.org/core-main/rev/b775c291d2d2
Comment 28 Quality Engineering 2012-10-03 02:42:59 UTC
Integrated into 'main-golden', will be available in build *201210030002* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/b775c291d2d2
User: Theofanis Oikonomou <theofanis@netbeans.org>
Log: Issue #218312 - Annotation to register keywords for panels in Options dialog
Comment 29 Milutin Kristofic 2012-10-11 16:18:04 UTC
Y01 I am sorry that I am late for this discussion, but it would be much easier to implement subRegistration for Fonts & Colors tabs then implementing new annotation and adding it everywhere and also in Fonts & Colors.