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.

Bug 217632 - Client Project SPI should support Template customiser
Summary: Client Project SPI should support Template customiser
Status: VERIFIED FIXED
Alias: None
Product: web
Classification: Unclassified
Component: HTML Project (show other bugs)
Version: 7.3
Hardware: All All
: P2 normal (vote)
Assignee: Jan Becicka
URL:
Keywords:
Depends on:
Blocks: 217553
  Show dependency tree
 
Reported: 2012-08-30 14:01 UTC by Jan Becicka
Modified: 2012-10-29 08:25 UTC (History)
4 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments
Suggested patch. (23.44 KB, patch)
2012-10-16 13:58 UTC, Jan Becicka
Details | Diff
Second version of patch (without checkbox) (29.61 KB, patch)
2012-10-17 12:36 UTC, Jan Becicka
Details | Diff
hg diff --git (45.56 KB, patch)
2012-10-17 13:21 UTC, Jan Becicka
Details | Diff
More general solution (44.47 KB, patch)
2012-10-17 19:59 UTC, Jan Becicka
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Becicka 2012-08-30 14:01:17 UTC
Client Project SPI allows to register templates to "New Wizard". Unfortunately these templates cannot be customised. Please make a possibility for templates to insert extra step into the wizard. There already was a prototype for such SPI (TemplateCustomizer interface).
Comment 1 David Konecny 2012-09-02 22:09:13 UTC
Tomas, I think we talked about this briefly a while ago. Could you make a proposal how we could resolve this? Or would you prefer me to do it?
Comment 2 Tomas Mysik 2012-09-03 09:05:49 UTC
Well, I can easily return back the customizer to the SPI. But currently:
(a) there is no space for it in the New Project wizard;
(b) it cannot be customized later in Project Properties dialog (site template is not stored in project properties).

Do we need any UI spec for (a)? I guess so... Also, do we want to support (b) too?

Personally, I would perhaps prefer to add it to the current site-template-selection step - site template customizers (archive & online) could be automatically shown/hidden once they are selected/deselected.

Thanks.
Comment 3 Petr Jiricka 2012-09-03 13:27:48 UTC
I believe this request is motivated by comment 2 in issue 217553#c2 (which is blocked by this request), i.e. we'd like to restore the PhoneGap functionality, which previously inserted an extra step if you selected the Cordova template. 

However, I am not sure if we still want this - we also discussed that it should be possible to enable+configure PhoneGap for all HTML5 projects, regardless of template. So this customization UI should not be template-specific, it should be template-agnostic. I.e. if the PhoneGap plugin is installed, there should always be the ability to enable PhoneGap when creating a HTML5 project. (The only thing that may be template-specific is that for the Cordova template, we may want to enable PhoneGap by default, for the others it would be off by default.)
Comment 4 David Konecny 2012-09-07 02:16:04 UTC
Easiest would be to just resurrect the old SPI and if a template has customizer add it into wizard as additional step named according to template. No UI spec needed.

But maybe there are better ways to handle PhoneGap support completely. PhoneGap could have its own wizard which would result into preconfigured clientside project and in such wizard PhoneGap support could do anything what's needed.

We should also cover a scenario where user opened an existing project which already has cordova.js and we should "automatically" detect that and enable our PhoneGap support.
Comment 5 Tomas Mysik 2012-09-07 03:50:22 UTC
Sorry, I do not know PhoneGap at all - Honzo, does it make sense what David wrote? If yes, please reassign to yourself. If not, I can improve the New Project wizard [1].

