Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workspace Integration Tutorial should promote child matches #8

Open
pcdavid opened this issue Sep 4, 2022 · 1 comment
Open

Workspace Integration Tutorial should promote child matches #8

pcdavid opened this issue Sep 4, 2022 · 1 comment
Labels
bugzilla Issues migrated from the old Eclipse Bugzilla

Comments

@pcdavid
Copy link
Contributor

pcdavid commented Sep 4, 2022

  • πŸ†” Bugzilla ID: #217616
  • πŸ“˜ Project: Modeling / EMF Services / Transaction
  • πŸ—“ Created: 2008-02-03T18:00:36Z
  • ❓ Status: NEW /
@pcdavid pcdavid added the bugzilla Issues migrated from the old Eclipse Bugzilla label Sep 4, 2022
@pcdavid
Copy link
Contributor Author

pcdavid commented Sep 4, 2022

Comment #0 on Sun Feb 03 2008 19:00:36 GMT+0100 (Central European Standard Time):

The Workspace Integration Tutorial historyNotification implementation replaces one undo context with another in order to promote plausible undo history.

This approach seems undesirable in that it modifies the operation and fails to direct the user to available facilities that are very important to use since EditingDomainUndoContexts have the unusual behaviour that a Set<EditingDomainUndoContext> fails to provide the expected content (it creates a filtered set of distinct EditingDomain not UndoContext.

A more desirable approach seems to be for the per-editor OperationHistoryListener to maintain a single unique well-labelled per-editor ObjectUndoContext for each editor, and associate all related contexts so that they respond as child matches. The child match then sorts out the confusion of diverse undocontexts for different styles of operation. The unique per-editor ObjectUndoContext then provides a clearer filter to inhibit one OperationHistoryListener responding to another editor's OperationHistoryListener.

Comment #1 on Mon Feb 04 2008 15:42:38 GMT+0100 (Central European Standard Time):

I don't recognize the EditingDomainUndoContext class.  It doesn't seem to be defined in the tutorial or anywhere else in the EMF Transaction component.

I like the idea, though, of an editor context matching resource contexts.  Is that what you are suggesting?  It's not clear what all of the "diverse undocontexts" are.  Also, I'm not sure whether "child match" is meant to suggest that matching would be a directional relationship.  The operation history always checks for matches in both directions.

Comment #2 on Tue Feb 05 2008 08:17:47 GMT+0100 (Central European Standard Time):

In a multi-page editor I found I got an undo context from

WorkspaceCommandStack,
each TextEditor,
each DiagramEditor

mostly badly labelled and none of them collaborating.

Adding my own ObjectUndoContext per-editor to mediate between them solves the problems. 

If the WorkspaceCommandStack UndoContext were an ObjectUndoContext, then it could be used as the editor-wide coordinator.

My listener starts as

	public void historyNotification(OperationHistoryEvent event) {
		IUndoableOperation operation = event.getOperation();
		boolean applicableOperation = false;
		for (IUndoContext opUndoContext : operation.getContexts())
			if (undoContext.matches(opUndoContext))
				applicableOperation = true;
		if (!applicableOperation) {
			return;
		switch (event.getEventType()) {
                     ....


The loop could be a convenience method in OperationHistoryEvent .



Comment #3 on Tue Feb 05 2008 13:12:56 GMT+0100 (Central European Standard Time):

(In reply to comment #2)
-----8<-----
> 
> If the WorkspaceCommandStack UndoContext were an ObjectUndoContext, then it
> could be used as the editor-wide coordinator.

Why should it be an ObjectUndoContext?  I'm not sure that other parties than the "owner" of an ObjectUndoContext should be adding matches to an undo context.  Otherwise, this API would be specified by IUndoContext.  It seems more appropriate to me that an Editor's undo context should match the command-stack undo context (which is available from the IWorkspaceCommandStack interface) rather than the other way around.


> My listener starts as
> 
>         public void historyNotification(OperationHistoryEvent event) {
>                 IUndoableOperation operation = event.getOperation();
>                 boolean applicableOperation = false;
>                 for (IUndoContext opUndoContext : operation.getContexts())
>                         if (undoContext.matches(opUndoContext))
>                                 applicableOperation = true;
>                 if (!applicableOperation) {
>                         return;
>                 switch (event.getEventType()) {
>                      ....
> 
> 
> The loop could be a convenience method in OperationHistoryEvent .

This convenience loop already exists:

    boolean IUndoableOperation::hasContext(IUndoContext)

Just do:

        public void historyNotification(OperationHistoryEvent event) {
                IUndoableOperation operation = event.getOperation();
                if (!operation.hasContext(undoContext)) {
                        return;
                switch (event.getEventType()) {
                     ....


Comment #4 on Tue Feb 05 2008 20:31:53 GMT+0100 (Central European Standard Time):

> > If the WorkspaceCommandStack UndoContext were an ObjectUndoContext, then it
> > could be used as the editor-wide coordinator.
> 
> Why should it be an ObjectUndoContext?  I'm not sure that other parties than
> the "owner" of an ObjectUndoContext should be adding matches to an undo
> context.  Otherwise, this API would be specified by IUndoContext.  It seems
> more appropriate to me that an Editor's undo context should match the
> command-stack undo context (which is available from the IWorkspaceCommandStack
> interface) rather than the other way around.

That's what my current solution does.

It just seems that with exactly one IWorkspaceCommandStack undoContext per-editor, it is a bit redundant creating another. Perhaps IWorkspaceCommandStack undoContext could offer an IObjectUndoContext to support its utility as the editor-wide undo context.

Comment #5 on Sat May 14 2022 15:51:45 GMT+0200 (Central European Summer Time):

Eclipse EMF Transaction is moving away from this bugs.eclipse.org issue tracker to https://github.com/eclipse/emf-transaction.

If this issue is relevant to you and still present in the latest release:

* Create a new issue at https://github.com/eclipse/emf-transaction/issues/.
  * Use as title in GitHub the title of this Bugzilla ticket (may include the bug number or not, at your own convenience)
  * In the GitHub description, start with a link to this bugzilla ticket
  * Optionally add new content to the description if it can helps towards resolution
* Update bugzilla ticket
  * Add to "See also" property (up right column) the link to the newly created GitHub issue
  * Add a comment "Migrated to <link-to-newly-created-GitHub-issue>"
  * Set status as CLOSED MOVED

All issues that remain open will be automatically closed next week or so. Then the Bugzilla component for EMF Transaction will be archived and made read-only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from the old Eclipse Bugzilla
Projects
None yet
Development

No branches or pull requests

1 participant