Mercurial > jhg

changeset 405:866fc3b597a0

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Add an explicit constant instead of -1 to indicate 'no revision' case
author Artem Tikhomirov <tikhomirov.artem@gmail.com>
date Wed, 14 Mar 2012 22:49:32 +0100
parents 2747b0723867
children d56ea1a2537a
files src/org/tmatesoft/hg/core/HgInvalidRevisionException.java src/org/tmatesoft/hg/repo/HgInternals.java src/org/tmatesoft/hg/repo/HgRepository.java src/org/tmatesoft/hg/repo/HgStatusCollector.java src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java src/org/tmatesoft/hg/repo/Revlog.java
diffstat 6 files changed, 69 insertions(+), 28 deletions(-) [+]
line wrap: on
line diff
--- a/src/org/tmatesoft/hg/core/HgInvalidRevisionException.java	Mon Mar 05 14:50:51 2012 +0100
+++ b/src/org/tmatesoft/hg/core/HgInvalidRevisionException.java	Wed Mar 14 22:49:32 2012 +0100
@@ -32,7 +32,7 @@
 	private Nodeid rev;
 	private Integer revIdx = BAD_REVISION;
 	// next two make sense only when revIdx is present
-	private int rangeLeftBoundary = -1, rangeRightBoundary = -1;
+	private int rangeLeftBoundary = BAD_REVISION, rangeRightBoundary = BAD_REVISION;
 
 	/**
 	 * 
@@ -110,9 +110,10 @@
 			case BAD_REVISION : sr = "UNKNOWN"; break;
 			case TIP : sr = "TIP"; break;
 			case WORKING_COPY: sr = "WORKING-COPY"; break;
+			case NO_REVISION : sr = "NO REVISION"; break;
 			default : sr = revIdx.toString();
 			}
-			if (rangeLeftBoundary != -1 || rangeRightBoundary != -1) {
+			if (rangeLeftBoundary != BAD_REVISION || rangeRightBoundary != BAD_REVISION) {
 				sb.append(String.format("%s is not from [%d..%d]", sr, rangeLeftBoundary, rangeRightBoundary));
 			} else {
 				sb.append(sr);
--- a/src/org/tmatesoft/hg/repo/HgInternals.java	Mon Mar 05 14:50:51 2012 +0100
+++ b/src/org/tmatesoft/hg/repo/HgInternals.java	Wed Mar 14 22:49:32 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
@@ -137,7 +137,7 @@
 	// Convenient check of revision index for validity (not all negative values are wrong as long as we use negative constants)
 	public static boolean wrongRevisionIndex(int rev) { // FIXME guess, usages shall throw HgInvalidRevision. \
 		// TODO Another method to check,throw and expand TIP at once
-		return rev < 0 && rev != TIP && rev != WORKING_COPY && rev != BAD_REVISION; 
+		return rev < 0 && rev != TIP && rev != WORKING_COPY && rev != BAD_REVISION && rev != NO_REVISION; 
 	}
 	
 	// throws HgInvalidRevisionException or IllegalArgumentException if [start..end] range is not a subrange of [0..lastRevision]
--- a/src/org/tmatesoft/hg/repo/HgRepository.java	Mon Mar 05 14:50:51 2012 +0100
+++ b/src/org/tmatesoft/hg/repo/HgRepository.java	Wed Mar 14 22:49:32 2012 +0100
@@ -27,6 +27,7 @@
 
 import org.tmatesoft.hg.core.HgException;
 import org.tmatesoft.hg.core.HgInvalidControlFileException;
+import org.tmatesoft.hg.core.HgInvalidRevisionException;
 import org.tmatesoft.hg.core.Nodeid;
 import org.tmatesoft.hg.core.SessionContext;
 import org.tmatesoft.hg.internal.ByteArrayChannel;
@@ -53,11 +54,35 @@
  */
 public final class HgRepository {
 
-	// if new constants added, consider fixing HgInternals#wrongRevisionIndex
-	public static final int TIP = -3;
+	// IMPORTANT: if new constants added, consider fixing HgInternals#wrongRevisionIndex and HgInvalidRevisionException#getMessage
+
+	/**
+	 * Revision index constant to indicate most recent revision
+	 */
+	public static final int TIP = -3; // XXX TIP_REVISION?
+
+	/**
+	 * Revision index constant to indicate invalid revision index value. 
+	 * Primary use is default/uninitialized values where user input is expected and as return value where 
+	 * an exception (e.g. {@link HgInvalidRevisionException}) is not desired
+	 */
 	public static final int BAD_REVISION = Integer.MIN_VALUE; // XXX INVALID_REVISION?
-	public static final int WORKING_COPY = -2;
+
+	/**
+	 * Revision index constant to indicate working copy
+	 */
+	public static final int WORKING_COPY = -2; // XXX WORKING_COPY_REVISION?
 	
+	/**
+	 * Constant ({@value #NO_REVISION}) to indicate revision absence (e.g. missing parent in from {@link HgChangelog#parents(int, int[], byte[], byte[])} call) 
+	 * or a fictitious revision of an empty repository, to use as an argument (contrary to {@link #BAD_REVISION})
+	 * e.g in a status operation to visit changes from the very beginning of a repository. 
+	 */
+	public static final int NO_REVISION = -1;
+	
+	/**
+	 * Name of the primary branch, "default".
+	 */
 	public static final String DEFAULT_BRANCH_NAME = "default";
 
 	// temp aux marker method
--- a/src/org/tmatesoft/hg/repo/HgStatusCollector.java	Mon Mar 05 14:50:51 2012 +0100
+++ b/src/org/tmatesoft/hg/repo/HgStatusCollector.java	Wed Mar 14 22:49:32 2012 +0100
@@ -77,7 +77,7 @@
 	private ManifestRevision get(int rev) throws HgInvalidControlFileException {
 		ManifestRevision i = cache.get(rev);
 		if (i == null) {
-			if (rev == -1) {
+			if (rev == NO_REVISION) {
 				return emptyFakeState;
 			}
 			ensureCacheSize();
@@ -89,7 +89,7 @@
 	}
 
 	private boolean cached(int revision) {
-		return cache.containsKey(revision) || revision == -1;
+		return cache.containsKey(revision) || revision == NO_REVISION;
 	}
 	
 	private void ensureCacheSize() {
@@ -117,7 +117,7 @@
 
 			public boolean begin(int manifestRevision, Nodeid nid, int changelogRevision) {
 				assert delegate == null;
-				if (cache.containsKey(changelogRevision)) { // don't need to check emptyFakeState hit as revision never -1 here
+				if (cache.containsKey(changelogRevision)) { // don't need to check emptyFakeState hit as revision never NO_REVISION here
 					cacheHit = true;
 				} else {
 					cache.put(changelogRevision, delegate = new ManifestRevision(cacheNodes, cacheFilenames));
@@ -153,11 +153,17 @@
 	
 	/*package-local*/ static ManifestRevision createEmptyManifestRevision() {
 		ManifestRevision fakeEmptyRev = new ManifestRevision(null, null);
-		fakeEmptyRev.begin(-1, null, -1);
-		fakeEmptyRev.end(-1);
+		fakeEmptyRev.begin(NO_REVISION, null, NO_REVISION);
+		fakeEmptyRev.end(NO_REVISION);
 		return fakeEmptyRev;
 	}
 	
+	/**
+	 * Access specific manifest revision
+	 * @param rev 
+	 * @return
+	 * @throws HgInvalidControlFileException
+	 */
 	/*package-local*/ ManifestRevision raw(int rev) throws HgInvalidControlFileException {
 		return get(rev);
 	}
@@ -191,17 +197,23 @@
 	 * @throws HgInvalidControlFileException if access to revlog index/data entry failed
 	 */
 	public void change(int revisionIndex, HgStatusInspector inspector) throws HgInvalidRevisionException, HgInvalidControlFileException {
-		int[] parents = new int[2];
-		repo.getChangelog().parents(revisionIndex, parents, null, null);
-		walk(parents[0], revisionIndex, inspector);
+		int p;
+		if (revisionIndex == 0) {
+			p = NO_REVISION;
+		} else {
+			int[] parents = new int[2];
+			repo.getChangelog().parents(revisionIndex, parents, null, null);
+			// #parents call above is responsible for NO_REVISION
+			p = parents[0]; // hg --change alsways uses first parent, despite the fact there might be valid (-1, 18) pair of parents
+		}
+		walk(p, revisionIndex, inspector);
 	}
 	
 	/**
 	 * Parameters <b>rev1</b> and <b>rev2</b> are changelog revision indexes, shall not be the same. Argument order matters.
-	 * FIXME Either rev1 or rev2 may be -1 to indicate comparison to empty repository (this is due to use of
-	 * parents in #change(), I believe. Perhaps, need a constant for this? Otherwise this hidden knowledge gets
-	 * exposed to e.g. Record
-	 * XXX cancellation?
+	 * Either rev1 or rev2 may be {@link HgRepository#NO_REVISION} to indicate comparison to empty repository
+	 * 
+	 * FIXME cancellation (at least exception)?
 	 * 
 	 * @param rev1 <em>from</em> changeset index, non-negative or {@link HgRepository#TIP}
 	 * @param rev2 <em>to</em> changeset index, non-negative or {@link HgRepository#TIP}
@@ -224,10 +236,10 @@
 		if (rev2 == TIP) {
 			rev2 = lastChangelogRevision; 
 		}
-		if (rev1 != -1 && (HgInternals.wrongRevisionIndex(rev1) || rev1 == WORKING_COPY || rev1 == BAD_REVISION || rev1 > lastChangelogRevision)) {
+		if (rev1 != NO_REVISION && (HgInternals.wrongRevisionIndex(rev1) || rev1 == WORKING_COPY || rev1 == BAD_REVISION || rev1 > lastChangelogRevision)) {
 			throw new HgInvalidRevisionException(rev1);
 		}
-		if (rev2 != -1 && (HgInternals.wrongRevisionIndex(rev2) || rev2 == WORKING_COPY || rev2 == BAD_REVISION || rev2 > lastChangelogRevision)) {
+		if (rev2 != NO_REVISION && (HgInternals.wrongRevisionIndex(rev2) || rev2 == WORKING_COPY || rev2 == BAD_REVISION || rev2 > lastChangelogRevision)) {
 			throw new HgInvalidRevisionException(rev2);
 		}
 		if (inspector instanceof Record) {
--- a/src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java	Mon Mar 05 14:50:51 2012 +0100
+++ b/src/org/tmatesoft/hg/repo/HgWorkingCopyStatusCollector.java	Wed Mar 14 22:49:32 2012 +0100
@@ -137,7 +137,7 @@
 	private void initDirstateParentManifest() throws HgInvalidControlFileException {
 		Nodeid dirstateParent = getDirstateImpl().parents().first();
 		if (dirstateParent.isNull()) {
-			dirstateParentManifest = baseRevisionCollector != null ? baseRevisionCollector.raw(-1) : HgStatusCollector.createEmptyManifestRevision();
+			dirstateParentManifest = baseRevisionCollector != null ? baseRevisionCollector.raw(NO_REVISION) : HgStatusCollector.createEmptyManifestRevision();
 		} else {
 			int changeloRevIndex = repo.getChangelog().getRevisionIndex(dirstateParent);
 			dirstateParentManifest = getManifest(changeloRevIndex);
--- a/src/org/tmatesoft/hg/repo/Revlog.java	Mon Mar 05 14:50:51 2012 +0100
+++ b/src/org/tmatesoft/hg/repo/Revlog.java	Wed Mar 14 22:49:32 2012 +0100
@@ -17,6 +17,7 @@
 package org.tmatesoft.hg.repo;
 
 import static org.tmatesoft.hg.repo.HgRepository.BAD_REVISION;
+import static org.tmatesoft.hg.repo.HgRepository.NO_REVISION;
 import static org.tmatesoft.hg.repo.HgRepository.TIP;
 
 import java.io.IOException;
@@ -228,13 +229,14 @@
 	}
 
 	/**
-	 * XXX perhaps, return value Nodeid[2] and boolean needNodeids is better (and higher level) API for this query?
+	 * Fills supplied arguments with information about revision parents.
 	 * 
 	 * @param revision - revision to query parents, or {@link HgRepository#TIP}
-	 * @param parentRevisions - int[2] to get local revision numbers of parents (e.g. {6, -1})
+	 * @param parentRevisions - int[2] to get local revision numbers of parents (e.g. {6, -1}), {@link HgRepository#NO_REVISION} indicates parent not set
 	 * @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 IllegalArgumentException
 	 */
 	public void parents(int revision, int[] parentRevisions, byte[] parent1, byte[] parent2) throws HgInvalidRevisionException, HgInvalidControlFileException {
@@ -265,10 +267,11 @@
 		};
 		ParentCollector pc = new ParentCollector();
 		content.iterate(revision, revision, false, pc);
-		parentRevisions[0] = pc.p1;
-		parentRevisions[1] = pc.p2;
+		// although next code looks odd (NO_REVISION *is* -1), it's safer to be explicit
+		parentRevisions[0] = pc.p1 == -1 ? NO_REVISION : pc.p1;
+		parentRevisions[1] = pc.p2 == -1 ? NO_REVISION : pc.p2;
 		if (parent1 != null) {
-			if (parentRevisions[0] == -1) {
+			if (parentRevisions[0] == NO_REVISION) {
 				Arrays.fill(parent1, 0, 20, (byte) 0);
 			} else {
 				content.iterate(parentRevisions[0], parentRevisions[0], false, pc);
@@ -276,7 +279,7 @@
 			}
 		}
 		if (parent2 != null) {
-			if (parentRevisions[1] == -1) {
+			if (parentRevisions[1] == NO_REVISION) {
 				Arrays.fill(parent2, 0, 20, (byte) 0);
 			} else {
 				content.iterate(parentRevisions[1], parentRevisions[1], false, pc);