Bug 175693 - [Actions] Convert popup menus to new menu support
Summary: [Actions] Convert popup menus to new menu support
Status: CLOSED WONTFIX
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Team (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 normal with 3 votes (vote)
Target Milestone: ---   Edit
Assignee: Platform Team Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords:
Depends on: 397986 182366 182368
Blocks: 196349 239003
  Show dependency tree
 
Reported: 2007-02-27 08:39 EST by Michael Valenta CLA
Modified: 2019-11-27 07:01 EST (History)
4 users (show)

See Also:


Attachments
Patch v1 (19.53 KB, patch)
2007-04-11 11:07 EDT, Tomasz Zarna CLA
no flags Details | Diff
Context menu with multiplied entries (46.69 KB, image/png)
2007-04-12 04:20 EDT, Tomasz Zarna CLA
no flags Details
Patch v2 (80.98 KB, patch)
2007-04-13 08:49 EDT, Tomasz Zarna CLA
no flags Details | Diff
mylar/context/zip (89.39 KB, application/octet-stream)
2007-04-13 08:49 EDT, Tomasz Zarna CLA
no flags Details
Patch v3 (4.30 KB, patch)
2009-08-28 10:56 EDT, Tomasz Zarna CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Valenta CLA 2007-02-27 08:39:24 EST
This is a follow on to bug 173456. Once we get the CVS and Team action sets converted to the new menu support, we should also convert the Team, CV and Compare  popup objectContributions.
Comment 1 Tomasz Zarna CLA 2007-04-11 11:07:00 EDT
Created attachment 63505 [details]
Patch v1

This is an extended version of patch from bug 173456 (attachment 62138 [details]). It has all features of new menu support enabled. The extra part is the one with popup objectContributions. My first comments/question are:

1) How do you place CVS menu items in a specific submenu? Should I use kind of "after" clause in locationURI after "popup:org.eclipse.ui.popup.any"?
2) How do you add separators to a popup's (sub)menu?
3) Why all these items are disabled all the time?

You don't need to answer this question right away.  I'm just thinking aloud. I'll be investigating it tomorrow anyway.
Comment 2 Michael Valenta CLA 2007-04-11 14:12:11 EDT
Paul's the expert (so I've CCed him) but I'll attempt to answer your questions:

1) I would assume it would look something like "popup:org.eclipse.ui.popup.any?team.main/group2".

2) It looks like Paul reorganized the Wiki page so now everything is hard to find. But I think this link explains how to add a submenu with separators:

http://wiki.eclipse.org/index.php/Menu_Contributions/Search_Menu

3) This may be the same issue as bug 179582
Comment 3 Tomasz Zarna CLA 2007-04-12 04:20:48 EDT
Created attachment 63591 [details]
Context menu with multiplied entries

Thanks Michael for your comment. This is (see the attachment) what I achived after adjusting your tips a little bit. As you can see entries in submenu are multiplied (excatly 4 times). It looks like similar problem I had with bug 173456. On the other hand I think this might be related to the fact that my current changes are more focused on items placement than on theirs enablemant/visibility rules, but this is just a guess. I will provide next version of the patch after more detailed examination.
Comment 4 Paul Webster CLA 2007-04-12 07:08:43 EDT
If you want to contribute a Team menu and a number of groups, and then have a variety of other contributions add to them, you can try:

<menuContribution locationURI="popup:org.eclipse.ui.popup.any?after=additions">
  <menu id="team.main" label="Team" ... etc>
    <separator name="group1" visible="false"/>
    <separator name="group2" visible="false"/>
    <separator name="group3" visible="false"/>
  </menu>
</menuContribution>

Then when adding a command:

<menuContribution locationURI="popup:team.main?after=group2">
  <command .../>
</menuContribution>

You can define any command that will always be included with the team menu in the first definition.  The important thing is that you can only define the team.main menu once.

PW
Comment 5 Tomasz Zarna CLA 2007-04-12 07:13:24 EDT
This is what I did to create screen shot from attachment 63591 [details]. Thanks Paul.
Comment 6 Michael Valenta CLA 2007-04-12 09:21:03 EDT
The CVS/UI plug-in should not create the Team menu and it's separators. This is done by the Team/UI plug-in. I am assuming that the menu groups as defined in the Team/UI plug-in using the old-style objectContributions can still be referenced from menuContributions and vica versa. I also assume that the reverse is true (i.e. if I define separators using a m4enuContribution, I can use those paths in an objectContribution). Paul, are these assumption correct?
Comment 7 Paul Webster CLA 2007-04-12 12:10:52 EDT
(In reply to comment #6)
> The CVS/UI plug-in should not create the Team menu and it's separators. This is
> done by the Team/UI plug-in. I am assuming that the menu groups as defined in
> the Team/UI plug-in using the old-style objectContributions can still be
> referenced from menuContributions and vica versa. I also assume that the
> reverse is true (i.e. if I define separators using a m4enuContribution, I can
> use those paths in an objectContribution). Paul, are these assumption correct?

Right now, the new stuff can't really see the old stuff reliably, and the old stuff can't see the new stuff at all.

I think the choice will have to be:

1) menuContributions are applied first.  menuContributions can see each other.  Also, menuContributions can see anything contributed by the WorkbenchActionBuilder for main menu/main coolbar, and menuContributions can see anything contributed programmatically in views (like is normally done in createPartControl).

2) older contributions are contributed next.  They can then see programmitic contributions (as always) and menu Contributions.  Legacy contributions are all written to include their menu structure and just not contribute it if it is already available.