Thanks.
[1] But if PhoneGap can be added to any project with any site template (I don't know that), then it make sense to me that PhoneGap is not site template at all.
Comment 6 Petr Jiricka 2012-09-07 09:36:17 UTC
> Easiest would be to just resurrect the old SPI and if a template has customizer
> add it into wizard as additional step

I agree that's easy, but I don't think that's correct. I am sure we discussed the requirement to create a PhoneGap project using other templates than just the Cordova template. For example, the Mobile Boilerplate template probably makes sense with PhoneGap.

> PhoneGap could have its own wizard which would result into preconfigured 
> clientside project

Yes, that's a possibility. So we would have the following wizards:

HTML5 Application
PhoneGap Application
HTML5/PhoneGap Application with Existing Sources (this would either create an ordinary project or a PhoneGap-enabled project depending on what it finds in the sources)

But then the PhoneGap application wizard would still want to reuse most of the HTML5 Application UI, right? Is that doable right now?

> But if PhoneGap can be added to any project with any site template, 
> then it make sense to me that PhoneGap is not site template at all.

I don't agree, the current Cordova template has some nice sample code, so it makes sense to provide it. I'd say that some templates only make sense with PhoneGap (like Cordova template), some make sense with both PhoneGap and non-PhoneGap projects, and there may be some that only make sense with non-PhoneGap.
Comment 7 David Konecny 2012-09-10 08:59:35 UTC
Here is my proposal:

Let's add to "Name and Location" panel of new "HTML5 Application" wizard checkbox "Enable PhoneGap". Let's define a new SPI in web.clientproject.api which would get implemented by NetBeans PhoneGap support and places in the global lookup. The SPI could be very similar to SiteTemplateImplementation but in addition it would have "getWizardPanel" and returned wizard panel would be automatically added to New HTML5 Application wizard if user ticked Enable PhoneGap. The SPI would allow to customize project similarly like SiteTemplateImplementation allows. That would cover project creation and would allow to pick any existing template and still add to it PhoneGap support.

For New HTML5 from Existing Sources we do not need to do anything - phonegap support is either already there or not. PhoneGap presence should be detected automagically.

For projects created without PhoneGap there could be either an action in project's popup menu saying "Enable PhoneGap" or a panel in project properties labelled "PhoneGap" which would contains checkbox "Enable". This part would not require any new API.

Would this work? And resovled all requirements?
Comment 8 Jan Becicka 2012-10-11 14:41:42 UTC
It should work.
Comment 9 Jan Becicka 2012-10-16 13:58:46 UTC
Created attachment 126018 [details]
Suggested patch.
Comment 10 David Konecny 2012-10-16 22:16:17 UTC
Thanks Honza.

Tomas, what do you think about the patch?

Overall it looks good to me. Few comments:

DK01 - the checkbox to enable Cordova should be in NewClientSideProject and not SiteTemplateWizard

DK02 - I do not think API needs CordovaSupport.createCordovaSupportCheckBox. Could we remove it?

DK03 - I'm not sure about CordovaSupport.isCordovaTemplate. How are you going to implement it? What is going to happen if user has a custom site template for example? Will you just scan for presence of cordova.js in it and if positive make sure that Cordova is properly configured in the IDE?
Comment 11 Jan Becicka 2012-10-17 08:39:12 UTC
DK01:
I don't think so. Cordova Support is additional stuff for existing templates.

DK02:
If 
[ ] Cordova Support
checkbox is checked and Mobile Platforms are not set, Options dialog is invoked to allow such setup.
So the functionality is needed, but the API can me improved. That's clear.
Will try to improve it a bit.

DK03:
Current impl. just checks, that template is provided by cordova module.
Comment 12 Jan Becicka 2012-10-17 12:36:59 UTC
Created attachment 126074 [details]
Second version of patch (without checkbox)
Comment 13 Tomas Mysik 2012-10-17 13:11:31 UTC
Unfortunately, I cannot apply the patch [1] (not sure why, maybe because it is created by NetBeans and not Mercurial?). I will have a look at it directly and provide my comments soon.

[1]
gapon@cattie ~/worx/sun/nb-main $ hg patch ~/cordova.patch 
applying /home/gapon/cordova.patch
unable to find 'nbproject/project.xml' for patching
2 out of 2 hunks FAILED -- saving rejects to file nbproject/project.xml.rej
abort: patch failed to apply
Comment 14 Tomas Mysik 2012-10-17 13:15:10 UTC
(In reply to comment #13)
> it is created by NetBeans and not Mercurial

This is the reason, please, next time use Mercurial as it is more common. Thanks.
Comment 15 Jan Becicka 2012-10-17 13:21:57 UTC
Created attachment 126078 [details]
hg diff --git
Comment 16 Jan Becicka 2012-10-17 13:23:29 UTC
(In reply to comment #14)
> (In reply to comment #13)
> > it is created by NetBeans and not Mercurial
> 
> This is the reason, please, next time use Mercurial as it is more common.
> Thanks.

How is that patch created via netbeans is not usable? Tomas S.?
Comment 17 Tomas Stupka 2012-10-17 13:33:18 UTC
(In reply to comment #16)
> (In reply to comment #14)
> > (In reply to comment #13)
> > > it is created by NetBeans and not Mercurial
> > 
> > This is the reason, please, next time use Mercurial as it is more common.
> > Thanks.
> 
> How is that patch created via netbeans is not usable? Tomas S.?
are we talking about "Second version of patch (without checkbox) "? because i was able to apply on the current state from trunk.
Comment 18 Tomas Mysik 2012-10-17 14:32:24 UTC
TM01: Personally, I would prefer to have more general support. It seems to me that Cordova is kind of HTML5 project "extender" - it would be great if we could be prepared to have more such "extenders" in the default Lookup. Also, I would guess that the code e.g. in the wizardIterator could be then more general [1].

[1] e.g.: for (Extender e : allExtenders_or_appliedExtenders) { e.doSomething() }
Comment 19 Petr Jiricka 2012-10-17 16:22:27 UTC
> Personally, I would prefer to have more general support.

I don't know - I would agree if this was a public API, but since this is friend API, will it not be easy to generalize it in the future and change all the clients (currently one) if a need arises? Also, with the general API, how would we merge the pieces of UI provided by various extenders - will that be doable?
Comment 20 Jan Becicka 2012-10-17 19:59:57 UTC
Created attachment 126114 [details]
More general solution
Comment 21 David Konecny 2012-10-18 01:05:31 UTC
(In reply to comment #11)
> DK01:
> I don't think so. Cordova Support is additional stuff for existing templates.

That's not what I proposed in comment #7. I do not see a connection between "Cordova checbox" and "site template" - these two are orthogonal (ie. I can enable cordova on empty project or on a completely different site template like 'Initializr: Classic'). That's why I would want to keep Cordova checkbox on the first wizard step. In a way your "Cordova Site Template" is not needed anymore - you now have ability to enhance new wizard with your own dedicated wizard panel and in apply() method do whatever you want, eg. add cordova files/configurations to a project being created.

> DK02:
> If 
> [ ] Cordova Support
> checkbox is checked and Mobile Platforms are not set, Options dialog is invoked
> to allow such setup.
> So the functionality is needed, but the API can me improved. That's clear.
> Will try to improve it a bit.

But that's what ClientProjectExtender.createWizardPanel is for, no? You can use it to add a new panel to New Project wizard and in that panel you can ensure that Mobile Platforms are configured properly (directly in the panel or via "Configure Platforms" button).

I would probably change ClientProjectExtender.createWizardPanel() to return array of "Panel<WizardDescriptor>".

So I think two methods:
+    public Panel<WizardDescriptor> createWizardPanel();
+    public void apply(FileObject projectRoot, FileObject siteRoot, FileObject libsFolder);
is all what's needed in ClientProjectExtender for now.

Re. "more general support" - if we go this way we have to present project extenders somewhere somehow similarly like "Web Frameworks" in Java Web Project. For 7.4 maybe; for 7.3 I would say only single (Cordova) implementation is supported.
Comment 22 Tomas Mysik 2012-10-18 06:48:36 UTC
TM01: Clarification - I just wanted to replace "ugly" isCordova() etc. methods with "nicer" (and more general) code, that's the reason why I wrote "I would prefer". If we add more "extenders" in the future, the API definitely needs to be changed/improved - but this is totally fine and expected.
Comment 23 Jan Becicka 2012-10-18 13:26:30 UTC
(In reply to comment #21)
> (In reply to comment #11)
> > DK01:
> > I don't think so. Cordova Support is additional stuff for existing templates.
> 
> That's not what I proposed in comment #7. I do not see a connection between
> "Cordova checbox" and "site template" - these two are orthogonal (ie. I can
> enable cordova on empty project or on a completely different site template like
> 'Initializr: Classic'). That's why I would want to keep Cordova checkbox on the
> first wizard step. In a way your "Cordova Site Template" is not needed anymore
> - you now have ability to enhance new wizard with your own dedicated wizard
> panel and in apply() method do whatever you want, eg. add cordova
> files/configurations to a project being created.

Well, I see no connection between Name & Location and Cordova.

> 
> > DK02:
> > If 
> > [ ] Cordova Support
> > checkbox is checked and Mobile Platforms are not set, Options dialog is invoked
> > to allow such setup.
> > So the functionality is needed, but the API can me improved. That's clear.
> > Will try to improve it a bit.
> 
> But that's what ClientProjectExtender.createWizardPanel is for, no? You can use
> it to add a new panel to New Project wizard and in that panel you can ensure
> that Mobile Platforms are configured properly (directly in the panel or via
> "Configure Platforms" button).
> 
> I would probably change ClientProjectExtender.createWizardPanel() to return
> array of "Panel<WizardDescriptor>".
> 
> So I think two methods:
> +    public Panel<WizardDescriptor> createWizardPanel();
> +    public void apply(FileObject projectRoot, FileObject siteRoot, FileObject
> libsFolder);
> is all what's needed in ClientProjectExtender for now.
> 
> Re. "more general support" - if we go this way we have to present project
> extenders somewhere somehow similarly like "Web Frameworks" in Java Web
> Project. For 7.4 maybe; for 7.3 I would say only single (Cordova)
> implementation is supported.

Please do whatever you want to do. I implemented solution, which is enough for me. Feel free to change it. It is your module ;)

changeset   : 236426:f70d033d7b0b
author      : Jan Becicka <jbecicka@netbeans.org>
date        : Thu Oct 18 15:06:36 CEST 2012
summary     : #217632: Client Project SPI should support Template customiser
Comment 24 Quality Engineering 2012-10-19 13:42:32 UTC
Integrated into 'main-golden', will be available in build *201210191216* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/f70d033d7b0b
User: Jan Becicka <jbecicka@netbeans.org>
Log: #217632: Client Project SPI should support Template customiser
Comment 25 David Konecny 2012-10-24 01:32:50 UTC
(In reply to comment #23)
> Please do whatever you want to do. I implemented solution, which is enough for
> me. Feel free to change it. It is your module ;)

I do not mind helping but the way it is right now is a mess and needs more work from you first:
* Cordova wizard panel is added as the last step? why? If user indicated that they want to use Cordova then the cordova panel should be mandatory (not optional as now) and should guide user to setup mobile platforms and everything else necessary for the project
* The codrova wizard step is not shown in the list of steps
* what's the "Setup" button about? and why is it on Site templates panel? It should be part of your wizard returned by ClientProjectExtender.createWizardPanel()
* following methods should be deleted from ClientProjectExtender because they are not needed to "extend project": isExtenderRequired, openOptionsDialog, isExtenderReady, getDisplayName

I do not insist that API needs to be the way I proposed it in comment #7 but you said it is OK and that's what I assume we are implementing. Current implementation in trunk is not "ClientProjectExtender" but some sort of SiteTemplateExtender in disguise. If that's what we want to do then we should be enhancing directly SiteTemplateImplementation and not call it ClientProjectExtender.

Btw. site templates were meant to be simple archive with files to unzip. Nothing else; no logic. That way if user creates their own site template ZIP files everything will just work. In your CordovaTemplate class you decided to implement site template dynamically which is doable but consequently you have the problem that you need Cordova SDK otherwise your site templates cannot work. Which is a root problem of most of your troubles and the need for extra site template SPI. If you instead turned that Cordova sample project into a ZIP and bundle this ZIP with your module you would not have any of these troubles. That would be also compatible with user's custom "cordova" templates - just bunch of files. Job of CordovaExtender would be also clearer and simpler: just setup project configurations, global mobile platforms and that's it. But it would not touch user sources nor libraries - that's a job of site template. And if user created a project with Cordova site template but did not enable Cordova Support? well that's the same problem which we have to deal with when user creates a new project from existing sources - they will have to enable Cordova Support via project properties; or will be given hint in editor on reference to "cordova.js" or on first execution or something else.
Comment 26 Petr Jiricka 2012-10-24 07:45:14 UTC
One other problem:
* When I enable Cordova, cordova.js is not listed in the JS libraries step, when in fact it will be added to the sources.
Comment 27 Jan Becicka 2012-10-26 13:45:00 UTC
OK. Lets do it simple. PhoneGap will not be integrated into current panels, but will always add one more panel where it can be enabled.

The API can be simplified to requested form:
+    public Panel<WizardDescriptor>[] createWizardPanel()s;
+    public void apply(FileObject projectRoot, FileObject siteRoot, FileObject
libsFolder);

changeset   : 236945:31927a92c333
author      : Jan Becicka <jbecicka@netbeans.org>
date        : Fri Oct 26 14:34:47 CEST 2012
summary     : Issue #217632 - Client Project SPI should support Template customiser
Comment 28 Quality Engineering 2012-10-27 01:34:55 UTC
Integrated into 'main-golden', will be available in build *201210270001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/31927a92c333
User: Jan Becicka <jbecicka@netbeans.org>
Log: Issue #217632 - Client Project SPI should support Template customiser
Comment 29 David Konecny 2012-10-29 08:25:31 UTC
Thanks Honza, API looks good to me now. Any implementation issues should be filed as new issues.