# HG changeset patch
# User Artem Tikhomirov <tikhomirov.artem@gmail.com>
# Date 1332446540 -3600
# Node ID 528b6780a8bdcdf7a957c14635086d58b1204e3a
# Parent  ccd7d25e5aea2aa06ba67e6ae91cd2a8d9dd443a
A bit of FIXME cleanup (mostly degraded to TODO post 1.0), comments and javadoc

diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/core/ChangesetTransformer.java
--- a/src/org/tmatesoft/hg/core/ChangesetTransformer.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/core/ChangesetTransformer.java	Thu Mar 22 21:02:20 2012 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011 TMate Software Ltd
+ * Copyright (c) 2011-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
@@ -30,7 +30,7 @@
 
 /**
  * Bridges {@link HgChangelog.RawChangeset} with high-level {@link HgChangeset} API
- * FIXME move to .internal once access to package-local HgChangeset cons is resolved
+ * TODO post-1.0 Move to .internal once access to package-local HgChangeset cons is resolved. For 1.0, enough it's package-local 
  * 
  * @author Artem Tikhomirov
  * @author TMate Software Ltd.
@@ -109,7 +109,10 @@
 			changeset.setParentHelper(pw);
 		}
 
-		// FIXME document instance reuse policy
+		/**
+		 * Callers shall not assume they get new HgChangeset instance each time, implementation may reuse instances.  
+		 * @return hi-level changeset description
+		 */
 		HgChangeset handle(int revisionNumber, Nodeid nodeid, RawChangeset cset) {
 			changeset.init(revisionNumber, nodeid, cset);
 			return changeset;
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/core/HgCloneCommand.java
--- a/src/org/tmatesoft/hg/core/HgCloneCommand.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/core/HgCloneCommand.java	Thu Mar 22 21:02:20 2012 +0100
@@ -208,7 +208,7 @@
 				base = -1;
 				offset = 0;
 				revisionSequence.clear();
-				fncacheFiles.add("data/" + name + ".i"); // FIXME this is pure guess, 
+				fncacheFiles.add("data/" + name + ".i"); // TODO post-1.0 this is pure guess, 
 				// need to investigate more how filenames are kept in fncache
 				File file = new File(hgDir, filename = storagePathHelper.rewrite(name).toString());
 				file.getParentFile().mkdirs();
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/core/HgLogCommand.java
--- a/src/org/tmatesoft/hg/core/HgLogCommand.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/core/HgLogCommand.java	Thu Mar 22 21:02:20 2012 +0100
@@ -114,7 +114,7 @@
 	// multiple?
 	public HgLogCommand date(Calendar date) {
 		this.date = date;
-		// FIXME implement
+		// TODO post-1.0 implement
 		// isSet(field) - false => don't use in detection of 'same date'
 		throw HgRepository.notImplemented();
 	}
@@ -369,7 +369,7 @@
 			}
 		}
 		if (date != null) {
-			// FIXME implement date support for log
+			// TODO post-1.0 implement date support for log
 		}
 		count++;
 		csetTransform.next(revisionNumber, nodeid, cset);
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/core/HgStatus.java
--- a/src/org/tmatesoft/hg/core/HgStatus.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/core/HgStatus.java	Thu Mar 22 21:02:20 2012 +0100
@@ -91,8 +91,9 @@
 			if (localFile.canRead()) {
 				return new Date(localFile.lastModified());
 			}
-			// FIXME check dirstate and/or local file for tstamp
-			return new Date(); // what's correct 
+			// TODO post-1.0 find out what to do in this case, perhaps, throw an exception?
+			// perhaps check dirstate and/or local file for tstamp
+			return new Date(); // what's correct? 
 		} else {
 			return cset.date();
 		}
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/core/HgStatusCommand.java
--- a/src/org/tmatesoft/hg/core/HgStatusCommand.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/core/HgStatusCommand.java	Thu Mar 22 21:02:20 2012 +0100
@@ -254,7 +254,7 @@
 		}
 		public void copied(Path fnameOrigin, Path fnameAdded) {
 			if (needCopies) {
-				// FIXME in fact, merged files may report 'copied from' as well, correct status kind thus may differ from Added
+				// TODO post-1.0 in fact, merged files may report 'copied from' as well, correct status kind thus may differ from Added
 				handler.handleStatus(new HgStatus(Added, fnameAdded, fnameOrigin, logHelper));
 			}
 		}
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/internal/DataAccess.java
--- a/src/org/tmatesoft/hg/internal/DataAccess.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/internal/DataAccess.java	Thu Mar 22 21:02:20 2012 +0100
@@ -87,7 +87,8 @@
 		throw new IOException(String.format("No data, can't read %d bytes", length));
 	}
 	// reads bytes into ByteBuffer, up to its limit or total data length, whichever smaller
-	// FIXME perhaps, in DataAccess paradigm (when we read known number of bytes, we shall pass specific byte count to read) 
+	// TODO post-1.0 perhaps, in DataAccess paradigm (when we read known number of bytes, we shall pass specific byte count to read)
+	// for 1.0, it's ok as it's our internal class
 	public void readBytes(ByteBuffer buf) throws IOException {
 //		int toRead = Math.min(buf.remaining(), (int) length());
 //		if (buf.hasArray()) {
@@ -97,7 +98,7 @@
 //			readBytes(bb, 0, bb.length);
 //			buf.put(bb);
 //		}
-		// FIXME optimize to read as much as possible at once
+		// TODO post-1.0 optimize to read as much as possible at once
 		while (!isEmpty() && buf.hasRemaining()) {
 			buf.put(readByte());
 		}
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/internal/EncodingHelper.java
--- a/src/org/tmatesoft/hg/internal/EncodingHelper.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/internal/EncodingHelper.java	Thu Mar 22 21:02:20 2012 +0100
@@ -48,16 +48,13 @@
 		encoder = fsEncoding.newEncoder();
 	}
 
+	/**
+	 * Translate file names from manifest to amazing Unicode string 
+	 */
 	public String fromManifest(byte[] data, int start, int length) {
-		try {
-			return decoder.decode(ByteBuffer.wrap(data, start, length)).toString();
-		} catch (CharacterCodingException ex) {
-			sessionContext.getLog().error(getClass(), ex, String.format("Use of charset %s failed, resort to system default", charset().name()));
-			// resort to system-default
-			return new String(data, start, length);
-		}
+		return decodeWithSystemDefaultFallback(data, start, length);
 	}
-
+	
 	/**
 	 * @return byte representation of the string directly comparable to bytes in manifest
 	 */
@@ -80,11 +77,24 @@
 		}
 	}
 
-	public String fromDirstate(byte[] data, int start, int length) throws CharacterCodingException { // FIXME perhaps, log is enough, and charset() may be private?
-		return decoder.decode(ByteBuffer.wrap(data, start, length)).toString();
+	/**
+	 * Translate file names from dirstate to amazing Unicode string 
+	 */
+	public String fromDirstate(byte[] data, int start, int length) {
+		return decodeWithSystemDefaultFallback(data, start, length);
 	}
 
-	public Charset charset() {
+	private String decodeWithSystemDefaultFallback(byte[] data, int start, int length) {
+		try {
+			return decoder.decode(ByteBuffer.wrap(data, start, length)).toString();
+		} catch (CharacterCodingException ex) {
+			sessionContext.getLog().error(getClass(), ex, String.format("Use of charset %s failed, resort to system default", charset().name()));
+			// resort to system-default
+			return new String(data, start, length);
+		}
+	}
+
+	private Charset charset() {
 		return encoder.charset();
 	}
 
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/internal/Internals.java
--- a/src/org/tmatesoft/hg/internal/Internals.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/internal/Internals.java	Thu Mar 22 21:02:20 2012 +0100
@@ -299,7 +299,7 @@
 		if (f.canRead() && f.isDirectory()) {
 			return listConfigFiles(f);
 		}
-		// FIXME query registry, e.g. with
+		// TODO post-1.0 query registry, e.g. with
 		// Runtime.exec("reg query HKLM\Software\Mercurial")
 		//
 		f = new File("C:\\Mercurial\\Mercurial.ini");
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/internal/KeywordFilter.java
--- a/src/org/tmatesoft/hg/internal/KeywordFilter.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/internal/KeywordFilter.java	Thu Mar 22 21:02:20 2012 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011 TMate Software Ltd
+ * Copyright (c) 2011-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
@@ -71,7 +71,7 @@
 				l = s.length();
 			}
 		}
