Community
Participate
Working Groups
After marking a java element as landmarked, the action is still enabled. Shouldn't this be disabled since you can't landmark something that already is landmarked?
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.
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); } }
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.
to create a patch, please commit locally and then follow these steps: http://wiki.eclipse.org/EGit/User_Guide#Creating_Patches
Created attachment 201681 [details] Patch
The patch seems huge. Can you create a patch that has the relevant commit only?
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.
Created attachment 201754 [details] Patch for diable landmark functionality
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.
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?
Created attachment 203834 [details] mylyn/context/zip
(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.
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
