Bug 354989

Comment 1 Steffen Pingel 2011-08-17 15:41:20 EDT

Shawn, is that somewhat simple to do? If requires XML trickery I wonder if it's worth it. Miles, if you already have ideas how to implement it, please feel free to attach a patch.

Comment 2 Miles Parker 2011-08-17 16:10:55 EDT

Hmm.. I can't figure out how to create a patch from git team provider. Anyway, here is some code that seems to work:


/*******************************************************************************
 * Copyright (c) 2004, 2008 Tasktop Technologies and others.
 * All rights reserved. This program and the accompanying materials
 * are made available under the terms of the Eclipse Public License v1.0
 * which accompanies this distribution, and is available at
 * http://www.eclipse.org/legal/epl-v10.html
 *
 * Contributors:
 *     Tasktop Technologies - initial API and implementation
 *******************************************************************************/

package org.eclipse.mylyn.internal.context.ui.actions;

import org.eclipse.jface.action.IAction;
import org.eclipse.jface.viewers.ISelection;
import org.eclipse.jface.viewers.StructuredSelection;
import org.eclipse.mylyn.context.core.IInteractionElement;

/**
 * @author Mik Kersten
 */
public class InterestIncrementAction extends AbstractInterestManipulationAction {

	@Override
	protected boolean isIncrement() {
		return true;
	}

	@Override
	public void selectionChanged(IAction action, ISelection selection) {
		super.selectionChanged(action, selection);
		StructuredSelection structuredSelection = (StructuredSelection) selection;
		boolean allLandmarked = true;
		for (Object object : structuredSelection.toList()) {
			IInteractionElement node = convertSelectionToInteractionElement(object);
			if (!node.getInterest().isLandmark()) {
				allLandmarked = false;
				break;
			}
		}
		action.setEnabled(!allLandmarked);
	}
}

Comment 3 Miles Parker 2011-08-17 16:15:04 EDT

I should note that as in cases where an action can impact most selected elements we do have to be careful about not burdening the selection UI. But this seems like a pretty quick check.

Comment 4 Steffen Pingel 2011-08-17 17:31:53 EDT

to create a patch, please commit locally and then follow these steps: http://wiki.eclipse.org/EGit/User_Guide#Creating_Patches

Comment 5 Miles Parker 2011-08-17 19:07:06 EDT

Created attachment 201681 [details]
Patch

Comment 6 Steffen Pingel 2011-08-18 06:22:57 EDT

The patch seems huge. Can you create a patch that has the relevant commit only?

Comment 7 Miles Parker 2011-08-18 14:19:11 EDT

Wow, I should have looked at that first! I wonder why -- I only made the one file commit. Ah. I didn't fetch first and it looks like it isn't patching against my local repos but against the remote repos. That's surprising.

Comment 8 Miles Parker 2011-08-18 18:25:58 EDT

Created attachment 201754 [details]
Patch for diable landmark functionality

Comment 9 Miles Parker 2011-08-18 18:27:40 EDT

OK, let's try this one. Creating the patch was a lot harder than making the fix. :) I was using the main branch from two years ago.

Comment 10 Steffen Pingel 2011-09-22 07:09:49 EDT

This patch looks good to me. I would only recommend adding an instanceof check before casting the selection and a null check for the node. 

I am not sure that making this change is a good idea though since disabled actions always have potential for confusion as to why are they are disabled. Shawn?

Comment 11 Steffen Pingel 2011-09-22 07:09:51 EDT

Created attachment 203834 [details]
mylyn/context/zip

Comment 12 Miles Parker 2011-09-28 12:48:32 EDT

(In reply to comment #10)
> This patch looks good to me. I would only recommend adding an instanceof check
> before casting the selection and a null check for the node.

Hi Steffen. I thought I responded to this and forgot. You're absolutely right, those checks should be in there -- in fact I ran into that issue a while back. I'll submit a revised patch.
 
> 
> I am not sure that making this change is a good idea though since disabled
> actions always have potential for confusion as to why are they are disabled.
> Shawn?

I see it as a tradeoff -- that it's more confusing to see an option to do something that has already happened.

Comment 13 Eclipse Webmaster 2022-11-15 11:45:08 EST

Mylyn has been restructured, and our issue tracking has moved to GitHub [1].

We are closing ~14K Bugzilla issues to give the new team a fresh start. If you feel that this issue is still relevant, please create a new one on GitHub.

[1] https://github.com/orgs/eclipse-mylyn