changeset 625:b4948b159ab1

Refactor internals of blame support, tests
author Artem Tikhomirov <tikhomirov.artem@gmail.com>
date Tue, 21 May 2013 17:24:22 +0200
parents 507602cb4fb3
children 5afc7eedb3dd
files src/org/tmatesoft/hg/internal/BlameHelper.java src/org/tmatesoft/hg/internal/FileRevisionHistoryChunk.java src/org/tmatesoft/hg/repo/HgDataFile.java test/org/tmatesoft/hg/test/TestBlame.java
diffstat 4 files changed, 97 insertions(+), 68 deletions(-) [+]
line wrap: on
line diff
--- a/src/org/tmatesoft/hg/internal/BlameHelper.java	Mon May 20 20:34:33 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/BlameHelper.java	Tue May 21 17:24:22 2013 +0200
@@ -16,6 +16,7 @@
  */
 package org.tmatesoft.hg.internal;
 
+import static org.tmatesoft.hg.core.HgIterateDirection.OldToNew;
 import static org.tmatesoft.hg.repo.HgRepository.NO_REVISION;
 
 import java.util.LinkedList;
@@ -48,15 +49,38 @@
 	private final HgBlameInspector insp;
 	private FileLinesCache linesCache;
 
-	// FIXME exposing internals (use of FileLinesCache through cons arg and #useFileUpTo) smells bad, refactor!
-
-	public BlameHelper(HgBlameInspector inspector, int cacheHint) {
+	public BlameHelper(HgBlameInspector inspector) {
 		insp = inspector;
-		linesCache = new FileLinesCache(cacheHint);
 	}
-	
-	public void useFileUpTo(HgDataFile df, int clogRevIndex) {
-		linesCache.useFileUpTo(df, clogRevIndex);
+
+	/**
+	 * Build history of the file for the specified range (follow renames if necessary). This history
+	 * is used to access various file revision data during subsequent {@link #diff(int, int, int, int)} and
+	 * {@link #annotateChange(int, int, int[], int[])} calls. Callers can use returned history for own approaches 
+	 * to iteration over file history.
+
+	 * <p>NOTE, clogRevIndexEnd has to list name of the supplied file in the corresponding manifest,
+	 * as it's not possible to trace rename history otherwise.
+	 */
+	public FileHistory prepare(HgDataFile df, int clogRevIndexStart, int clogRevIndexEnd) {
+		assert clogRevIndexStart <= clogRevIndexEnd;
+		FileHistory fileHistory = new FileHistory(df, clogRevIndexStart, clogRevIndexEnd);
+		fileHistory.build();
+		int cacheHint = 5; // cache comes useful when we follow merge branches and don't want to
+		// parse base revision twice. There's no easy way to determine max(distance(all(base,merge))),
+		// hence the heuristics to use the longest history chunk:
+		for (FileRevisionHistoryChunk c : fileHistory.iterate(OldToNew)) {
+			// iteration order is not important here
+			if (c.revisionCount() > cacheHint) {
+				cacheHint = c.revisionCount();
+			}
+		}
+		linesCache = new FileLinesCache(cacheHint);
+		for (FileRevisionHistoryChunk fhc : fileHistory.iterate(OldToNew)) {
+			// iteration order is not important here
+			linesCache.useFileUpTo(fhc.getFile(), fhc.getEndChangeset());
+		}
+		return fileHistory;
 	}
 	
 	// NO_REVISION is not allowed as any argument
@@ -116,6 +140,9 @@
 		private final int limit;
 		private final LinkedList<Pair<Integer, HgDataFile>> files; // TODO in fact, need sparse array 
 
+		/**
+		 * @param lruLimit how many parsed file revisions to keep
+		 */
 		public FileLinesCache(int lruLimit) {
 			limit = lruLimit;
 			lruCache = new LinkedList<Pair<Integer, LineSequence>>();
--- a/src/org/tmatesoft/hg/internal/FileRevisionHistoryChunk.java	Mon May 20 20:34:33 2013 +0200
+++ b/src/org/tmatesoft/hg/internal/FileRevisionHistoryChunk.java	Tue May 21 17:24:22 2013 +0200
@@ -163,6 +163,13 @@
 		return rv;
 	}
 	
+	/**
+	 * @return number of file revisions in this chunk of its history
+	 */
+	public int revisionCount() {
+		return fileRevsToVisit.size();
+	}
+	
 	public int changeset(int fileRevIndex) {
 		return file2changelog[fileRevIndex];
 	}
--- a/src/org/tmatesoft/hg/repo/HgDataFile.java	Mon May 20 20:34:33 2013 +0200
+++ b/src/org/tmatesoft/hg/repo/HgDataFile.java	Tue May 21 17:24:22 2013 +0200
@@ -16,7 +16,6 @@
  */
 package org.tmatesoft.hg.repo;
 
-import static org.tmatesoft.hg.core.HgIterateDirection.OldToNew;
 import static org.tmatesoft.hg.repo.HgInternals.wrongRevisionIndex;
 import static org.tmatesoft.hg.repo.HgRepository.*;
 import static org.tmatesoft.hg.util.LogFacility.Severity.Info;
@@ -430,12 +429,10 @@
 	 * mimic 'hg diff -r clogRevIndex1 -r clogRevIndex2'
 	 */
 	public void diff(int clogRevIndex1, int clogRevIndex2, HgBlameInspector insp) throws HgCallbackTargetException {
-		// FIXME clogRevIndex1 and clogRevIndex2 may point to different files, need to decide whether to throw an exception
-		// or to attempt to look up correct file node (tricky)
 		int fileRevIndex1 = fileRevIndex(this, clogRevIndex1);
 		int fileRevIndex2 = fileRevIndex(this, clogRevIndex2);
-		BlameHelper bh = new BlameHelper(insp, 5);
-		bh.useFileUpTo(this, clogRevIndex2);
+		BlameHelper bh = new BlameHelper(insp);
+		bh.prepare(this, clogRevIndex1, clogRevIndex2);
 		bh.diff(fileRevIndex1, clogRevIndex1, fileRevIndex2, clogRevIndex2);
 	}
 	
@@ -463,13 +460,9 @@
 		if (!exists()) {
 			return;
 		}
-		FileHistory fileHistory = new FileHistory(this, changelogRevIndexStart, changelogRevIndexEnd);
-		fileHistory.build();
-		BlameHelper bh = new BlameHelper(insp, 10);
-		for (FileRevisionHistoryChunk fhc : fileHistory.iterate(OldToNew)) {
-			// iteration order is not important here
-			bh.useFileUpTo(fhc.getFile(), fhc.getEndChangeset());
-		}
+		BlameHelper bh = new BlameHelper(insp);
+		FileHistory fileHistory = bh.prepare(this, changelogRevIndexStart, changelogRevIndexEnd);
+
 		int[] fileClogParentRevs = new int[2];
 		int[] fileParentRevs = new int[2];
 		for (FileRevisionHistoryChunk fhc : fileHistory.iterate(iterateOrder)) {
@@ -498,11 +491,12 @@
 		if (changelogRevisionIndex == TIP) {
 			changelogRevisionIndex = getChangesetRevisionIndex(fileRevIndex);
 		}
-		BlameHelper bh = new BlameHelper(insp, 5);
-		bh.useFileUpTo(this, changelogRevisionIndex);
 		int[] fileClogParentRevs = new int[2];
 		fileClogParentRevs[0] = fileRevParents[0] == NO_REVISION ? NO_REVISION : getChangesetRevisionIndex(fileRevParents[0]);
 		fileClogParentRevs[1] = fileRevParents[1] == NO_REVISION ? NO_REVISION : getChangesetRevisionIndex(fileRevParents[1]);
+		BlameHelper bh = new BlameHelper(insp);
+		int clogIndexStart = fileClogParentRevs[0] == NO_REVISION ? (fileClogParentRevs[1] == NO_REVISION ? 0 : fileClogParentRevs[1]) : fileClogParentRevs[0];
+		bh.prepare(this, clogIndexStart, changelogRevisionIndex);
 		bh.annotateChange(fileRevIndex, changelogRevisionIndex, fileRevParents, fileClogParentRevs);
 	}
 
--- a/test/org/tmatesoft/hg/test/TestBlame.java	Mon May 20 20:34:33 2013 +0200
+++ b/test/org/tmatesoft/hg/test/TestBlame.java	Tue May 21 17:24:22 2013 +0200
@@ -45,12 +45,14 @@
 import org.tmatesoft.hg.core.HgCallbackTargetException;
 import org.tmatesoft.hg.core.HgIterateDirection;
 import org.tmatesoft.hg.core.HgRepoFacade;
+import org.tmatesoft.hg.core.Nodeid;
 import org.tmatesoft.hg.internal.FileAnnotation;
 import org.tmatesoft.hg.internal.FileAnnotation.LineDescriptor;
 import org.tmatesoft.hg.internal.FileAnnotation.LineInspector;
 import org.tmatesoft.hg.internal.IntVector;
 import org.tmatesoft.hg.repo.HgBlameInspector;
 import org.tmatesoft.hg.repo.HgBlameInspector.BlockData;
+import org.tmatesoft.hg.repo.HgChangelog;
 import org.tmatesoft.hg.repo.HgDataFile;
 import org.tmatesoft.hg.repo.HgLookup;
 import org.tmatesoft.hg.repo.HgRepository;
@@ -71,7 +73,7 @@
 	public void testSingleParentBlame() throws Exception {
 		HgRepository repo = new HgLookup().detectFromWorkingDir();
 		final String fname = "src/org/tmatesoft/hg/internal/PatchGenerator.java";
-		final int checkChangeset = 539;
+		final int checkChangeset = repo.getChangelog().getRevisionIndex(Nodeid.fromAscii("946b131962521f9199e1fedbdc2487d3aaef5e46")); // 539
 		HgDataFile df = repo.getFileNode(fname);
 		ByteArrayOutputStream bos = new ByteArrayOutputStream();
 		df.annotateSingleRevision(checkChangeset, new DiffOutInspector(new PrintStream(bos)));
@@ -91,7 +93,12 @@
 		HgDataFile df = repo.getFileNode(fname);
 		AnnotateRunner ar = new AnnotateRunner(df.getPath(), null);
 
-		for (int cs : new int[] { 539, 541 /*, TIP */}) {
+		final HgChangelog clog = repo.getChangelog();
+		final int[] toTest = new int[] { 
+			clog.getRevisionIndex(Nodeid.fromAscii("946b131962521f9199e1fedbdc2487d3aaef5e46")), // 539
+			clog.getRevisionIndex(Nodeid.fromAscii("1e95f48d9886abe79b9711ab371bc877ca5e773e")), // 541 
+			/*, TIP */};
+		for (int cs : toTest) {
 			ar.run(cs, false);
 			FileAnnotateInspector fa = new FileAnnotateInspector();
 			FileAnnotation.annotate(df, cs, fa);
@@ -254,6 +261,45 @@
 			errorCollector.assertEquals(hgAnnotateLine.trim(), apiLine);
 		}
 	}
+	
+	
+	@Test
+	public void testDiffTwoRevisions() throws Exception {
+		HgRepository repo = Configuration.get().find("test-annotate");
+		HgDataFile df = repo.getFileNode("file1");
+		LineGrepOutputParser gp = new LineGrepOutputParser("^@@.+");
+		ExecHelper eh = new ExecHelper(gp, repo.getWorkingDir());
+		int[] toTest = { 3, 4, 5 }; // p1 ancestry line, p2 ancestry line, not in ancestry line
+		for (int cs : toTest) {
+			ByteArrayOutputStream bos = new ByteArrayOutputStream();
+			df.diff(cs, 8, new DiffOutInspector(new PrintStream(bos)));
+			eh.run("hg", "diff", "-r", String.valueOf(cs), "-r", "8", "-U", "0", df.getPath().toString());
+			//
+			String[] apiResult = splitLines(bos.toString());
+			String[] expected = splitLines(gp.result());
+			Assert.assertArrayEquals(expected, apiResult);
+			gp.reset();
+		}
+	}
+	
+	/**
+	 * Make sure boundary values are ok (down to BlameHelper#prepare and FileHistory) 
+	 */
+	@Test
+	public void testAnnotateFirstFileRev() throws Exception {
+		HgRepository repo = Configuration.get().find("test-annotate");
+		HgDataFile df = repo.getFileNode("file1");
+		LineGrepOutputParser gp = new LineGrepOutputParser("^@@.+");
+		ExecHelper eh = new ExecHelper(gp, repo.getWorkingDir());
+		eh.run("hg", "diff", "-c", "0", "-U", "0", df.getPath().toString());
+		//
+		ByteArrayOutputStream bos = new ByteArrayOutputStream();
+		df.annotateSingleRevision(0, new DiffOutInspector(new PrintStream(bos)));
+		//
+		String[] apiResult = splitLines(bos.toString());
+		String[] expected = splitLines(gp.result());
+		Assert.assertArrayEquals(expected, apiResult);
+	}
 
 	// TODO HgWorkingCopyStatusCollector (and HgStatusCollector), with their ancestors (rev 59/69) have examples
 	// of *incorrect* assignment of common lines (like "}") - our impl doesn't process common lines in any special way
@@ -285,47 +331,6 @@
 	}
 	
 	
-	private void aaa() throws Exception {
-		HgRepository repo = new HgLookup().detectFromWorkingDir();
-		final String fname = "src/org/tmatesoft/hg/internal/PatchGenerator.java";
-		final int checkChangeset = 539;
-		HgDataFile df = repo.getFileNode(fname);
-		DiffOutInspector dump = new DiffOutInspector(System.out);
-		System.out.println("541 -> 543");
-		df.annotateSingleRevision(543, dump);
-		System.out.println("539 -> 541");
-		df.annotateSingleRevision(541, dump);
-		System.out.println("536 -> 539");
-		df.annotateSingleRevision(checkChangeset, dump);
-		System.out.println("531 -> 536");
-		df.annotateSingleRevision(536, dump);
-		System.out.println(" -1 -> 531");
-		df.annotateSingleRevision(531, dump);
-		
-		FileAnnotateInspector fai = new FileAnnotateInspector();
-		FileAnnotation.annotate(df, 541, fai);
-		for (int i = 0; i < fai.lineRevisions.length; i++) {
-			System.out.printf("%3d: LINE %d\n", fai.lineRevisions[i], i+1);
-		}
-	}
-
-	private void bbb() throws Exception {
-		HgRepository repo = new HgLookup().detectFromWorkingDir();
-		final String fname = "src/org/tmatesoft/hg/repo/HgManifest.java";
-		final int checkChangeset = 415;
-		HgDataFile df = repo.getFileNode(fname);
-		DiffOutInspector dump = new DiffOutInspector(System.out);
-//		System.out.println("413 -> 415");
-//		af.diff(df, 413, 415, dump);
-//		System.out.println("408 -> 415");
-//		af.diff(df, 408, 415, dump);
-//		System.out.println("Combined (with merge):");
-//		dump.needRevisions(true);
-//		af.annotateChange(df, checkChangeset, dump);
-		dump.needRevisions(true);
-		df.annotate(checkChangeset, dump, HgIterateDirection.OldToNew);
-	}
-	
 	private void ccc() throws Throwable {
 		HgRepository repo = new HgLookup().detect("/home/artem/hg/hgtest-annotate-merge/");
 		HgDataFile df = repo.getFileNode("file.txt");
@@ -354,10 +359,6 @@
 	}
 
 	public static void main(String[] args) throws Throwable {
-//		System.out.println(Arrays.equals(new String[0], splitLines("")));
-//		System.out.println(Arrays.equals(new String[] { "abc" }, splitLines("abc")));
-//		System.out.println(Arrays.equals(new String[] { "a", "bc" }, splitLines("a\nbc")));
-//		System.out.println(Arrays.equals(new String[] { "a", "bc" }, splitLines("a\nbc\n")));
 		TestBlame tt = new TestBlame();
 		tt.ccc();
 	}