PW
Comment 8 Tomasz Zarna CLA 2007-04-13 08:49:09 EDT
Created attachment 63731 [details]
Patch v2

Here is the next version of the patch. Although it's not fully working I thought it would be a good thing to submit it so you can actually see what problems I have. The patch modifies 3 plugin.xml files (Team/UI, CVS/UI and Compare). Major changes are made in the plugin.xml file from org.eclipse.team.cvs.ui project. I will shortly describe them in a moment. In plugin.xmls for Team/UI and Compare  I've replaced the old-style menu definition with new menuContributions.

Changes in org.eclipse.team.cvs.ui's plugin.xml (comparing to attachment 62138 [details] from bug 173456):
* more actions converted to commands+handlers
* old-style objectContribution commented out
* popups definitions in org.eclipse.ui.menu extension point

The problem from comment 3 is actual. I can still see multiple entries in a context menu. But as I was afraid it was just the beginning of an ice berg. The more I'm working on the patch the more questions I have. Here they are:

1) What will happen when activeWhen section will be omitted in a handler's definition? I thought that using visibleWhen clauses with the conditions from old objectContribution is more natural solution.

2) I haven't copied helpContextIds from actions, because I don't know where should they be copied to? To commands in org.eclipse.ui.commands or org.eclipse.ui.menus?

3) While converting objectContributions for ResourceMappings I've used visibleWhen clause in place of old enablement clause. Is that ok?

4) I've used only adapt tag (with no test clause inside) for ICVSRemoteFolder, ICVSRemoteFile, DateTagCategory, BranchCategory, ICVSRemoteResource, RepositoryRoot, CVSFileRevision. Can it be used this way? I assumed that it will simply test if selected item can be adapted to the given type.

5) I will need to look for project with miscGroup and add.group menu definitions. You can see them in one the plugins from the patch. I suppose I will need to defined them using new menu support mechanism.

6) Does the xml snippet below means that an entry will be visible when selection is either type of ICVSRemoteFolder or RemoteModule. Or am I using it completely wrong?

				<visibleWhen>
                 <iterate
                       operator="or">
                    <adapt
                          type="org.eclipse.team.internal.ccvs.core.ICVSRemoteFolder">
                    </adapt>
                    <adapt
                          type="org.eclipse.team.internal.ccvs.model.RemoteModule">
                    </adapt>
					</iterate>
					</visibleWhen>

7) Actions with id org.eclipse.team.ccvs.ui.configureTagsOnRemoteModule and org.eclipse.team.ccvs.ui.configureTagsOnRemoteFolder used the same action class. Was there any reason for that? Can I just make them use the same command and/or handler?

8) The following context menu entries are missing (at least this is what I've noticed so far):
* in Package Explorer view
	shared project : Team > Apply Patch
	non-shared project: whole Team menu
* in CVS Repositories view
	folder: tag as version, compare and compare with

I'm almost sure that this is related to the problem Paul mentions in comment 7 (mixing old and new menu definitions).

9) I've used org.eclipse.team.cvs.ui.commit instead of org.eclipse.team.ccvs.ui.commit id in context menu definition for Commit action (same with *.ccvs.ui.showHistory, *.ccvs.ui.showAnnotation). I suppose I should start with reading how to use definitionId tag.

This is all what I can think of at this moment. Thanks in advance for any help.

Have a good weekend guys :)
Comment 9 Tomasz Zarna CLA 2007-04-13 08:49:11 EDT
Created attachment 63732 [details]
mylar/context/zip
Comment 10 Michael Valenta CLA 2007-04-17 09:53:43 EDT
This is not containable in the 3.3 release so we're postponing it until 3.4.
Comment 11 Tomasz Zarna CLA 2008-04-17 06:55:00 EDT
I hope I will have time to get back to this issue during 3.5 cycle, until then let's update the target milestone to better match the reality.
Comment 12 Remy Suen CLA 2008-11-30 07:37:25 EST
+1 on this, I have a need to contribute to Team's popup context submenu. :O
Comment 13 Szymon Brandys CLA 2009-03-06 05:34:19 EST
We haven't investigated the issue during 3.5 and we don't plan to address it now. I'm removing the target milestone but we'll set more realistic one when we have feedback from anyone interested in fixing this.
Comment 14 Tomasz Zarna CLA 2009-08-28 10:56:57 EDT
Created attachment 145946 [details]
Patch v3

A quick patch I made in 30 minutes, but it looks promising. I haven't looked at the previous patches, they're way to big and quite old ;) The patch is limited to Team/UI plugin only, the only change made there is that now the Team sub-menu is defined using org.eclipse.ui.menus ext point. I wish I could create working patches that fast, unfortunately this is not the case:

1. The Team menu is not visible for shared Java projects. Why is that?
2. I haven't figure out how the place the Team sub-menu before the Compare With, just where it was so far.
3. For a shared project all sub-items (including Add to Version Control, Add to .cvsignore) are enabled. Is this issue covered in bug 182366 or bug 182368?

Another then that it looks fine, but like I said, it took my 30 minutes and that includes testing ;)

Paul it would be great if you could stop on this bug for 10 minutes and let me know if any of those issues are known to you?
Comment 15 Lars Vogel CLA 2019-11-27 07:01:49 EST
This bug hasn't had any activity in quite some time. Maybe the problem got
resolved, was a duplicate of something else, or became less pressing for some
reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it.
The information can be, for example, that the problem still occurs, that you
still want the feature, that more information is needed, or that the bug is
(for whatever reason) no longer relevant.

If the bug is still relevant, please remove the stalebug whiteboard tag.