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

Bug 337647

Summary: [client] the "editorContainer" should be a consumable component
Product: [ECD] Orion Reporter: Boris Bokowski <bokowski>
Component: ClientAssignee: Susan McCourt <susan>
Status: RESOLVED FIXED QA Contact:
Severity: enhancement    
Priority: P3 CC: eclipse.felipe, johnjbarton, libingw, Mike_Wilson, pwebster, Silenio_Quarti, susan
Version: 0.2   
Target Milestone: 0.2   
Hardware: PC   
OS: All   
Whiteboard:

Description Boris Bokowski CLA 2011-02-19 13:31:23 EST
The current "editorContainer" is not reusable as a component in its current form. (Perhaps now is the time to find a better name for it, too.)

Questions: What's the interface to this component? The simplest possible interface I can think of is an object you can call setText and getText on. Clients could pass the id of a div in the constructor, or something like that. Clients probably want to be called when the dirty state changes. Anything else? Who would react to Ctrl+S, the editor? What about the outline and the error markers, should that be part of the component?
Comment 1 Boris Bokowski CLA 2011-02-19 14:44:37 EST
Copied from jjb's email:

I also need to highlight a line in the editor and tell the user why the line was 
selected (hit various kinds of breakpoints, errors, links to functions).
Eventually to complete the integration we'll need
   obtain the code under the mouse (for fly-over inspector)
   highlight a range of text,
   get the selected range of source, (for watch evaluation)

> Who would react to Ctrl+S, the editor?
>
If by Ctrl+S you mean save to server,  I think the answer is "the 
editor, but if it can't, don't go nuts". For normal web app dev it seems 
like the editor should save; for prototyping work the ideal case is the 
editor should save and the server should support trivial roll back; for 
exploratory work on sites you don't own the editor can't save but 
Firebug can still apply the changes to the live image.

Separately there is "I'm happy with my editing so far, I want to see how 
well its working". That can be bound to Ctrl+S.  But it also could be 
more or less aggressive, and it could be a separate control. The Java 
hot loading solution in eclipse binds it to save so that is where to start.

> What about the outline and the error markers, would you expect that to 
> be part of the component?
>
Yes, I believe that is what users will want. However, one of my goals 
however is it make error markers obsolete. I want to stop and fix errors 
immediately, not create a list of them, fix them, then retry.
Comment 2 Susan McCourt CLA 2011-02-20 19:58:39 EST
I'm running into similar requirements as John's while trying to separate editor commands from editorContainer. 

This is assuming that all editor commands (even modal ones such as incremental search) should be implementable using only API.  It sounds as if we need API that lets clients access the underlying editor so we can use its API.  I think most of what John is asking for is implemented already at the editor level.

I'm just starting to move all this code around so that the editor commands/key bindings are defined via the command service.  So I can start moving the API in this direction.
Comment 3 Susan McCourt CLA 2011-02-20 20:16:40 EST
Boris, I'm going to assign this bug to myself unless you had something else in mind.  I imagine that the editor component->editor relationship is akin to Jface->SWT.  We expose the underlying editor so that you can use its API if needed, but the function provided above and beyond the widget is accessible via the component API.  

We can also assume for a start that user notification occurs in the status line, but it might be nice to have an API for notifying the editor user about stuff so that we can change our mind later (for example, a popup that is located somewhere adjacent to editor text, etc.)
Comment 4 Susan McCourt CLA 2011-02-21 12:14:32 EST
Boris and I talked over IRC, capturing some salient points:

