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.
Created attachment 101916 [details] patch for moving GoToMarkOccurrencesAction to csl.api GoToMarkOccurrencesAction is needed by HtmlKit for providing the csl shortcuts for navigating to next/prev occurrences. Attached is a patch that moves the action to the csl.api package. I'm not sure if this area has currently an owner, so if nobody takes this over I will integrate the patch myself on Friday.
Integrated into 'main-golden', will be available in build *201009110000* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main/rev/412f4907336a User: Erno Mononen <emononen@netbeans.org> Log: #190209 - Expose GoToMarkOccurrencesAction
Please note the API_REVIEW_FAST keyword was added postmortem; I can revert the change if needed.
Y01 Missing @since tag Y02 Missing description of the change in apichanges.xml but most importantly: Y03 Why the class needs to be in API at all? I don't understand why somebody would need to call an action class directly.
Y01, Y02 - will fix these. Y03 - HtmlKit needs to provide the action (in #createActions); similarly to e.g. o.n.m.csl.api.SelectCodeElementAction.
Re. Y03: If HtmlKit needs to provide the action, then just expose the instance so they can use it. The preferred method since 6.10 is to use @ActionID(...) @ActionRegistration(...) class TheActionClass { } and share the parameters for @ActionID with whoever wants to use them. Those people then use @ActionReference to use the action. They also use Utilities.actions2Popup to instantiate the popup menu. Should the above not be possible for some unsolvable reason, expose just a factory method, not the whole class in the API. E.g. just a single public static Action createXYZAction(); which will return the class.
Re - Y03: I think there is some value in following the existing conventions in csl.api - every action there is exposed the same way as GoToMarkOccurrencesAction. IMO it would make more sense to either change them all or stick to the existing way, even if the existing way is not ideal. Having different conventions in a large code base is unavoidable, but it seems undesirable to me that a single package would expose equivalent actions in different ways.
The "consistency defence" is good one. On the other hand, being always consistent with something which is buggy would prevent us to learn from past mistakes. Preferably you could deprecate all the existing action classes, create one CSLActions with many factory methods and just add new one for the actionGoToMarkOccurences(). But I understand this is more work than you probably want to donate.
I think I can find the time for creating CslActions, just not until next week. I'll attach a new patch here then.
Created attachment 102128 [details] CslActions Attached is a patch for introducing CslActions, please review.
Created attachment 102129 [details] HtmlKit migrated to use CslActions
Y11: Summary in apichanges.xml is probably wrong. Y12: deprecation="yes" would be more appropriate Otherwise fine, looks good.
Thanks for the review, I'll fix Y11 and Y12 before pushing the change tomorrow.
web-main f26091baaec7