Community
Participate
Working Groups
The two sub-menus of the "Resource Configurations" context menu item and its sub-menu items "Exclude from Build..." and "Reset to Default..." don't seem to apply in our product. We don't use managed build and we manage our own indexer settings. Yet, this menu is visible and confusing to our users. The history of the commands seems to point to them being used to manage indexer behavior. Since these menus are provided as popup menu elements in object contribution extensions, they can't be turned off via capabilities. I would like to convert them to the new command handler mechanism so they can be hidden. (Could someone explain how these are supposed to work in case they may be useful in our case?)
"Exclude from build" alters the source path path-entries. This has knock on effects for managedbuild (the source isn't built) and the indexer - the source isn't indexed (I believe). See: Properties > C/C++ General > Paths & Symbols > Source Location. "Reset to Default" allows removing resource-specific paths & symbols (as well as build settings...). So if you add an include search path to a folder or file: properties > C/C++ General > Paths & Symbols > Includes > Add ... Reset to Default deletes the resource specific configuration. Both of these should be applicable to all integrations irrespective of MBS being used. Ideally there would be a better name / description, but I'm not sure what that might be...
That is an interesting idea to control those via capabilities. See also some of a discussion in bug 207816.
(In reply to comment #1) > "Exclude from build" alters the source path path-entries. This has knock on > effects for managedbuild (the source isn't built) and the indexer - the source > isn't indexed (I believe). > See: Properties > C/C++ General > Paths & Symbols > Source Location. Ok, I see. But this specific panel is not available in our product's projects. We have a custom builder and custom CConfigurationDataProvider, and the indexer settings and source paths are derived entirely from build files. (I.e. we don't want the user to make changes like this through this UI, since it may be misleading). > "Reset to Default" allows removing resource-specific paths & symbols (as well > as build settings...). So if you add an include search path to a folder or > file: properties > C/C++ General > Paths & Symbols > Includes > Add ... > Reset to Default deletes the resource specific configuration. This one wouldn't be useful for us either -- we only support project-wide paths & symbols settings. > Both of these should be applicable to all integrations irrespective of MBS > being used. Ideally there would be a better name / description, but I'm not > sure what that might be... I agree the code seems to be amenable to any sort of CDT project, but our concern isn't about the naming, but just about not exposing it to our users unless they want to see it (i.e. capabilities). I recommend we just provide a patch to convert these actions into commands, giving "Resource configurations" submenu its own capability-controllable identifier. The actual functionality/naming/enablement tweaks can be done elsewhere.
(In reply to comment #3) > Ok, I see. But this specific panel is not available in our product's projects. > We have a custom builder and custom CConfigurationDataProvider, and the > indexer settings and source paths are derived entirely from build files. (I.e. > we don't want the user to make changes like this through this UI, since it may > be misleading). Wow, finally found someone outside managedbuild who uses this API :)! > I recommend we just provide a patch to convert these actions into commands, > giving "Resource configurations" submenu its own capability-controllable > identifier. The actual functionality/naming/enablement tweaks can be done > elsewhere. Sounds reasonable to me.
Created attachment 172684 [details] proposed change Most of the code is the same between the old actions and the new handlers. Since the actions were API, I had to up the bundle version.
(In reply to comment #5) > Created an attachment (id=172684) > proposed change Here are some comments: - You changed where Resource Configuration menu appears, is it for a reason? It was in the "build" group with indexer and Make Targets before. - Resource Configuration menu is not supposed to appear on project level, only for files and folders. - You limit menu appearance in editor to 5 hardcoded content types. So it does not show for assembler files for example. I assume it is conceivable to have other source types as well and it is inconsistent with PE. - The enablement actually does not work from the editor anymore, the submenu items are always grayed. - One provoked observation not really related to the patch: currently the menu shows in Project Explorer pretty much for every resource (except projects) and items enabled in some cases where it does not make sense. A question (or 3 of them): - Are you interested in "Build Configurations" item? Do you think it should be controlled by the same capability? Do you have configurations in your custom managed build? A couple of comments related to the code: - You need to add copyright headers to the new files, see http://www.eclipse.org/legal/copyrightandlicensenotice.php - In manifest, you should increment to 5.3.0, not to 5.3.100 (from 5.2.100). - I suggest to indicate in JavaDoc when the interface is deprecated (i.e. "deprecated as of CDT 8.0"). That will help later to decide when to retire it.
Created attachment 172774 [details] proposed change - post review Hi Andrew, thanks for the review. :) The issues you mention were caused inadvertently, but thanks for pointing them out. See my comments below. (In reply to comment #6) > Here are some comments: > > - You changed where Resource Configuration menu appears, is it for a reason? > It was in the "build" group with indexer and Make Targets before. I was able to put it into the build group when it was off the resource, but there is no such group in the menu when in the editor, so I left it after additions which I believe was just like before. > - Resource Configuration menu is not supposed to appear on project level, only > for files and folders. Fixed. > - You limit menu appearance in editor to 5 hardcoded content types. So it does > not show for assembler files for example. I assume it is conceivable to have > other source types as well and it is inconsistent with PE. You are right. I've replaced the whole mechanism for determining whether it is an appropriate editor input by adapting it to a file in a project with c nature. This is closer to the original test. > - The enablement actually does not work from the editor anymore, the submenu > items are always grayed. Fixed. > - One provoked observation not really related to the patch: currently the menu > shows in Project Explorer pretty much for every resource (except projects) and > items enabled in some cases where it does not make sense. I didn't address this, but did retain the behavior where it was shown for only files and folders in projects with c nature. > > A question (or 3 of them): > - Are you interested in "Build Configurations" item? Do you think it should be > controlled by the same capability? Do you have configurations in your custom > managed build? You are right in that my primary concern is to be able to hide them using capabilities since they are not very applicable to our project model - see comment #3. The current implementation using object contributions and actions don't respond to capabilities at all. The capabilities to hide them were going to be added in Nokia-specific code. Converting them to the 3.3+ handler model would just allow capability hiding. > A couple of comments related to the code: > - You need to add copyright headers to the new files, see > http://www.eclipse.org/legal/copyrightandlicensenotice.php Done. I put the copyright header from the action files and added a contribution line to show the conversion to handlers. Most of the code is exactly the same as it was in the action class. > - In manifest, you should increment to 5.3.0, not to 5.3.100 (from 5.2.100). Fixed. > - I suggest to indicate in JavaDoc when the interface is deprecated (i.e. > "deprecated as of CDT 8.0"). That will help later to decide when to retire it. Done.
Created attachment 172775 [details] proposed change - post review (2) Oops, removed an extraneous file.
(In reply to comment #8) > Created an attachment (id=172775) > proposed change - post review (2) I don't see anything to pick on in the latest patch, thanks. Marking review flag with a +.
(In reply to comment #9) > I don't see anything to pick on in the latest patch, thanks. Marking review > flag with a +. Thanks for the review!
*** cdt cvs genie on behalf of ddubrow *** Bug 317707 convert resource configs context menu to handlers to allow hiding via capabilities [*] ExcludeFromBuildAction.java 1.11 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/actions/ExcludeFromBuildAction.java?root=Tools_Project&r1=1.10&r2=1.11 [+] ExcludeFromBuildHandler.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/actions/ExcludeFromBuildHandler.java?root=Tools_Project&revision=1.1&view=markup [+] DeleteResConfigsHandler.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/actions/DeleteResConfigsHandler.java?root=Tools_Project&revision=1.1&view=markup [*] DeleteResConfigsAction.java 1.11 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/actions/DeleteResConfigsAction.java?root=Tools_Project&r1=1.10&r2=1.11 [*] plugin.properties 1.202 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/plugin.properties?root=Tools_Project&r1=1.201&r2=1.202 [*] plugin.xml 1.380 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/plugin.xml?root=Tools_Project&r1=1.379&r2=1.380 [*] MANIFEST.MF 1.55 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/META-INF/MANIFEST.MF?root=Tools_Project&r1=1.54&r2=1.55
I committed a fix to DeleteResConfigsHandler for an NPE I'm seeing during shutdown. Simple null pointer check. Genie shoud post the commit info shortly...
*** cdt cvs genie on behalf of jcortell *** Bug 317707: Fixed NPE during shutdown [*] DeleteResConfigsHandler.java 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/actions/DeleteResConfigsHandler.java?root=Tools_Project&r1=1.1&r2=1.2 [*] ExcludeFromBuildHandler.java 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/actions/ExcludeFromBuildHandler.java?root=Tools_Project&r1=1.1&r2=1.2
(In reply to comment #12) > I committed a fix to DeleteResConfigsHandler for an NPE I'm seeing during > shutdown. Simple null pointer check. Genie shoud post the commit info > shortly... Thanks! I never saw the null selection when I tested.
Created attachment 173214 [details] proposed change Andrew, would you mind for the new handlers to not be API (added to an internal package)? This would allow me to backport this to 7.0.1 since it would allow me to maintain the same bundle version. I could keep the deprecated tags out of the 7.0.1 version in case someone is depending on the action classes. The only difference is that the menus would be using the non-api handlers rather than the old actions.
(In reply to comment #15) > Created an attachment (id=173214) > proposed change > Andrew, would you mind for the new handlers to not be API (added to an internal > package)? This would allow me to backport this to 7.0.1 since it would allow me > to maintain the same bundle version. No, not at all. In fact, I regret that I did not suggest that in my review. I am not aware of any reason to keep them as API.
(In reply to comment #16) > No, not at all. In fact, I regret that I did not suggest that in my review. I > am not aware of any reason to keep them as API. Thanks! I had also forgotten that we plan to ship our next release with 7.0.1! :)
*** cdt cvs genie on behalf of ddubrow *** Bug 317707 make new handlers non-api and revert bundle version [*] ExcludeFromBuildAction.java 1.12 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/actions/ExcludeFromBuildAction.java?root=Tools_Project&r1=1.11&r2=1.12 [-] ExcludeFromBuildHandler.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/actions/ExcludeFromBuildHandler.java?root=Tools_Project&view=markup [-] DeleteResConfigsHandler.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/actions/DeleteResConfigsHandler.java?root=Tools_Project&view=markup [*] DeleteResConfigsAction.java 1.12 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/actions/DeleteResConfigsAction.java?root=Tools_Project&r1=1.11&r2=1.12 [+] DeleteResConfigsHandler.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/actions/DeleteResConfigsHandler.java?root=Tools_Project&revision=1.1&view=markup [+] ExcludeFromBuildHandler.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/actions/ExcludeFromBuildHandler.java?root=Tools_Project&revision=1.1&view=markup [*] plugin.xml 1.381 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/plugin.xml?root=Tools_Project&r1=1.380&r2=1.381 [*] MANIFEST.MF 1.56 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/META-INF/MANIFEST.MF?root=Tools_Project&r1=1.55&r2=1.56
*** cdt cvs genie on behalf of ddubrow *** Bug 317707 convert resource configs context menu to handlers to allow hiding via capabilities [+] DeleteResConfigsHandler.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/actions/DeleteResConfigsHandler.java?root=Tools_Project&revision=1.1&view=markup [+] ExcludeFromBuildHandler.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/actions/ExcludeFromBuildHandler.java?root=Tools_Project&revision=1.1&view=markup [*] plugin.properties 1.201.2.1 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/plugin.properties?root=Tools_Project&r1=1.201&r2=1.201.2.1 [*] plugin.xml 1.379.2.1 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/all/org.eclipse.cdt.ui/plugin.xml?root=Tools_Project&r1=1.379&r2=1.379.2.1