- we agree clients should be able to have access to the editor for editor-level API like selections, locations, etc.
- we agree it's a good thing to provide a notify API so we can hide the service registry/status service and also allow ourselves to change our minds about how notification works.  (Status is probably something pluggable because in jjb's case the status service is not there.)
- we agree that commands should be API citizens.  We need a notion of a mode with respect to keystrokes, because the modal commands, such as incremental find, wil want to own the keystrokes until incremental search is done.  Boris was thinking of some kind of simple state machine that's part of the editor/key binding API
- an open question is how much glue is needed to get things like undo stack, rulers, content assist.  I think that the current glue is too low-level, because assembling all the rulers, undo stack, etc. seems too complicated.  Boris pointed out that some of the features should be optional, and for each feature (undo, code assist, etc.) there is the implementation and usually some corresponding key bindings. So the client may still have to instantiate "undo support" or "problem rulers" etc. but these "features" could do the heavy lifting and register the keybindings, etc.
Comment 5 Susan McCourt CLA 2011-02-22 01:24:08 EST
I've started some of this work.

There is now a file called "editorFeatures.js" in which I'm starting to put various "features" of the editor, such as generation of basic commands, creation of undo stack and related bindings/commands, etc.  

The intention is that a user of editorContainer can supply features(such as an undo stack factory, content assist factory, command generator, etc.), but these are optional.  editorContainer will not assume that all these features are there, but will conditionally use them.

I've still got a lot more work to do, because as part of this work, I want to pull out all those keybindings that are now in editorContainer and distribute them across the appropriate features.  This also means dealing with the modal bindings.

What I've done so far:
- define an optional command generator that generates basic commands (such as save) as well as traversing the plug-in registry for the editor action commands.   This generator could be replaced with something different in scenarios where the editor isn't being used alongside all the various services.
- the undo stack factory now registers the undo/redo commands rather than have the editor container do so.  

Next steps:
- divide more stuff into features:  problem rulers, content assist, etc.
- decide how remaining keybindings are divided amongst the features (a good start is separating plain text stuff such as undo from source code-related stuff)
- come up with a decent story for the modal keybindings
- once this is all done, we could conceivably provide an editor context menu for the various commands
Comment 6 Susan McCourt CLA 2011-03-01 19:10:23 EST
I did a little work here, but we decided on the last Orion call that feature work is important.  I'll pick this up during M7.
Comment 7 Susan McCourt CLA 2011-03-18 18:48:03 EDT
*** Bug 340479 has been marked as a duplicate of this bug. ***
Comment 8 Susan McCourt CLA 2011-03-24 20:15:39 EDT
Marking fixed, as the major refactoring of editorContainer is done.  I'm sure we will continue to refine it, but it's a good start, and I'd like to let the Firebug integration and Libing's compare work drive how this evolves.  To answer the original questions posed by Boris:

(In reply to comment #0)
> Questions: What's the interface to this component? The simplest possible
> interface I can think of is an object you can call setText and getText on.
Per previous discussions, we decided to expose the underlying widget, so clients can get the widget and perform the low-level operations, including getText and setText

> Clients could pass the id of a div in the constructor, or something like that.
Clients now only pass in the div and various objects that allow customizing of behaviors such as:
- undo stack and associated bindings or toolbar commands
- content assist
- plain text keybindings, source code-aware keybindings
- editor creation parameters
- how status is reported
- optional creation of line number and annotation rulers

> Clients probably want to be called when the dirty state changes. Anything else?

We have notifications for input change and dirty state change.

> Who would react to Ctrl+S, the editor? 
Nope.  Since the editor doesn't know how/where the contents will be saved, it's up to clients to hook the keybinding or provide UI for saving.  We are now doing this in our glue code.

> What about the outline and the error
> markers, should that be part of the component?
The outline is not part of the component.  It's simple enough to create one if you need it, and the updating of the editor will happen via the input service.

Error markers are provided with an AnnotationFactory which sets up the rulers and can show problems.  The glue code decides how problems are identified and calls the AnnotationFactory to show the markers.

All knowledge of the containing page, such as status area placement, use of breadcrumbs, how to show dirty status, etc....is now handled outside of edtiorContainer.

I built a sample called
http://localhost:8080/examples/minimaleditor.html

This sample shows how to instantiate an editorContainer without needing the service registry, etc., but still providing custom behavior for save, problem markers, etc.