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: | Annotation to register keywords for panels in Options dialog | ||
---|---|---|---|
Product: | platform | Reporter: | Theofanis Oikonomou <theofanis> |
Component: | Options&Settings | Assignee: | 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
Created attachment 124324 [details]
new annotation
Created attachment 124325 [details]
OptionsPanel getting the registered keywords
Created attachment 124326 [details]
Editor and F&C panels using the new annotation
Please comment on the new annotation. Thank you 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. 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+"? Created attachment 124572 [details]
new annotation
Created attachment 124573 [details]
OptionsPanel getting the registered keywords
(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 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!? (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. Created attachment 124882 [details]
new annotation
Created attachment 124883 [details]
OptionsPanel getting the registered keywords
Created attachment 124884 [details]
General, Editor, F&C, Keymap, Java and Miscellaneous panels using the new annotation
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 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? (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 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
Created attachment 125013 [details]
OptionsPanel getting the registered keywords
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
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 (In reply to comment #21) > I was not able to locate or handle (such as options for PHP and > C/C++) Just curious - why? (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 (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. Created attachment 125141 [details]
PHP and C/C++ panels using the new annotation
(In reply to comment #25) > Created attachment 125141 [details] > PHP and C/C++ panels using the new annotation Thanks a lot! Thank you for all your comments. Integrated into core-main: http://hg.netbeans.org/core-main/rev/b775c291d2d2 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 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. |