Skip to main content

Notice: this Wiki will be going read only early in 2024 and edits will no longer be possible. Please see: https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/wikis/Wiki-shutdown-plan for the plan.

Jump to: navigation, search

Difference between revisions of "Compare"

(Updated with state as of 3.3 M3)
m (Navigation)
Line 75: Line 75:
 
It would be good if we could condense this into a consistent unified navigation behavior.
 
It would be good if we could condense this into a consistent unified navigation behavior.
  
There are several bugs on this matter: bugs [https://bugs.eclipse.org/bugs/show_bug.cgi?id=143858 143858], [https://bugs.eclipse.org/bugs/show_bug.cgi?id=36710 36710], [https://bugs.eclipse.org/bugs/show_bug.cgi?id=143855 143855]
+
There are several bugs on this matter: bugs [https://bugs.eclipse.org/bugs/show_bug.cgi?id=143858 143858], [https://bugs.eclipse.org/bugs/show_bug.cgi?id=36710 36710], [https://bugs.eclipse.org/bugs/show_bug.cgi?id=143855 143855]. This is targetted to be addressed in 3.3 M4.
  
 
== ContentMergeViewer Extendability ==
 
== ContentMergeViewer Extendability ==
  
 
There are several package private methods on ContentMergeViewer that would be helpful for subclasses to be able to override. A case in point is the TextMergeViewer that does this already because it is in the same package (see bug [https://bugs.eclipse.org/bugs/show_bug.cgi?id=144638 144638]). In 3.3 M3, changes were made to ContentMergeViewer to surface as API the methods that were required by TextMergeViewer.
 
There are several package private methods on ContentMergeViewer that would be helpful for subclasses to be able to override. A case in point is the TextMergeViewer that does this already because it is in the same package (see bug [https://bugs.eclipse.org/bugs/show_bug.cgi?id=144638 144638]). In 3.3 M3, changes were made to ContentMergeViewer to surface as API the methods that were required by TextMergeViewer.

Revision as of 08:51, 27 October 2006

In the 3.3 release, we are planning on working on some of the outstanding defects that have accumulated for the Compare component. Some of the areas we plan on looking at are:

  • Asynchronous loading and update of compare editors
  • Improving the save life-cycling
  • Improving difference and change Navigation
  • Use of text buffers when comparing local content
  • Upgrade to use the new Commands/Key bindings framework
  • Switch to using the History view for local history operations (Compare/Replace/Restore)

The following sections outline some thoughts on each of these areas.

Asynchronous Loading

Currently, the compare editor input is prepared in the foreground before the editor is opened. Given that this operation may involve server round-trips and difference calculations, it would be bnefitial if it could be done in the background.

Save Life-cycle

Saving properly in the Compare framework has always been a bit of a black art. See bug 125538 for a description of one way of making it work. The reason for this is the layered nature of the Compare framework. For a local file, the layers are:

  1. The file (instance of IFile)
  2. The ResourceNode (instance of IEditableContent)
  3. The TextMergeViewer
  4. The CompareEditorInput
  5. The CompareEditor or CompareDialog

Assume you have an merge viewer open and edit the left side. A save is triggered by a Save operation or selecting another element to edit. What happens in this case is the following:

  1. Save is called on ContentMergeViewer
  2. Save obtains the content from the left side of the content merge viewer and passes it to the viewer content provider (instance of MergeViewerContentProvider).
  3. The content provider calls saveContent on the left typed element of the compare input (if the compare input is an IEditableContent).
  4. the ResourceNode updates its buffer to contain the new content and fire a change event
  5. if you want the file to be saved, you need to listen to that event and call commit on the ResourceDiffNode. For Team, we implement the save behavior in a couple of places. In the old-style syncs, it was a subclass of CompareEditorInput (SyncInfoCompareInput) that listened to the contet change and performed the commit (i.e. the method on ResourceNode that saves). For the new-style sync, we have a class associated with the resource (ResourceSaveableComparison) that performs the save.

There are a couple of observations that can be made about this:

  • The ContentMergeViewer portion of the above only works with byte oriented content.
  • Both the document in the TextMergeViewer and the ResourceNode buffer the contents of the file (that means there are at least two in-memory versions of the file for each side (and the ancestor if there is one).
    • I have entered bug 150526 to request that the use of BufferedContent be avoided for remote files.
    • I have reopened bug 114 to consider linking resource nodes (and also possibly IStorage nodes) with underlying file buffers so that in-memory content is shared between editors.
  • the save method of ContentMergeViewer is really a flush.
    • However most client (e.g. Team Synchronize) treat the flush as an indication that a save should occur.

Saveables

There are a few other issues to consider here:

  • Implementors of custom ContentMergeViewers need to be able to participate in the saving of ContentMergeViewers (see bug 143852)
  • The Saveable API was introduced in Eclipse 3.2 and could potentially be used by merge viewers. The new-style model-based sync makes use of this API already.

Here are some thoughts on how this could work.

  • Each CompareInput may have one or more Saveables associated with them.
  • The CompareEditorInput would keep track of what Saveables it contained and would keep the workbench view of them up-to-date.
  • If the ContentMergeViewer or subclass wants to save, they can flush and then invoke save on the appropriate Saveables (i.e. it may only be a subset of the Saveables in the CompareEditorInput).
  • If the editor is saved, again, a flush is required before the save is issued to all dirty saveables of the CompareEditorInput.
  • Of course, backwards compatibility must be maintained.

Viewer flushing

There are two different styles at odds here:

  • The current style involes a merge viewer that buffers the changes in the viewer's document and flushes them to the compare input (or diff node) before a save or if the comapre input changes.
  • An alternate style involves the changes being recording into the compare input (i.e. Saveable or file buffer) immediately.

The second style, in combination with the use of file buffers, would make the Compare editor save lifecycle more in-line with the workbench. The problem is backwards compatibility. We can modify ContentMergeViewer, TextMergeViewer and all other SDK subclasses to work with Saveables but the old behavior must remain for other ContentMergeViewer subclasses. Similarly, CompareEditorInputs that don't use Saveables and require viewer flushes must still work as well.

Navigation

When opening a Text or Java comparison from the Synchronize view, there are three sets of up/down navigation buttons that will appear: one set in the Synchronize view, one set in the toolbar and one set in the merge viewer. Each set of buttons has slightly different behavior.

  • The toolbar buttons will cycle through the differences in the Content Compare pane and then cycle through the entries in the Structure Compare pane.
  • The viewer buttons will cycle through each individual change in the Content Compare pane.
  • The Synchronize view buttons will do what the toolbar buttons do but go to the next/previous element in the Synchronize view when the end/beginning is reached.

It would be good if we could condense this into a consistent unified navigation behavior.

There are several bugs on this matter: bugs 143858, 36710, 143855. This is targetted to be addressed in 3.3 M4.

ContentMergeViewer Extendability

There are several package private methods on ContentMergeViewer that would be helpful for subclasses to be able to override. A case in point is the TextMergeViewer that does this already because it is in the same package (see bug 144638). In 3.3 M3, changes were made to ContentMergeViewer to surface as API the methods that were required by TextMergeViewer.

Back to the top