Bug # 228953 Usability: Project Properties->C/C++ Build->Variables

build/org.eclipse.cdt.managedbuilder.ui/src/org/eclipse/cdt/managedbuilder/ui/properties/CPropertyVarsTab.java

@@ -156,7 +156,13 @@ public class CPropertyVarsTab extends AbstractCPropertyTab {
 		public String getText(Object element) { return getColumnText(element, 0); }
 		public Font getFont(Object element) { return getFont(element, 0); }
 		public Image getColumnImage(Object element, int columnIndex) { return null; }
-		public Color getBackground(Object element){	return null; }
+
+		public Color getBackground(Object element){
+			ICdtVariable var = (ICdtVariable)element;
+			if(isUserVar(var))
+				return BACKGROUND_FOR_USER_VAR;
+			return null; 
+		}
 		
 		public String getColumnText(Object element, int columnIndex) {
 			ICdtVariable var = (ICdtVariable)element;
@@ -190,26 +196,14 @@ public class CPropertyVarsTab extends AbstractCPropertyTab {
 			return EMPTY_STR;
 		}
 		
-		private Font getValueFont(ICdtVariable var){
-			Font font = null;
-			if(isUserVar(var))
-				font = JFaceResources.getFontRegistry().getItalic(JFaceResources.DIALOG_FONT);
-			return font;
-		}
 		public Font getFont(Object element, int columnIndex) {
 			ICdtVariable var = (ICdtVariable)element;
-			switch(columnIndex){
+			if(columnIndex == 0 && isUserVar(var))
-			case 0:
-			case 1:
-				break;
-			case 2:
-				return getValueFont(var);
-			}
-			if(isUserVar(var))
 				return JFaceResources.getFontRegistry().getBold(JFaceResources.DIALOG_FONT);
 			return null;
 		}
-	    public Color getForeground(Object element){
+
+		public Color getForeground(Object element){
 			if(fIncorrectlyDefinedMacrosNames.contains(((ICdtVariable)element).getName()))
 				return JFaceResources.getColorRegistry().get(JFacePreferences.ERROR_COLOR);
 			return null;
@@ -255,6 +249,7 @@ public class CPropertyVarsTab extends AbstractCPropertyTab {
 			handleDelButton();
 			break;
 		}
+		tv.getTable().setFocus();
 	}
 	
 	private void replaceMacros() {

core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/core/envvar/ContributedEnvironment.java

@@ -151,8 +151,11 @@ public class ContributedEnvironment implements IContributedEnvironment{
 				return Messages.getString("ContributedEnvironment.0"); //$NON-NLS-1$
 			if (sup instanceof EclipseEnvironmentSupplier)
 				return Messages.getString("ContributedEnvironment.1"); //$NON-NLS-1$
-			if (sup instanceof UserDefinedEnvironmentSupplier)
+			if (sup instanceof UserDefinedEnvironmentSupplier) {
+				if (((EnvVarDescriptor)var).getContextInfo().getContext() != null)
+					return Messages.getString("ContributedEnvironment.4"); //$NON-NLS-1$
 				return Messages.getString("ContributedEnvironment.2"); //$NON-NLS-1$
+			}
 		}
 		return Messages.getString("ContributedEnvironment.3"); //$NON-NLS-1$
 	}

core/org.eclipse.cdt.core/src/org/eclipse/cdt/internal/core/envvar/messages.properties

@@ -1,4 +1,5 @@
 ContributedEnvironment.0=BUILD SYSTEM
 ContributedEnvironment.1=ECLIPSE ENV
-ContributedEnvironment.2=USER
+ContributedEnvironment.2=USER: PREFS
 ContributedEnvironment.3=UNKNOWN
+ContributedEnvironment.4=USER: CONFIG

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/newui/AbstractCPropertyTab.java

@@ -26,6 +26,7 @@ import org.eclipse.swt.SWT;
 import org.eclipse.swt.custom.CLabel;
 import org.eclipse.swt.events.SelectionAdapter;
 import org.eclipse.swt.events.SelectionEvent;
+import org.eclipse.swt.graphics.Color;
 import org.eclipse.swt.graphics.FontMetrics;
 import org.eclipse.swt.graphics.GC;
 import org.eclipse.swt.graphics.Image;
@@ -36,6 +37,7 @@ import org.eclipse.swt.widgets.Button;
 import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Control;
 import org.eclipse.swt.widgets.DirectoryDialog;
+import org.eclipse.swt.widgets.Display;
 import org.eclipse.swt.widgets.FileDialog;
 import org.eclipse.swt.widgets.Group;
 import org.eclipse.swt.widgets.Label;
@@ -98,6 +100,7 @@ public abstract class AbstractCPropertyTab implements ICPropertyTab {
 	public static final String WORKSPACE_FILE_DIALOG_ERR = UIMessages.getString("BrowseEntryDialog.wsp.file.dlg.err");	//$NON-NLS-1$
 	public static final String WORKSPACE_DIR_DIALOG_ERR = UIMessages.getString("BrowseEntryDialog.wsp.dir.dlg.err");	//$NON-NLS-1$
 	public static final String BACKGROUND_TEXT_DEFAULT = UIMessages.getString("AbstractCPropertyTab.2"); //$NON-NLS-1$
+	public static final Color BACKGROUND_FOR_USER_VAR = new Color(Display.getDefault(), 255, 255, 200); // light yellow
 	
 	public static final int TRI_UNKNOWN = 2;
 	public static final int TRI_YES = 1;

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/newui/EnvironmentTab.java

@@ -13,6 +13,7 @@ package org.eclipse.cdt.ui.newui;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.Map;
 
@@ -54,6 +55,7 @@ import org.eclipse.cdt.core.settings.model.ICResourceDescription;
 import org.eclipse.cdt.utils.envvar.StorableEnvironment;
 import org.eclipse.cdt.utils.spawner.EnvironmentReader;
 
+import org.eclipse.cdt.internal.core.envvar.EnvVarDescriptor;
 import org.eclipse.cdt.internal.core.envvar.EnvironmentVariableManager;
 import org.eclipse.cdt.internal.core.envvar.UserDefinedEnvironmentSupplier;
 
@@ -78,13 +80,19 @@ public class EnvironmentTab extends AbstractCPropertyTab {
 	private ICConfigurationDescription cfgd = null;
 	private StorableEnvironment vars = null;
 
-	private class TabData {
+	private class TabData implements Comparable<TabData> {
 		IEnvironmentVariable var;
 		boolean changed;
 		TabData(IEnvironmentVariable _var, boolean _changed) {
 			var = _var;
 			changed = _changed;
 		}
+		public int compareTo(TabData a) {
+			String s = var.getName();
+			if (a != null && s != null && a.var != null)
+					return (s.compareTo(a.var.getName())); 
+			return 0;
+		}
 	}
 	
 	private class EnvironmentLabelProvider extends LabelProvider implements ITableLabelProvider, IFontProvider , ITableFontProvider, IColorProvider{
@@ -122,30 +130,29 @@ public class EnvironmentTab extends AbstractCPropertyTab {
 
 		public Font getFont(Object element, int columnIndex) {
 			TabData td = (TabData)element;
-			Font f = null;
-			if (cfgd == null || ce.isUserVariable(cfgd, td.var))
-				f = JFaceResources.getFontRegistry().getBold(JFaceResources.DIALOG_FONT);
-
 			switch(columnIndex){
 			case 0:
+				if (isUsers(td.var))
+					return JFaceResources.getFontRegistry().getBold(JFaceResources.DIALOG_FONT);
 				break;
 			case 1:
 				if(td.var.getOperation() == IEnvironmentVariable.ENVVAR_REMOVE)
-					f = JFaceResources.getFontRegistry().getItalic(JFaceResources.DIALOG_FONT);
+					return JFaceResources.getFontRegistry().getItalic(JFaceResources.DIALOG_FONT);
+				break;
+			default:
 				break;
-			case 2:
-				return null;
 			}
-			return f;
+			return null;
 		}
 		
-	    /* (non-Javadoc)
-	     * @see org.eclipse.jface.viewers.IColorProvider#getForeground(java.lang.Object)
-	     */
 	    public Color getForeground(Object element){
 			return null;
 	    }
-		public Color getBackground(Object element){
+
+	    public Color getBackground(Object element){
+			TabData td = (TabData)element;
+			if (isUsers(td.var))
+				return BACKGROUND_FOR_USER_VAR;
 			return null; 
 	    }
 	}
@@ -270,20 +277,33 @@ public class EnvironmentTab extends AbstractCPropertyTab {
 			handleEnvUndefButtonSelected(table.getSelectionIndex());
 			break;
 		}
+		table.setFocus();
 	}
 	
 	@Override
-	public void updateButtons() {
+	protected void updateButtons() {
 		if (table == null || table.isDisposed()) return;
 		
-		int pos = table.getSelectionIndex();
+		boolean canEdit = table.getSelectionCount() == 1;
-		buttonSetEnabled(2, pos != -1);
+		boolean canDel  = false;
-		buttonSetEnabled(3, pos != -1);
+		boolean canUndef = table.getSelectionCount() >= 1;
-		buttonSetEnabled(4, pos != -1);
+		if (canUndef) {
+			for (int i : table.getSelectionIndices()) {
+				IEnvironmentVariable var = ((TabData)tv.getElementAt(i)).var;
+				if (isUsers(var)) {
+				//	if (cfgd == null || !wse.getVariable(var.))
+					canDel = true;
+					break;
+				}
+			}
+		}
+		buttonSetEnabled(2, canEdit); // edit
+		buttonSetEnabled(3, canDel); // delete
+		buttonSetEnabled(4, canUndef); // undefine
 	}
 
 	@Override
-	public void updateData(ICResourceDescription _cfgd) {
+	protected void updateData(ICResourceDescription _cfgd) {
 		// null means preference configuration 
 		cfgd = (_cfgd != null) ? _cfgd.getConfiguration() : null;
 		if (cfgd == null && vars == null)
@@ -311,6 +331,7 @@ public class EnvironmentTab extends AbstractCPropertyTab {
 				data.add(new TabData(_var, false));
 			}
 		}
+		Collections.sort(data);
 		tv.setInput(data);
 		
 		updateLbs(lb1, lb2);
@@ -418,6 +439,11 @@ public class EnvironmentTab extends AbstractCPropertyTab {
 				ce.removeVariable(var.getName(), cfgd);
 		}
 		updateData();
+		int x = table.getItemCount() - 1;
+		if (x >= 0) {
+			table.setSelection(Math.min(x, n));
+			updateButtons();
+		}
 	}
 	
 	private void handleEnvAddButtonSelected() {
@@ -429,22 +455,36 @@ public class EnvironmentTab extends AbstractCPropertyTab {
 				page.isMultiCfg(),
 				cfgd);
 		if (dlg.open() == Window.OK) {
-			if (dlg.t1.trim().length() > 0) {
+			String name = dlg.t1.trim();
+			if (name.length() > 0) {
 				ICConfigurationDescription[] cfgs;
 				if (dlg.toAll)
 					cfgs = page.getCfgsEditable();
 				else 
 					cfgs = new ICConfigurationDescription[] {cfgd};
 				if (cfgd == null)
-					vars.createVariable(dlg.t1.trim(), dlg.t2.trim(), 
+					vars.createVariable(name, dlg.t2.trim(), 
 							IEnvironmentVariable.ENVVAR_APPEND,	SEMI);
 				else
 					for (ICConfigurationDescription cfg : cfgs) { 
-						ce.addVariable(dlg.t1.trim(), dlg.t2.trim(), 
+						ce.addVariable(name, dlg.t2.trim(), 
 								IEnvironmentVariable.ENVVAR_APPEND, 
 								SEMI, cfg);
 					}
 				updateData();
+				setPos(name);
+			}
+		}
+	}
+
+	private void setPos(String name) {
+		if (name == null || name.length() == 0)
+			return;
+		for (int i=0; i<table.getItemCount(); i++) {
+			if (name.equals(table.getItem(i).getText())) {
+				table.setSelection(i);
+				updateButtons();
+				break;
 			}
 		}
 	}
@@ -463,8 +503,9 @@ public class EnvironmentTab extends AbstractCPropertyTab {
 			else 
 				cfgs = new ICConfigurationDescription[] {cfgd};
 			
+			String name = null;
 			for (Object element : selected) {
-				String name = (String)element;
+				name = (String)element;
 				String value = EMPTY_STR;
 				int x = name.indexOf(LBR);
 				if (x >= 0) {
@@ -483,6 +524,7 @@ public class EnvironmentTab extends AbstractCPropertyTab {
 				}
 			}
 			updateData();
+			setPos(name);
 		}
 	}
 	
@@ -537,4 +579,11 @@ public class EnvironmentTab extends AbstractCPropertyTab {
 		vars = null;
 		updateData();
 	}
+	
+	private boolean isUsers(IEnvironmentVariable var) {
+		return cfgd == null || 
+		      (ce.isUserVariable(cfgd, var) &&
+			  ((EnvVarDescriptor)var).getContextInfo().getContext() != null);
+
+	}
 }

core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/newui/PluginResources.properties

@@ -120,7 +120,7 @@ NewEnvVarDialog.label.operation=Operation
 NewEnvVarDialog.label.operation.replace=Replace
 NewEnvVarDialog.label.operation.prepend=Prepend
 NewEnvVarDialog.label.operation.append=Append
-NewEnvVarDialog.label.operation.remove=Remove
+NewEnvVarDialog.label.operation.remove=Delete
 NewEnvVarDialog.label.value.prepend=Prepended Value
 NewEnvVarDialog.label.value.append=Appended Value
 NewEnvVarDialog.label.value.undef=<UNDEFINED>
@@ -128,13 +128,6 @@ NewEnvVarDialog.label.title.new=Define a new variable
 NewEnvVarDialog.label.title.edit=Edit existing variable
 NewEnvVarDialog.label.status.cannot.create=The "{0}" Variable can not be created by user
 
-# ----------- Macros Set Block -----------
-MacrosSetBlock.label.macros=Macros
-MacrosSetBlock.label.macros.group=Macros
-MacrosSetBlock.label.tab.configuration=Configuration
-MacrosSetBlock.label.tab.project=Project
-MacrosSetBlock.label.tab.workspace=Workspace
-
 # ----------- Macros Block -----------
 MacrosBlock.label.header.name=Name
 MacrosBlock.label.header.type=Type
@@ -142,13 +135,13 @@ MacrosBlock.label.header.value=Value
 MacrosBlock.label.button.new=New
 MacrosBlock.label.button.edit=Edit
 MacrosBlock.label.button.delete=Delete
-MacrosBlock.label.button.check.chow.parent=Show parent context macros
+MacrosBlock.label.button.check.chow.parent=Show parent context Build Variables
-MacrosBlock.label.user.macros=User Macros
+MacrosBlock.label.user.macros=User Build Variables
-MacrosBlock.label.system.macros=System Macros
+MacrosBlock.label.system.macros=System Build Variables
-MacrosBlock.label.delete.confirm.title=Macro deletion confirmation
+MacrosBlock.label.delete.confirm.title=Build Variable deletion confirmation
-MacrosBlock.label.delete.confirm.message=Are you sure you want to delete the selected user Macro(s)? 
+MacrosBlock.label.delete.confirm.message=Are you sure you want to delete the selected user Build Variable(s)? 
-MacrosBlock.label.delete.all.confirm.title=Macro deletion confirmation
+MacrosBlock.label.delete.all.confirm.title=Build Variables deletion confirmation
-MacrosBlock.label.delete.all.confirm.message=Are you sure you want to delete all user macros?
+MacrosBlock.label.delete.all.confirm.message=Are you sure you want to delete all user Build Variables?
 MacrosBlock.label.value.eclipse.dynamic=<ECLIPSE DYNAMIC VARIABLE>
 MacrosBlock.label.type.text=String
 MacrosBlock.label.type.text.list=String List
@@ -284,7 +277,7 @@ ConfigurationConvert.confirmdialog.title=Confirm Configuration Conversion
 # ----------- Build Property Common -----------
 BuildPropertyCommon.label.title=Enter Value
 BuildPropertyCommon.label.new=New...
-BuildPropertyCommon.label.remove=Remove
+BuildPropertyCommon.label.remove=Delete
 BuildPropertyCommon.label.up=Up
 BuildPropertyCommon.label.down=Down
 BuildPropertyCommon.label.editVar=Edit...
@@ -456,7 +449,7 @@ EnvironmentTab.4=Replace native environment with specified one
 EnvironmentTab.5=New...
 EnvironmentTab.6=Select...
 EnvironmentTab.7=Edit...
-EnvironmentTab.8=Remove
+EnvironmentTab.8=Delete
 EnvironmentTab.9=Undefine
 EnvironmentTab.10=New variable
 EnvironmentTab.11=Edit variable