Bug 74103 - The editor toolbar should prefer the TopComponent lookup as the context for context-aware actions
The editor toolbar should prefer the TopComponent lookup as the context for c...
Status: RESOLVED FIXED
Product: editor
Classification: Unclassified
Component: -- Other --
5.x
All All
: P2 (vote)
: 5.x
Assigned To: Andrei Badea
issues@editor
5.5_fix
: API_REVIEW_FAST
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-03-28 16:35 UTC by Andrei Badea
Modified: 2007-11-05 13:44 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
:


Attachments
Proposed fix (1.61 KB, text/plain)
2006-03-28 16:36 UTC, Andrei Badea
Details
Proposed fix incl. tests (7.84 KB, text/plain)
2006-03-29 17:12 UTC, Andrei Badea
Details
Action context is composed of the node and Lookup.Provider lookups (10.18 KB, text/plain)
2006-03-30 10:04 UTC, Andrei Badea
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrei Badea 2006-03-28 16:35:00 UTC
Currently the editor toolbar prefers a lookup containing the node corresponding
to the editor component's document as the context for context-aware actions.
This is unexpected for e.g. an action in the toolbar of a CloneableEditor which
wants to make use of an interface in the lookup of this CloneableEditor.

The editor toolbar should prefer the lookup of the TopComponent (or another 
Lookup.Provider) containing the editor component, and only delegate to the node
corresponding to the current document if such a lookup can not be found. This is
unlikely to cause regressions, since CloneableEditor-s contain the nodes
corresponding to the edited documents in their lookups.

I will attach a patch with the proposed fix. P2 since it blocks a P2 enancement
in the SQL editor.
Comment 1 Andrei Badea 2006-03-28 16:36:26 UTC
Created attachment 29452 [details]
Proposed fix
Comment 2 Andrei Badea 2006-03-29 17:10:04 UTC
As Mila's suggestion I would like to ask for a review of this change. I am
attaching the implementation and some tests proving the implementation is correct.
Comment 3 Andrei Badea 2006-03-29 17:12:43 UTC
Created attachment 29486 [details]
Proposed fix incl. tests
Comment 4 Jesse Glick 2006-03-29 18:19:53 UTC
Am I correct that this would in fact permit the toolbar to be attached to an
editor window that is not a TopComponent at all, provided that some container
implements Lookup.Provider? If so, +1. Though it looks like that may already be
the case...?
Comment 5 Milos Kleint 2006-03-30 05:52:34 UTC
does it work in multiviews as well?
Comment 6 Andrei Badea 2006-03-30 09:56:34 UTC
jglick: I think this was already possible (although I haven't tried). But I
think the fact that you don't have the lookup of the enclosing Lookup.Provider
available somehow limits this use case.

mkleint: no, it doesn't work in multiviews :-( The first Lookup.Provider is a
CloneableEditor having only a o.o.w.DelegateActionMap in its lookup, but no
nodes. Thank you for the catch!

It seems the action context lookup should be composed from the first
Lookup.Provider's lookup and the lookup of the node delegate corresponding to
the current document. The implementation has to ensure that the resulting lookup
doesn't contain the node delegate twice -- see the new diff.
Comment 7 Andrei Badea 2006-03-30 10:04:46 UTC
Created attachment 29495 [details]
Action context is composed of the node and Lookup.Provider lookups
Comment 8 Jesse Glick 2006-03-30 21:57:43 UTC
The while-loop can be simplified using Collection.contains().

Lookups.exclude might be helpful, I don't know.
Comment 9 Andrei Badea 2006-03-31 12:19:49 UTC
Re. loop: sure. It's true what they say, too much copy-pasting is bad... I will
fix it before committing.

Re. Lookups.exclude: I have considered it shortly, but I prefer the current
approach unless there is some problem with it. It seems cleaner to me that I
don't exclude anything from the lookups, I either return one of them or an union
of both.
Comment 10 Andrei Badea 2006-04-11 20:11:37 UTC
If there are no more comments, I would like to integrate by the end of the week.
Thank you for the review.
Comment 11 Andrei Badea 2006-04-17 19:29:31 UTC
Integrated into the trunk:

Checking in src/org/netbeans/modules/editor/NbEditorToolBar.java;
/cvs/editor/src/org/netbeans/modules/editor/NbEditorToolBar.java,v  <-- 
NbEditorToolBar.java
new revision: 1.25; previous revision: 1.24
done
RCS file:
/cvs/editor/test/unit/src/org/netbeans/modules/editor/NbEditorToolBarTest.java,v
done
Checking in test/unit/src/org/netbeans/modules/editor/NbEditorToolBarTest.java;
/cvs/editor/test/unit/src/org/netbeans/modules/editor/NbEditorToolBarTest.java,v
 <--  NbEditorToolBarTest.java
initial revision: 1.1
done

Integrated into the release55 branch:

Checking in nbproject/project.properties;
/cvs/editor/nbproject/project.properties,v  <--  project.properties
new revision: 1.17.6.1.2.1; previous revision: 1.17.6.1
done
Checking in src/org/netbeans/modules/editor/NbEditorToolBar.java;
/cvs/editor/src/org/netbeans/modules/editor/NbEditorToolBar.java,v  <-- 
NbEditorToolBar.java
new revision: 1.18.2.1.2.1; previous revision: 1.18.2.1
done
Checking in test/unit/src/org/netbeans/modules/editor/NbEditorToolBarTest.java;
/cvs/editor/test/unit/src/org/netbeans/modules/editor/NbEditorToolBarTest.java,v
 <--  NbEditorToolBarTest.java
new revision: 1.1.2.1; previous revision: 1.1
done


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo