--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/src/org/tmatesoft/hg/core/HgIOException.java	Fri Oct 26 18:17:15 2012 +0200
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2012 TMate Software Ltd
+ *  
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * For information on how to redistribute this software under
+ * the terms of a license other than GNU General Public License
+ * contact TMate Software at support@hg4j.com
+ */
+package org.tmatesoft.hg.core;
+
+import java.io.File;
+import java.io.IOException;
+
+/**
+ * Tailored wrap for {@link IOException} and similar I/O-related issues. Unlike {@link IOException}, 
+ * keeps track of {@link File} that caused the problem. Besides, additional information (like revision, 
+ * see {@link HgException}) may be attached.
+ * 
+ * @author Artem Tikhomirov
+ * @author TMate Software Ltd.
+ */
+@SuppressWarnings("serial")
+public class HgIOException extends HgException {
+	private final File file;
+
+	public HgIOException(String message, File troubleFile) {
+		this(message, null, troubleFile);
+	}
+
+	/**
+	 * @param message describes the issue, never <code>null</code>
+	 * @param cause root cause for the error, likely {@link IOException} or its subclass, but not necessarily, and may be omitted. 
+	 * @param troubleFile file we tried to deal with, never <code>null</code>
+	 */
+	public HgIOException(String message, Exception cause, File troubleFile) {
+		super(message, cause);
+		file = troubleFile;
+	}
+
+	public File getFile() {
+		return file;
+	}
+}

--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/src/org/tmatesoft/hg/core/HgMissingConfigElementException.java	Fri Oct 26 18:17:15 2012 +0200
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2012 TMate Software Ltd
+ *  
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * For information on how to redistribute this software under
+ * the terms of a license other than GNU General Public License
+ * contact TMate Software at support@hg4j.com
+ */
+package org.tmatesoft.hg.core;
+
+/**
+ * Exception 
+ * 
+ * @author Artem Tikhomirov
+ * @author TMate Software Ltd.
+ */
+@SuppressWarnings("serial")
+public class HgMissingConfigElementException extends HgException {
+
+	private final String section;
+	private final String key;
+
+	public HgMissingConfigElementException(String message, String troubleSection, String missingKey) {
+		super(message);
+		section = troubleSection;
+		key = missingKey;
+	}
+
+	public String getSection() {
+		return section;
+	}
+	
+	public String getKey() {
+		return key;
+	}
+}

--- 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);
+		}
 	}
 }

--- a/src/org/tmatesoft/hg/internal/ConfigFile.java	Thu Oct 25 19:59:08 2012 +0200
+++ b/src/org/tmatesoft/hg/internal/ConfigFile.java	Fri Oct 26 18:17:15 2012 +0200
@@ -62,6 +62,11 @@
 		return sections == null ? Collections.<String>emptyList() : Collections.unmodifiableList(sections);
 	}
 
+	/**
+	 * Access map of section keys and values.
+	 * @param sectionName name of the section to retrieve
+	 * @return never <code>null</code>, empty map in case no section with specified name found 
+	 */
 	public Map<String,String> getSection(String sectionName) {
 		if (sections == null) {
 			return Collections.emptyMap();

--- a/src/org/tmatesoft/hg/internal/ConfigFileParser.java	Thu Oct 25 19:59:08 2012 +0200
+++ b/src/org/tmatesoft/hg/internal/ConfigFileParser.java	Fri Oct 26 18:17:15 2012 +0200
@@ -16,7 +16,6 @@
  */
 package org.tmatesoft.hg.internal;
 
-import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
@@ -209,7 +208,27 @@
 			// if section comes more than once, update only first one.
 			processedSections.add(section.name);
 		}
+		// push rest of the contents
 		out.write(contents, contentsOffset, contents.length - contentsOffset);
+		//
+		// add entries in new sections
+		LinkedHashSet<String> newSections = new LinkedHashSet<String>();
+		for (Iterator<String> it = additions.iterator(); it.hasNext();) {
+			String s = it.next(); it.next(); it.next();
+			if (!processedSections.contains(s)) {
+				newSections.add(s);
+			}
+		}
+		for (String newSectionName : newSections) {
+			out.write(String.format("\n[%s]", newSectionName).getBytes());
+			for (Iterator<String> it = additions.iterator(); it.hasNext();) {
+				String s = it.next(), k = it.next(), v = it.next();
+				if (newSectionName.equals(s)) {
+					out.write(String.format("\n%s = %s", k, v).getBytes());
+				}
+			}
+			out.write("\n".getBytes());
+		}
 	}
 	
 	private void processLine(int lineNumber, int offset, byte[] line) throws IOException {
@@ -359,25 +378,4 @@
 			e.toArray(entries);
 		}
 	}
-
-	public static void main(String[] args) throws Exception {
-		ConfigFileParser p = new ConfigFileParser();
-		p.parse(new ByteArrayInputStream(xx.getBytes()));
-		System.out.println(">>>");
-		System.out.println(xx);
-		System.out.println("===");
-		p.add("sect1", "key5", "x");
-		ByteArrayOutputStream out = new ByteArrayOutputStream(xx.length());
-		p.update(out);
-		System.out.println(new String(out.toByteArray()));
-		/*
-		for (Section s : p.sections) {
-			System.out.printf("[%s@%d]\n", s.name, s.start);
-			for (Entry e : s.entries) {
-				System.out.printf("%s@%d = %d..%d\n", e.name, e.start, e.valueStart, e.valueEnd);
-			}
-		}
-		*/
-	}
-	private static final String xx = "#comment1\n [sect1]\nkey = value #not a comment2\n#comment3\nkey2=   \nkey3 =  \n  value1, #cc\n  value2\nkey4 = v1,\n  v2 \n  ,v3\n\n\n[sect2]\nx = a";
 }