-		// FIXME later may implement #filter() not to read full kw value (just "$kw:"). However, limit of maxLen + 2 would keep valid.
+		// TODO post-1.0 later may implement #filter() not to read full kw value (just "$kw:"). However, limit of maxLen + 2 would keep valid.
 		// for buffers less then minBufferLen, there are chances #filter() implementation would never end
 		// (i.e. for input "$LongestKey"$
 		minBufferLen = l + 2 + (isExpanding ? 0 : 120 /*any reasonable constant for max possible kw value length*/);
@@ -113,7 +113,7 @@
 					// end of buffer reached
 					if (rv == null) {
 						if (keywordStart == x) {
-							// FIXME in fact, x might be equal to keywordStart and to src.position() here ('$' is first character in the buffer, 
+							// TODO post-1.0 in fact, x might be equal to keywordStart and to src.position() here ('$' is first character in the buffer, 
 							// and there are no other '$' not eols till the end of the buffer). This would lead to deadlock (filter won't consume any
 							// bytes). To prevent this, either shall copy bytes [keywordStart..buffer.limit()) to local buffer and use it on the next invocation,
 							// or add lookup of the keywords right after first '$' is found (do not wait for closing '$'). For now, large enough src buffer would be sufficient
@@ -259,7 +259,8 @@
 
 	private String revision() {
 		try {
-			// FIXME add cset's nodeid into Changeset class
+			// TODO post-1.0 Either add cset's nodeid into Changeset class or use own inspector 
+			// when accessing changelog, see below, #getChangeset
 			int csetRev = repo.getFileNode(path).getChangesetRevisionIndex(HgRepository.TIP);
 			return repo.getChangelog().getRevision(csetRev).shortNotation();
 		} catch (HgException ex) {
@@ -290,7 +291,10 @@
 	
 	private RawChangeset getChangeset() throws HgInvalidControlFileException {
 		if (latestFileCset == null) {
-			// XXX consider use of ChangelogHelper
+			// TODO post-1.0 Use of TIP is likely incorrect in cases when working copy is not based
+			// on latest revision. Perhaps, a constant like HgRepository.DIRSTATE_PARENT may come handy
+			// Besides, it's reasonable to pass own inspector instead of implicit use of RawCsetCollector
+			// to get changeset nodeid/index right away. Also check ChangelogHelper if may be of any use
 			int csetRev = repo.getFileNode(path).getChangesetRevisionIndex(HgRepository.TIP);
 			latestFileCset = repo.getChangelog().range(csetRev, csetRev).get(0);
 		}
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/internal/PathGlobMatcher.java
--- a/src/org/tmatesoft/hg/internal/PathGlobMatcher.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/internal/PathGlobMatcher.java	Thu Mar 22 21:02:20 2012 +0100
@@ -52,7 +52,7 @@
 	// HgIgnore.glob2regex is similar, but IsIgnore solves slightly different task 
 	// (need to match partial paths, e.g. for glob 'bin' shall match not only 'bin' folder, but also any path below it,
 	// which is not generally the case
-	private static String glob2regexp(String glob) { // FIXME TESTS NEEDED!!!
+	private static String glob2regexp(String glob) { // TODO TESTS NEEDED!!!
 		int end = glob.length() - 1;
 		if (glob.length() > 2 && glob.charAt(end) == '*' && glob.charAt(end - 1) == '.') {
 			end-=2;
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/internal/StoragePathHelper.java
--- a/src/org/tmatesoft/hg/internal/StoragePathHelper.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/internal/StoragePathHelper.java	Thu Mar 22 21:02:20 2012 +0100
@@ -61,9 +61,10 @@
 		charEncodingBuf = CharBuffer.allocate(1);
 	}
 
-	// FIXME document what path argument is, whether it includes .i or .d, and whether it's 'normalized' (slashes) or not.
-	// since .hg/store keeps both .i files and files without extension (e.g. fncache), guees, for data == false 
-	// we shall assume path has extension
+	/**
+	 * path argument is repository-relative name of the user's file.
+	 * It has to be normalized (slashes) and shall not include extension .i or .d.
+	 */
 	public CharSequence rewrite(CharSequence p) {
 		final String STR_STORE = "store/";
 		final String STR_DATA = "data/";
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/repo/HgBundle.java
--- a/src/org/tmatesoft/hg/repo/HgBundle.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/repo/HgBundle.java	Thu Mar 22 21:02:20 2012 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011 TMate Software Ltd
+ * Copyright (c) 2011-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
@@ -150,7 +150,7 @@
 								throw new IllegalStateException(String.format("Revision %s needs a parent %s, which is missing in the supplied repo %s", ge.node().shortNotation(), base.shortNotation(), hgRepo.toString()));
 							}
 							ByteArrayChannel bac = new ByteArrayChannel();
-							changelog.rawContent(base, bac); // FIXME get DataAccess directly, to avoid
+							changelog.rawContent(base, bac); // TODO post-1.0 get DataAccess directly, to avoid
 							// extra byte[] (inside ByteArrayChannel) duplication just for the sake of subsequent ByteArrayDataChannel wrap.
 							prevRevContent = new ByteArrayDataAccess(bac.toArray());
 						}
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/repo/HgChangelog.java
--- a/src/org/tmatesoft/hg/repo/HgChangelog.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/repo/HgChangelog.java	Thu Mar 22 21:02:20 2012 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2011 TMate Software Ltd
+ * Copyright (c) 2010-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
@@ -325,7 +325,7 @@
 			String _comment;
 			try {
 				_comment = new String(data, breakIndex4 + 2, bufferEndIndex - breakIndex4 - 2, "UTF-8");
-				// FIXME respect ui.fallbackencoding and try to decode if set
+				// TODO post-1.0 respect ui.fallbackencoding and try to decode if set; use EncodingHelper
 			} catch (UnsupportedEncodingException ex) {
 				_comment = "";
 				// Could hardly happen
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/repo/HgDirstate.java
--- a/src/org/tmatesoft/hg/repo/HgDirstate.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/repo/HgDirstate.java	Thu Mar 22 21:02:20 2012 +0100
@@ -23,7 +23,6 @@
 import java.io.FileNotFoundException;
 import java.io.FileReader;
 import java.io.IOException;
-import java.nio.charset.CharacterCodingException;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
@@ -147,8 +146,6 @@
 					repo.getContext().getLog().warn(getClass(), "Dirstate record for file %s (size: %d, tstamp:%d) has unknown state '%c'", r.name1, r.size(), r.time, state);
 				}
 			}
-		} catch (CharacterCodingException ex) {
-			throw new HgInvalidControlFileException(String.format("Failed reading file names from dirstate using encoding %s", encodingHelper.charset().name()), ex, dirstateFile);
 		} catch (IOException ex) {
 			throw new HgInvalidControlFileException("Dirstate read failed", ex, dirstateFile); 
 		} finally {
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/repo/HgRepository.java
--- a/src/org/tmatesoft/hg/repo/HgRepository.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/repo/HgRepository.java	Thu Mar 22 21:02:20 2012 +0100
@@ -180,7 +180,7 @@
 			tags = new HgTags(this);
 			HgDataFile hgTags = getFileNode(".hgtags");
 			if (hgTags.exists()) {
-				for (int i = 0; i <= hgTags.getLastRevision(); i++) { // FIXME in fact, would be handy to have walk(start,end) 
+				for (int i = 0; i <= hgTags.getLastRevision(); i++) { // TODO post-1.0 in fact, would be handy to have walk(start,end) 
 					// method for data files as well, though it looks odd.
 					try {
 						ByteArrayChannel sink = new ByteArrayChannel();
@@ -315,8 +315,8 @@
 		return repoDir;
 	}
 	
-	// FIXME remove once NPE in HgWorkingCopyStatusCollector.areTheSame is solved
 	/*package-local, debug*/String getStoragePath(HgDataFile df) {
+		// may come handy for debug
 		return dataPathHelper.rewrite(df.getPath().toString()).toString();
 	}
 
diff -r ccd7d25e5aea -r 528b6780a8bd src/org/tmatesoft/hg/repo/Revlog.java
--- a/src/org/tmatesoft/hg/repo/Revlog.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/src/org/tmatesoft/hg/repo/Revlog.java	Thu Mar 22 21:02:20 2012 +0100
@@ -215,8 +215,8 @@
 		} catch (IOException ex) {
 			HgInvalidControlFileException e = new HgInvalidControlFileException(String.format("Access to revision %d content failed", revisionIndex), ex, null);
 			e.setRevisionIndex(revisionIndex);
-			// FIXME e.setFileName(content.getIndexFile() or this.getHumanFriendlyPath()) - shall decide whether 
-			// protected abstract getPath() with impl in HgDataFile, HgManifest and HgChangelog or path is data of either Revlog or RevlogStream
+			// TODO post 1.0 e.setFileName(content.getIndexFile() or this.getHumanFriendlyPath()) - shall decide whether 
+			// protected abstract getHFPath() with impl in HgDataFile, HgManifest and HgChangelog or path is data of either Revlog or RevlogStream
 			// Do the same (add file name) below
 			throw e;
 		} catch (HgInvalidControlFileException ex) {
@@ -236,7 +236,7 @@
 	 * @param parent1 - byte[20] or null, if parent's nodeid is not needed
 	 * @param parent2 - byte[20] or null, if second parent's nodeid is not needed
 	 * @throws HgInvalidRevisionException
-	 * @throws HgInvalidControlFileException FIXME
+	 * @throws HgInvalidControlFileException FIXME EXCEPTIONS
 	 * @throws IllegalArgumentException
 	 */
 	public void parents(int revision, int[] parentRevisions, byte[] parent1, byte[] parent2) throws HgInvalidRevisionException, HgInvalidControlFileException {
diff -r ccd7d25e5aea -r 528b6780a8bd test/org/tmatesoft/hg/test/TestAuxUtilities.java
--- a/test/org/tmatesoft/hg/test/TestAuxUtilities.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/test/org/tmatesoft/hg/test/TestAuxUtilities.java	Thu Mar 22 21:02:20 2012 +0100
@@ -222,7 +222,7 @@
 	}
 
 	@Test
-	public void testRevlogInspectors() throws Exception { // FIXME move to better place
+	public void testRevlogInspectors() throws Exception { // TODO move to better place
 		HgRepository repository = Configuration.get().find("branches-1"); // any repo
 		repository.getChangelog().walk(0, TIP, new HgChangelog.RevisionInspector() {
 
diff -r ccd7d25e5aea -r 528b6780a8bd test/org/tmatesoft/hg/test/TestStatus.java
--- a/test/org/tmatesoft/hg/test/TestStatus.java	Thu Mar 22 20:14:06 2012 +0100
+++ b/test/org/tmatesoft/hg/test/TestStatus.java	Thu Mar 22 21:02:20 2012 +0100
@@ -646,7 +646,7 @@
 		reportNotEqual(what + "#IGNORED", r.get(Ignored), statusParser.getIgnored());
 		reportNotEqual(what + "#MISSING", r.get(Missing), statusParser.getMissing());
 		reportNotEqual(what + "#UNKNOWN", r.get(Unknown), statusParser.getUnknown());
-		// FIXME test copies
+		// TODO test copies
 	}
 
 	private void report(String what, HgStatusCollector.Record r, StatusOutputParser statusParser) {