Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 358767

Summary:	[commands] commands that collect parameters
Product:	[ECD] Orion	Reporter:	Susan McCourt <susan>
Component:	Client	Assignee:	Susan McCourt <susan>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	P3	CC:	libingw, malgorzata.tomczyk, Mike_Wilson
Version:	unspecified
Target Milestone:	0.4 M1
Hardware:	PC
OS:	Windows 7
Whiteboard:
Bug Depends on:	342739
Bug Blocks:

Description Susan McCourt

2011-09-23 15:13:44 EDT

Bug 342739 has focused on what UI design/technology we would use to collect command parameters in the UI. One thing implied in this discussion, that we haven't said explicitly, is that it would be nice for the command to just say what it needs (how many and what types of parameters) and then the command framework "does the right thing" in the UI depending on the scope of the command, etc., and calls the handler with the correct parameters.

McQ and I discussed this notion today, and I thought this should be put in a separate bug, since working on the various UI tricks for collecting the info is kind of separate from deciding how we specify them in commands.

For example, the find/replace bar implementation essentially collects parameters that used to appear in a dialog. It plays pretty well. But we might like to collect other parameters for page-level commands in that same way rather than have the find/replace implementation provide it. So the command framework rendering code would know that at the page scope, the best way to collect parameters is to expand the dark bar.

But in another context (such as desktop eclipse), we would not want that bar to be the implementation for find/replace, we'd probably want the eclipse dialog. (I don't know if this is the best example because in eclipse most dialogs do the actual work, they aren't just a parameter collecting agent).

Once command parameters were formalized, we could look at a URL structure for, say, opening a page, and then running a command with a parameter. For example, opening a search result might consist of a URL for the editor plus an instruction to run the command "find 'blort'"

Comment 1 Susan McCourt

2011-11-11 14:04:57 EST

There is a raw first cut released now.
When defining a command, you can define parameters that you need to collect.
Then, in globalCommands, we specify a "parameterCollector" that implements a slideout of the banner.

I converted a few commands to use this technique:
- new project, new file, new folder
- editor goto line

I also converted the text find/replace to use the same collector.
In the case of the text searcher, it's a little bit different.  We currently don't define find/replace as a command, it's just an editor binding.  So rather than have the command service get the parameters, the text searcher goes to the command service to get the slideout, and then it generates its own stuff in there.  It is definitely more modularized, in that the text searcher no longer knows dom id's of the banner, toolbar, etc.  And it's a good proof of concept for having custom hooks for using the slideout.

The next steps for this bug are to generalize the parameter definitions and start using it in more commands.  I still need to work a little on this before having others go off and try to use it, as the API for parameters, and whether they are required/optional, and how to hook in optional stuff, is not generalized.

Comment 2 Susan McCourt

2011-11-16 14:38:16 EST

I released some work in bug 359080 to get "clone git repo" working with command parameters.  I think the API needs a good deal of cleanup before we copy the pattern.  Right now the parameter description is an object that is used as an associative array, with some predefined slots used by the command framework.

parameters.options is set to "true" if there are optional parameters to collect (done by the command callback UI)
parameters.optionsTriggered is set by the framework if the user triggers the options button in the slideout.

So the pattern for a command is to define its required parameters as slots in the parameters object, and to set "options" to true if there are optional/advanced parameters. 

var cloneParameters = {};
cloneParameters.url = {label: 'Repository URL:', type: 'url'};
cloneParameters.options = true;

Then, in the callback the command can check to see if the parameter values have been set.
if (cloneParameters.url.value)
And it can decide whether the options dialog needs to be shown.
if (cloneParameters.optionsTriggered)

For adapting preexisting code, it's kind of nice to be able to keep the old dialogs, etc. around, and invoke them if the desired parameter values aren't supplied.  Another approach might be to never call the callback if required parameters aren't supplied.  

For in-page code, I don't see too much harm in the current approach.
I think that when we expose command parameters in the command extensions for plug-ins, we would treat this differently.  We wouldn't want to call a plugin unless we had gathered all required parameters.

There's still lots of work to do before closing this bug:
- haven't played with multiple parameters and how they look in the slideout, maxwidth, etc.
- none of this is documented
- this associative array is pretty weird for API, though convenient to get things running.  We probably want to define a proper class for describing the concepts of options, optionsTriggered, and a paramters array

Comment 3 Susan McCourt

2011-11-22 20:56:21 EST

(In reply to comment #2)
> There's still lots of work to do before closing this bug:

> - haven't played with multiple parameters and how they look in the slideout,
> maxwidth, etc.
This will be addressed in the continuing work in bug 342739, which focuses on the presentation aspects

> - none of this is documented
Wrote a bunch of doc for the various command and parameter classes

> - this associative array is pretty weird for API, though convenient to get
> things running.  We probably want to define a proper class for describing the
> concepts of options, optionsTriggered, and a paramters array
Defined a bunch of new classes for the API part, documented in commands.js.  The lookup table is now hidden inside ParametersDescription, so clients are using good ol' objects to create parameters and describe the options, etc.  See CommandParameter, ParametersDescription, CommandInvocation.

Closing this bug as I think the first cut is now somewhat reasonable.  We can open bugs on new features or changes.