Bug 109569

I think Jan is handling template issues for 6.0, right?

I can't check in my freemarker changes without this fix. 

Here's how I worked around this locally while waiting for a fix:

Index: openide/loaders/src/org/openide/loaders/DataObject.java
===================================================================
RCS file: /cvs/openide/loaders/src/org/openide/loaders/DataObject.java,v
retrieving revision 1.38
diff -u -u -r1.38 DataObject.java
--- openide/loaders/src/org/openide/loaders/DataObject.java     19 May 2007 03:38:13 -0000      1.38
+++ openide/loaders/src/org/openide/loaders/DataObject.java     17 Jul 2007 18:14:52 -0000
@@ -1229,6 +1229,10 @@
             }
             if (c.param != null) {
                 for (Map.Entry<String,? extends Object> e : c.param.entrySet()) {
+                    // Workaround for #109569
+                    if (e.getKey().equals("project")) {
+                        continue;
+                    }
                     all.put(e.getKey(), e.getValue());
                 }
             }

This patch just avoids picking up the "project" descriptor property when building the freemarker property set.

If the proper fix (e.g. changing WIZARD_KEY_PROJECT's value) is tricky - would you mind temporarily applying the above patch in the mean time?

I am definitely not handling template issues for 6.0. In order to be able to migrate java templates to freemarker I just
added ProjectTemplateAttributesProvider to collect template attributes from the particular project type.

But to your issue. Your Ruby project should register own CreateFromTemplateAttributesProvider impl in its lookup
providing project.license and possibly other attributes you need in ruby templates. See e.g.
org.netbeans.modules.java.JavaTemplateAttributesProvider. It has no connection to WizardDescriptor properties by default.

I have not any idea how you can get all those attributes for RHTML except you use your own "new file" wizard or
iterator. Looking at the default one org.netbeans.modules.project.ui.NewFileWizard and its NewFileIterator I cannot find
anything suspicious like DataObject.createFromTemplate(DataFolder,String,Map) that would pass attributes of the wizard
descriptor to freemarker as you mentioned.

It's happening right in the diff I submitted yesterday.   But to make it a lot clearer, here's how the default TemplateWizard calls 
DataObject.createFromTemplate:

org.openide.loaders.TemplateWizard:
            DataObject obj = template.createFromTemplate (folder, n, wiz.getProperties());

In other words, the TemplateWizard.DefaultIterator passes the wizard's descriptor properties on to DataObject createFromTemplate.

Then, in DataObject's createFromTemplate, the findParameters method builds up the parameters to pass to Freemarker. It -first- works its way through all 
the CreateFromTemplateAttributeProviders.  In my case this DOES add a project.license=default property - not because my own project provided one, but 
because there is a default provider - ProjectTemplateAttributesProvider's checkLicense method (which you wrote, so I'm sure you knew that :).

        public static Map<String,Object> findParameters(String name) {
            CreateAction c  = CURRENT.get();
            if (c == null) {
                return Collections.emptyMap();
            }
            HashMap<String,Object> all = new HashMap<String,Object>();
            for (CreateFromTemplateAttributesProvider provider : Lookup.getDefault().lookupAll(CreateFromTemplateAttributesProvider.class)) {
                Map<String,? extends Object> map = provider.attributesFor(c.orig, c.f, c.name);
                if (map != null) {
                    for (Map.Entry<String,? extends Object> e : map.entrySet()) {
                        all.put(e.getKey(), e.getValue());
                    }
                }
            }

It THEN goes through all the properties passed to the CreateAction and adds these into the map; these are the wizard properties that were passed to 
createFromTemplate above:

            if (c.param != null) {
                for (Map.Entry<String,? extends Object> e : c.param.entrySet()) {
                    all.put(e.getKey(), e.getValue());
                }
            }

This is where "project" gets remapped from a map (which contains "license"=>"default") to a reference to an actual project instance.

My workaround simply removes anything called "project" from the create properties since we know this is now defined as a part of the API for templates - 
templates can contain "${project}" so create maps had better not contain this symbol and overwrite it.

            if (c.param != null) {
                for (Map.Entry<String,? extends Object> e : c.param.entrySet()) {
                    // Workaround for #109569
                    if (e.getKey().equals("project")) {
                        continue;
                    }
                    all.put(e.getKey(), e.getValue());
                }
            }

This wouldn't be necessary if WIZARD_KEY_PROJECT = "Wizard_project" or something like that.

An alternative fix would be to modify the findParameters method to ONLY add create properties that are not already in the map, to avoid any accidental 
overrides. In other words, CreateFromAttributeTemplateProvider properties should always "win".

            if (c.param != null) {
                for (Map.Entry<String,? extends Object> e : c.param.entrySet()) {
                    if (!all.containsKey(e.getKey()) {
                        all.put(e.getKey(), e.getValue());
                    }
                }
            }

Sorry, I was not aware of TemplateWizard: template.createFromTemplate(folder, n, wiz.getProperties());
since my Find Usages action throws NPEs so I searched just the Project UI module. Neither the project default iterator
or the java project iterator use template.createFromTemplate(folder, n, wiz.getProperties())

to Jarda: why TemplateWizard.DefaultIterator translates wizard descriptor properties to template attributes
automatically? I have not been aware of such a contract.

I do not like the idea to change WIZARD_KEY_PROJECT content since it would be an incompatible API change.

IMO the proper and simplest solution would be to stop translate wizard descriptor properties to template attributes
automatically in TemplateWizard.DefaultIterator. The Datasystems should also describe merging algorithm in API.

Reassigning to jtulach who wrote the scripting support to datasystems.

btw Freemarker permits following datamodel

(root)
 |
 +- mouse = "Yerri"
     |
     +- age = 12
     |
     +- color = "brown"

So merging project = <instance> and project.license = "default" attributes in findParameters(String name) should not
result in any conflict ;-)

By the way, passing in the wizard descriptors to the freemarker properties is really handy in my scenario. In my Ruby New file wizard I'm inputting a 
number of different (optional) parameters from the user -- class name, surrounding module, and an optional superclass. These just get stored as string 
attributes in the wizard descriptor - but also get inherited as parameters for the template. 

And my freemarker templates take advantage of this; here's the freemarker template which builds up a class definition, using the parameters "class", 
"module", and "extends" parameters from the template:

<#-- This is a FreeMarker template -->
<#assign licenseFirst = "# ">
<#assign licensePrefix = "# ">
<#assign licenseLast = " ">
<#include "../Licenses/license-${project.license}.txt">

<#assign indent = "">
<#-- If the "module" parameter is set, emit a series of module Name lines -->
<#if module?? && module != "">
<#assign modulelist = module?split("::")>
<#list modulelist as modulename>
${indent}module ${modulename}
<#assign indent = indent + "  ">
</#list>
</#if>
<#-- If the "extend" parameter is set, add < Superclass to the class definition -->
${indent}class ${class}<#if extend?? && extend != ""> < ${extend}</#if>
${indent}  def initialize
${indent}
${indent}  end
${indent}end
<#if modulelist??>
<#list modulelist as x>
<#assign indent = indent?substring(2)>
${indent}end
</#list>
</#if>

I was trying to fix the double registration of project and project.license, but I failed. FreeMarker documentation is 
quite about this corner case.

That is why I suggest to fix this in projects. In addition to already existing project.license, let's also define 
project_license and let Tor change his script to use this new variable.

Jan, do you think this is acceptable? Can you do it?

I have to repeat my question. Why does TemplateWizard.DefaultIterator translate wizard descriptor properties to template
attributes automatically? Where is this contract described?

Present template attributes that are shared by others go through the API review process. WD properties are a kind of
private stuff. Merging them by default in openide is a bug IMO. Only clients can decide (in custom
TemplateWizard.Iterator) how to resolve possible conflicts for their templates.

Having 2 same attributes ${project.license} and ${project_license} in API does not seem to me as a right solution. If
you do not want to resolve this in openide I would propose to replace ${project.license} with ${project_license} in API
and all its usages. nb6.0 has not been released yet so it should be still possible.

Re: Where is the contract described?
http://www.netbeans.org/source/browse/openide/loaders/test/unit/src/org/openide/loaders/CreateFromTemplateHandlerTest.java?r1=1.1.2.1&r2=1.1.2.2
as the comment says, I am not sure if this is really blessed behaviour, but at least Tor found it convenient.

I am fine with changing the project.license to project_license, but please finish this before beta1. Still we may 
encounter compatibility problems for people who modified their templates in their userdir...

The test is nice but templates documentation does not mention it at all. I hate such a black magic in architecture but
if you are willing to fix future clashes ...

I will attach the patch with the new project_licence template attribute.

Created attachment 46074 [details]
patch

I would like to ask for fast API review in order to rename the template attribute from ${project.license} to
${project_license}.

While I have no problem with the rename of project.license to project_license as such, I agree with Jan that the real
problem is the automagical merging of all TemplateWizard attributes into the variable map:

  template.createFromTemplate(folder, n, wiz.getProperties())

in TemplateWizard. This pollutes the macro namespace with totally irrelevant and probably undocumented variables and
looks wrong to me.

I would rather suggest that if Tor wants a place to keep added variables introduced in various wizard steps, he use
CreateFromTemplateAttributesProvider - isn't this what everyone else does, e.g. for Java templates? Can someone explain
from scratch why we need to have WizardDescriptor attributes dumped into the variable map?

Or if it is desirable to have WD attrs automatically accessible to any template, can they at least be placed under a
single obvious key like 'wizard', e.g.

  template.createFromTemplate(folder, n, Collections.singletonMap('wizard', wiz.getProperties()))

(so that they can be accessed using ${wizard.something} if desired) without risking conflicts with other unrelated
variables?

${wizard.foo} looks convenient.

OK, I take my request back. Reassigning to Jarda who is the author of dumping wizard properties to explain/resolve this.
${wizard.*} is an elegant solution and I favor it instead of renaming ${project.license} if ${wizard.*} is really necessary.

wizard.* looks very good. Thanks for the idea.

IDE:-------------------------------------------------
IDE: [14.8.07 14:11] Committing "Datasystems API" started
Checking in src/org/openide/loaders/TemplateWizard.java;
/shared/data/ccvs/repository/openide/loaders/src/org/openide/loaders/TemplateWizard.java,v  <--  TemplateWizard.java
new revision: 1.28; previous revision: 1.27
done
Checking in src/org/openide/loaders/DataObject.java;
/shared/data/ccvs/repository/openide/loaders/src/org/openide/loaders/DataObject.java,v  <--  DataObject.java
new revision: 1.40; previous revision: 1.39
done
Checking in api/apichanges.xml;
/shared/data/ccvs/repository/openide/loaders/api/apichanges.xml,v  <--  apichanges.xml
new revision: 1.33; previous revision: 1.32
done
Checking in test/unit/src/org/openide/loaders/CreateFromTemplateHandlerTest.java;
/shared/data/ccvs/repository/openide/loaders/test/unit/src/org/openide/loaders/CreateFromTemplateHandlerTest.java,v  
<--  CreateFromTemplateHandlerTest.java
new revision: 1.6; previous revision: 1.5
done
Checking in manifest.mf;
/shared/data/ccvs/repository/openide/loaders/manifest.mf,v  <--  manifest.mf
new revision: 1.37; previous revision: 1.36
done
IDE: [14.8.07 14:11] Committing "Datasystems API" finished