--- a/src/org/tmatesoft/hg/core/HgUpdateConfigCommand.java	Thu Oct 25 19:59:08 2012 +0200
+++ b/src/org/tmatesoft/hg/core/HgUpdateConfigCommand.java	Fri Oct 26 18:17:15 2012 +0200
@@ -17,33 +17,33 @@
 package org.tmatesoft.hg.core;
 
 import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.LinkedHashMap;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 
 import org.tmatesoft.hg.internal.ConfigFile;
-import org.tmatesoft.hg.internal.Experimental;
+import org.tmatesoft.hg.internal.ConfigFileParser;
 import org.tmatesoft.hg.internal.Internals;
 import org.tmatesoft.hg.repo.HgInternals;
+import org.tmatesoft.hg.repo.HgInvalidStateException;
 import org.tmatesoft.hg.repo.HgRepository;
 
 /**
- * WORK IN PROGRESS, DO NOT USE
+ * Command to alter Mercurial configuration settings at various levels (system-wide, user-wide, repository-wide).
  *
  * @author Artem Tikhomirov
  * @author TMate Software Ltd.
  */
-@Experimental(reason="Investigating approaches to alter Hg configuration files")
 public final class HgUpdateConfigCommand extends HgAbstractCommand<HgUpdateConfigCommand> {
 	
+	private final SessionContext sessionCtx;
 	private final File configFile;
 	
-	private Map<String,List<String>> toRemove;
-	private Map<String,Map<String,String>> toSet;
-
-	private final SessionContext sessionCtx;
+	private final List<Operation> changes = new LinkedList<Operation>();
+	private boolean ignoreMissingKeys = false;
 
 	private HgUpdateConfigCommand(SessionContext sessionContext, File configurationFile) {
 		sessionCtx = sessionContext;
@@ -51,7 +51,6 @@
 	}
 	
 	public static HgUpdateConfigCommand forRepository(HgRepository hgRepo) {
-		// XXX HgRepository to implement SessionContext.Provider (with getContext())?
 		return new HgUpdateConfigCommand(hgRepo.getSessionContext(), HgInternals.getImplementationRepo(hgRepo).getFileFromRepoDir("hgrc"));
 	}
 	
@@ -62,94 +61,230 @@
 	public static HgUpdateConfigCommand forInstallation(SessionContext ctx) {
 		return new HgUpdateConfigCommand(ctx, Internals.getInstallationConfigurationFileToWrite(ctx));
 	}
-	
+
 	/**
-	 * Remove a property altogether
+	 * Remove an entry altogether. If no entry with the key found, {@link #execute()} fails.
+	 * 
+	 * @param section identifies section to alter, not <code>null</code> or otherwise ill-formed name
+	 * @param key identifies entry within section, not <code>null</code> or otherwise ill-formed name 
 	 * @return <code>this</code> for convenience
+	 * @throws IllegalArgumentException if arguments are <code>null</code> or empty
 	 */
-	public HgUpdateConfigCommand remove(String section, String key) {
-		if (toRemove == null) {
-			toRemove = new LinkedHashMap<String, List<String>>();
-		}
-		List<String> s = toRemove.get(section);
-		if (s == null) {
-			toRemove.put(section, s = new ArrayList<String>(5));
-		}
-		s.add(key);
-		if (toSet != null && toSet.containsKey(section)) {
-			toSet.get(section).remove(key);
-		}
+	public HgUpdateConfigCommand remove(String section, String key) throws IllegalArgumentException {
+		checkSection(section);
+		checkKey(key);
+		changes.add(Operation.deleteEntry(section, key));
 		return this;
 	}
 
 	/**
-	 * Delete single attribute in a multi-valued property
-	 * @return <code>this</code> for convenience
-	 */
-	public HgUpdateConfigCommand remove(String section, String key, String value) {
-		throw new UnsupportedOperationException();
-	}
-	
-	/**
-	 * Set single-valued properties or update multi-valued with a single value
+	 * Delete single attribute in a multi-valued property. If specified value not found among values of
+	 * the identified entry, {@link #execute()} fails. 
+	 * 
+	 * @param section identifies section to alter, not <code>null</code> or otherwise ill-formed name
+	 * @param key identifies entry within section, not <code>null</code> or otherwise ill-formed name 
+	 * @param value one of the values to remove, not <code>null</code> or an empty value
 	 * @return <code>this</code> for convenience
+	 * @throws IllegalArgumentException if arguments are <code>null</code> or empty
 	 */
-	public HgUpdateConfigCommand put(String section, String key, String value) {
-		if (toSet == null) {
-			toSet =  new LinkedHashMap<String, Map<String,String>>();
-		}
-		Map<String,String> s = toSet.get(section);
-		if (s == null) {
-			toSet.put(section, s = new LinkedHashMap<String, String>());
-		}
-		s.put(key, value);
+	public HgUpdateConfigCommand remove(String section, String key, String value) throws IllegalArgumentException {
+		checkSection(section);
+		checkKey(key);
+		changes.add(Operation.deleteValue(section, key, value));
 		return this;
 	}
 	
 	/**
-	 * Multi-valued properties
+	 * Set single-valued property or update multi-valued with a single value
+	 * 
+	 * @param section identifies section to alter, not <code>null</code> or otherwise ill-formed name
+	 * @param key identifies entry within section, not <code>null</code> or otherwise ill-formed name 
+	 * @param value new value, may be <code>null</code>
 	 * @return <code>this</code> for convenience
+	 * @throws IllegalArgumentException if arguments are <code>null</code> or empty
 	 */
-	public HgUpdateConfigCommand add(String section, String key, String value) {
-		throw new UnsupportedOperationException();
+	public HgUpdateConfigCommand put(String section, String key, String value) throws IllegalArgumentException {
+		checkSection(section);
+		checkKey(key);
+		changes.add(Operation.setValue(section, key, value));
+		return this;
 	}
 	
 	/**
-	 * Perform config file update
+	 * Add value to a multi-valued entry. If specified entry not found, {@link #execute()} fails.
 	 * 
+	 * @param section identifies section to alter, not <code>null</code> or otherwise ill-formed name
+	 * @param key identifies entry within section, not <code>null</code> or otherwise ill-formed name 
+	 * @param value new value to add, not <code>null</code> or an empty value
+	 * @return <code>this</code> for convenience
+	 * @throws IllegalArgumentException if arguments are <code>null</code> or empty
+	 */
+	public HgUpdateConfigCommand add(String section, String key, String value) throws IllegalArgumentException {
+		checkSection(section);
+		checkKey(key);
+		changes.add(Operation.addValue(section, key, value));
+		return this;
+	}
+	
+	/**
+	 * Tells whether {@link #execute()} shall fail with exception if keys selected for modification were not found. 
+	 * If <code>true</code>, missing keys would be silently ignored. 
+	 * When <code>false</code>(<em>default</em>), exception would be raised.
+	 *  
+	 * @param ignoreMissing pass <code>true</code> to ignore any incorrect keys
+	 * @return <code>this</code> for convenience
+	 */
+	public HgUpdateConfigCommand ignoreMissing(boolean ignoreMissing) {
+		ignoreMissingKeys = ignoreMissing;
+		return this;
+	}
+	
+	/**
+	 * Perform configuration file update.
+	 * 
+	 * @throws HgMissingConfigElementException if attempt to alter an entry failed to find one, and missing keys are not ignored
+	 * @throws HgIOException when configuration file read/write attemt has failed
 	 * @throws HgException subclass thereof to indicate specific issue with the command arguments or repository state
 	 */
-	public void execute() throws HgException {
+	public void execute() throws HgMissingConfigElementException, HgIOException, HgException {
 		try {
-			ConfigFile cfg = new ConfigFile(sessionCtx);
-			cfg.addLocation(configFile);
-			if (toRemove != null) {
-				for (Map.Entry<String,List<String>> s : toRemove.entrySet()) {
-					for (String e : s.getValue()) {
-						cfg.putString(s.getKey(), e, null);
+			ConfigFile cfgRead = new ConfigFile(sessionCtx);
+			cfgRead.addLocation(configFile);
+			ConfigFileParser cfgWrite = new ConfigFileParser();
+			FileInputStream fis = new FileInputStream(configFile);
+			cfgWrite.parse(fis);
+			fis.close();
+			for (Operation op : changes) {
+				if (!ignoreMissingKeys && !cfgRead.hasSection(op.section)) {
+					throw new HgMissingConfigElementException("Bad section name", op.section, op.key);
+				}
+				Map<String, String> sect = cfgRead.getSection(op.section);
+				if (!ignoreMissingKeys && !sect.containsKey(op.key)) {
+					throw new HgMissingConfigElementException("Bad key name", op.section, op.key);
+				}
+				String oldValue = sect.get(op.key);
+				if (oldValue == null) {
+					oldValue = "";
+				}
+				switch (op.kind) {
+				case AddValue: {
+					String separator = ", "; // XXX shall parse and find out separator kind in use
+					String newValue = oldValue + separator + op.value;
+					if (sect.containsKey(op.key)) {
+						cfgWrite.change(op.section, op.key, newValue);
+					} else {
+						cfgWrite.add(op.section, op.key, newValue);
 					}
+					break;
+				}
+				case DelValue: {
+					if (!ignoreMissingKeys && (oldValue.length() == 0 || !oldValue.contains(op.value))) {
+						throw new HgMissingConfigElementException(String.format("Bad value '%s' to delete from '%s'", op.value, oldValue), op.section, op.key);
+					}
+					int start = oldValue.indexOf(op.value);
+					if (start == -1) {
+						// nothing to change
+						break;
+					}
+					int commaPos = -1;
+					for (int i = start-1; i >=0; i--) {
+						if (oldValue.charAt(i) == ',') {
+							commaPos = i;
+							break;
+						}
+					}
+					for (int i = start + op.value.length(); commaPos == -1 && i < oldValue.length(); i++) {
+						if (oldValue.charAt(i) == ',') {
+							commaPos = i;
+							break;
+						}
+					}
+					String newValue;
+					if (commaPos >= 0) {
+						if (commaPos < start) {
+							// from preceding comma up to end of value
+							newValue = oldValue.substring(0, commaPos) + oldValue.substring(start + op.value.length());
+						} else {
+							// from value start up to and including subsequent comma
+							newValue = oldValue.substring(0, start) + oldValue.substring(commaPos+1);
+						}
+					} else {
+						// found no separator, just remove the value
+						// extra whitespaces (if space, not a comma is a separator) won't hurt
+						newValue = oldValue.substring(0, start) + oldValue.substring(start + op.value.length());
+					}
+					cfgWrite.change(op.section, op.key, newValue);
+					break;
+				}
+				case SetValue: {
+					if (sect.containsKey(op.key)) {
+						cfgWrite.change(op.section, op.key, op.value);
+					} else {
+						cfgWrite.add(op.section, op.key, op.value);
+					}
+					break;
+				}
+				case DelEntry: {
+					cfgWrite.delete(op.section, op.key);
+					break;
+				}
+				default: throw new HgInvalidStateException(String.format("Unknown change %s", op.kind));
 				}
 			}
-			if (toSet != null) {
-				for (Map.Entry<String,Map<String,String>> s : toSet.entrySet()) {
-					for (Map.Entry<String, String> e : s.getValue().entrySet()) {
-						cfg.putString(s.getKey(), e.getKey(), e.getValue());
-					}
-				}
-			}
-			cfg.writeTo(configFile);
+			FileOutputStream fos = new FileOutputStream(configFile);
+			cfgWrite.update(fos);
+			fos.close();
 		} catch (IOException ex) {
 			String m = String.format("Failed to update configuration file %s", configFile);
 			throw new HgBadArgumentException(m, ex); // TODO [post-1.0] better exception, it's not bad argument case
 		}
 	}
+	
+	private static void checkSection(String section) throws IllegalArgumentException {
+		if (section == null || section.trim().length() == 0) {
+			throw new IllegalArgumentException(String.format("Section name can't be empty: %s", section));
+		}
+	}
+	
+	private static void checkKey(String key) throws IllegalArgumentException {
+		if (key == null || key.trim().length() == 0) {
+			throw new IllegalArgumentException(String.format("Entry key can't be empty: %s", key));
+		}
+	}
 
 
-	public static void main(String[] args) throws Exception {
-		HgUpdateConfigCommand cmd = HgUpdateConfigCommand.forUser(null);
-		cmd.remove("test1", "sample1");
-		cmd.put("test2", "sample2", "value2");
-		cmd.put("ui", "user-name", "Another User <email@domain.com>");
-		cmd.execute();
+	private static class Operation {
+		private enum OpKind { AddValue, SetValue, DelValue, DelEntry };
+
+		public final OpKind kind;
+		public final String section;
+		public final String key;
+		public final String value;
+		
+		private Operation(OpKind t, String s, String k, String v) {
+			kind = t;
+			section = s;
+			key = k;
+			value = v;
+		}
+		
+		public static Operation deleteEntry(String section, String key) throws IllegalArgumentException {
+			return new Operation(OpKind.DelEntry, section, key, null);
+		}
+		public static Operation deleteValue(String section, String key, String value) throws IllegalArgumentException {
+			if (value == null || value.trim().length() == 0) {
+				throw new IllegalArgumentException(String.format("Can't remove empty value '%s'", value));
+			}
+			return new Operation(OpKind.DelValue, section, key, value);
+		}
+		public static Operation addValue(String section, String key, String value) throws IllegalArgumentException {
+			if (value == null || value.trim().length() == 0) {
+				throw new IllegalArgumentException(String.format("Can't add empty value '%s'", value));
+			}
+			return new Operation(OpKind.AddValue, section, key, value);
+		}
+		public static Operation setValue(String section, String key, String value) throws IllegalArgumentException {
+			return new Operation(OpKind.SetValue, section, key, value);
+		}
 	}
 }