Community
Participate
Working Groups
We should add an option to the view template to create context sensitive help for the view. We should also reduce the number of options on the template and remove the second wizard page.
Created attachment 71796 [details] Patch This patch adds the improvements discussed above. I just realized that the use of the <enablement> tag on the contexts.xml file that is generated will probably require the template to be versioned higher than 3.0, where it currently is. This might require some additional review.
Adam, you could check the version of the target platform in the template class and decide whether to show/process this option or not. In this case, you need the target to be >= 3.3
Created attachment 71908 [details] Patch This patch includes a version check to decide whether or not to include the <enablement> tag in the xml file. It also prevents an NPE that I got when testing that came from calling AbstractTemplateSection.getTargetVersion() too early (before the model was initialized). This is done with an assertion in the method to allow template developers to be informed when they are calling the method too early.
Hi Adam. I was reviewing your patch (and your comment) and figured out why you were having a problem with AbstractTemplateSection.getTargetVersion(). Until now, that function was only called after the user clicked finish and the model existed. In your case, you need to check the target version before the user clicks finish and therefore the model is not yet created. The best thing to do here is to return the Target Platform's version (TargetPlatformHelper.getTargetVersion()) instead of using an Assert. The current implementation will always fail at the assert which will cause problems when the user clicks next. By returning the target's version, we take a best guess at what the uesr is targeting and continue on our way. Wassim, what do you think about adding 'getTargetVersion()' back to IFieldData? I noticed it was added and then removed during 3.1 development. I think this would be a better value to key off of for the target version. If the user targets to run with 3.0, yet has his target platform set to 3.3 (because he forgot to change it), we should not offer the option.
Sorry, Chris discovered my mistake, but I forgot to mention it in the bug. I was using the if statement in initializeFields (3 lines of code) to test the assert, but they should be removed. I actually have that code in the generateFiles method now (check the very bottom of ViewTempalte.java), so it is run after the model is initialized. Otherwise, the patch should be good and I don't think any changes are required to the templates framework.
Brian, TargetPlatformHelper.getTargetVersion() will not work, because it will not necessarily match the Eclipse version specified on the first page of the wizard. IFieldData is API. Adding methods to it would be breaking API. We would need to introduce a new interface. Before doing this big production, we would need to revisit the whole New Project API and do something better as a whole. I don't think this particular feature should wait for all that. We can revisit and improve it later. Adam, Please re-attach a better patch that does not contain the extraneous lines.
Created attachment 71958 [details] Patch
Figured Adam should get credit for the work he did.
Slightly modified patch applied to HEAD. With the latest patch, we were no longer calling the getTargetVersion before the model was initialized, therefore we did not require any changes to the pde.core plug-in. Also removed the BooleanOption 'useEnablement'. After doing some code searching I figured out PDE evaluated the conditional statements in source files by calling 'getValue(String key)'. Therefore I removed the BooleanOption and extended 'getValue(String key)' to check to see if the key is useEnablement. If it was, it would return a new Boolean(getTargetVersion >= 3.3). The changes were very minor compared to all the work done in the patch. Thanks for the good work.
