Mercurial > jhg
changeset 624:507602cb4fb3
FIXMEs and TODOs: pay some technical debt
| author | Artem Tikhomirov <tikhomirov.artem@gmail.com> |
|---|---|
| date | Mon, 20 May 2013 20:34:33 +0200 |
| parents | fedc54356091 |
| children | b4948b159ab1 |
| files | src/org/tmatesoft/hg/internal/DiffHelper.java src/org/tmatesoft/hg/internal/FNCacheFile.java src/org/tmatesoft/hg/internal/FileHistory.java src/org/tmatesoft/hg/internal/FileUtils.java src/org/tmatesoft/hg/internal/LineReader.java src/org/tmatesoft/hg/repo/HgBookmarks.java src/org/tmatesoft/hg/repo/ext/MqManager.java test/org/tmatesoft/hg/test/TestBlame.java test/org/tmatesoft/hg/test/TestCommit.java test/org/tmatesoft/hg/test/TestDiffHelper.java |
| diffstat | 10 files changed, 90 insertions(+), 37 deletions(-) [+] |
line wrap: on
line diff
--- a/src/org/tmatesoft/hg/internal/DiffHelper.java Mon May 20 18:35:13 2013 +0200 +++ b/src/org/tmatesoft/hg/internal/DiffHelper.java Mon May 20 20:34:33 2013 +0200 @@ -20,8 +20,6 @@ import java.util.HashMap; import java.util.Map; -import org.tmatesoft.hg.repo.HgInvalidStateException; - /** * Mercurial cares about changes only up to the line level, e.g. a simple file version dump in manifest looks like (RevlogDump output): * @@ -201,9 +199,7 @@ } else { assert changeStartS2 == matchStartSeq2; if (matchStartSeq1 > 0 || matchStartSeq2 > 0) { - // FIXME perhaps, exception is too much for the case - // once diff is covered with tests, replace with assert false : msg; - throw new HgInvalidStateException(String.format("adjustent equal blocks %d, %d and %d,%d", changeStartS1, matchStartSeq1, changeStartS2, matchStartSeq2)); + assert false : String.format("adjustent equal blocks %d, %d and %d,%d", changeStartS1, matchStartSeq1, changeStartS2, matchStartSeq2); } } }
--- a/src/org/tmatesoft/hg/internal/FNCacheFile.java Mon May 20 18:35:13 2013 +0200 +++ b/src/org/tmatesoft/hg/internal/FNCacheFile.java Mon May 20 20:34:33 2013 +0200 @@ -68,7 +68,7 @@ // names in fncache are in local encoding, shall translate to unicode new LineReader(f, repo.getSessionContext().getLog(), repo.getFilenameEncoding()).read(new LineReader.SimpleLineCollector(), entries); for (String e : entries) { - // FIXME plain wrong, need either to decode paths and strip off .i/.d or (if keep names as is) change write() + // XXX plain wrong, need either to decode paths and strip off .i/.d or (if keep names as is) change write() files.add(pathFactory.path(e)); } }
--- a/src/org/tmatesoft/hg/internal/FileHistory.java Mon May 20 18:35:13 2013 +0200 +++ b/src/org/tmatesoft/hg/internal/FileHistory.java Mon May 20 20:34:33 2013 +0200 @@ -30,7 +30,7 @@ * History of a file, with copy/renames, and corresponding revision information. * Facility for file history iteration. * - * FIXME [post-1.1] Utilize in HgLogCommand and anywhere else we need to follow file history + * TODO [post-1.1] Utilize in HgLogCommand and anywhere else we need to follow file history * * @author Artem Tikhomirov * @author TMate Software Ltd.
--- a/src/org/tmatesoft/hg/internal/FileUtils.java Mon May 20 18:35:13 2013 +0200 +++ b/src/org/tmatesoft/hg/internal/FileUtils.java Mon May 20 20:34:33 2013 +0200 @@ -82,6 +82,10 @@ String m = String.format("Failed to copy %s to %s", from.getName(), to.getName()); throw new HgIOException(m, ex, from); } + /* Copy of cpython's 00changelog.d, 20Mb+ + * Linux&Windows: 300-400 ms, + * Windows uncached run: 1.6 seconds + */ } public void closeQuietly(Closeable stream) { @@ -94,12 +98,4 @@ } } } - - public static void main(String[] args) throws Exception { - final long start = System.nanoTime(); - final File src = new File(".../hg/cpython/.hg/store/00changelog.d"); - copyFile(src, new File("/tmp/zxczxczxc234")); - final long end = System.nanoTime(); - System.out.printf("Copy of %,d bytes took %d ms", src.length(), (end-start)/1000000); - } }
--- a/src/org/tmatesoft/hg/internal/LineReader.java Mon May 20 18:35:13 2013 +0200 +++ b/src/org/tmatesoft/hg/internal/LineReader.java Mon May 20 20:34:33 2013 +0200 @@ -16,8 +16,6 @@ */ package org.tmatesoft.hg.internal; -import static org.tmatesoft.hg.util.LogFacility.Severity.Warn; - import java.io.BufferedReader; import java.io.File; import java.io.FileInputStream; @@ -29,7 +27,6 @@ import java.util.Collection; import org.tmatesoft.hg.repo.HgInvalidFileException; -import org.tmatesoft.hg.repo.ext.MqManager; import org.tmatesoft.hg.util.LogFacility; /** @@ -125,17 +122,11 @@ } catch (IOException ex) { throw new HgInvalidFileException(ex.getMessage(), ex, file); } finally { - if (statusFileReader != null) { - try { - statusFileReader.close(); - } catch (IOException ex) { - log.dump(MqManager.class, Warn, ex, null); - } - } + new FileUtils(log).closeQuietly(statusFileReader); // try { // consumer.end(file, paramObj); // } catch (IOException ex) { -// log.warn(MqManager.class, ex, null); +// log.warn(getClass(), ex, null); // } } }
--- a/src/org/tmatesoft/hg/repo/HgBookmarks.java Mon May 20 18:35:13 2013 +0200 +++ b/src/org/tmatesoft/hg/repo/HgBookmarks.java Mon May 20 20:34:33 2013 +0200 @@ -167,10 +167,7 @@ } Nodeid activeRev = getRevision(activeBookmark); if (!activeRev.equals(p1) && !activeRev.equals(p2)) { - // from the wiki: - // "active bookmarks are automatically updated when committing to the changeset they are pointing to" - // FIXME: test ci 1, hg bookmark active, ci 2, hg bookmark -f -r 0 active, ci 3, check active still points to r0 - return; + return; // TestCommit#testNoBookmarkUpdate } if (child.equals(activeRev)) { return;
--- a/src/org/tmatesoft/hg/repo/ext/MqManager.java Mon May 20 18:35:13 2013 +0200 +++ b/src/org/tmatesoft/hg/repo/ext/MqManager.java Mon May 20 20:34:33 2013 +0200 @@ -39,9 +39,6 @@ * Mercurial Queues Support. * Access to MqExtension functionality. * - * FIXME check we don't hold any mq files for too long, close them, use - * the same lock mechanism as mq does (if any). Check if MQ uses Mercurial's store lock - * * @since 1.1 * @author Artem Tikhomirov * @author TMate Software Ltd. @@ -65,6 +62,8 @@ * @return <code>this</code> for convenience */ public MqManager refresh() throws HgInvalidControlFileException { + // MQ doesn't seem to use any custom lock mechanism. + // MQ uses Mercurial's wc/store lock when updating repository (strip/new queue) applied = allKnown = Collections.emptyList(); queueNames = Collections.emptyList(); final LogFacility log = repo.getSessionContext().getLog();
--- a/test/org/tmatesoft/hg/test/TestBlame.java Mon May 20 18:35:13 2013 +0200 +++ b/test/org/tmatesoft/hg/test/TestBlame.java Mon May 20 20:34:33 2013 +0200 @@ -255,7 +255,7 @@ } } - // FIXME HgWorkingCopyStatusCollector (and HgStatusCollector), with their ancestors (rev 59/69) have examples + // 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 // while original diff lib does. Would be nice to behave as close to original, as possible.
--- a/test/org/tmatesoft/hg/test/TestCommit.java Mon May 20 18:35:13 2013 +0200 +++ b/test/org/tmatesoft/hg/test/TestCommit.java Mon May 20 20:34:33 2013 +0200 @@ -75,8 +75,6 @@ // HgRepository hgRepo = new HgLookup().detect(repoLoc); CommitFacility cf = new CommitFacility(Internals.getInstance(hgRepo), 0); - // FIXME test diff for processing changed newlines (ie \r\n -> \n or vice verse) - if a whole line or - // just changed endings are in the patch! HgDataFile df = hgRepo.getFileNode("file1"); cf.add(df, new ByteArrayDataSource("hello\nworld".getBytes())); Transaction tr = newTransaction(hgRepo); @@ -319,6 +317,42 @@ errorCollector.assertEquals(c, hgRepo.getBookmarks().getRevision(activeBookmark)); } + /** + * from the wiki: + * "active bookmarks are automatically updated when committing to the changeset they are pointing to" + * Synopsis: commit 1 (c1), hg bookmark active (points to commit1), make commit 2, hg bookmark -f -r c1 active, commit 3, check active still points to c1 + */ + @Test + public void testNoBookmarkUpdate() throws Exception { + File repoLoc = RepoUtils.cloneRepoToTempLocation("log-1", "test-no-bookmark-upd", false); + HgRepository hgRepo = new HgLookup().detect(repoLoc); + assertNull("[sanity]", hgRepo.getBookmarks().getActiveBookmarkName()); + ExecHelper eh = new ExecHelper(new OutputParser.Stub(), repoLoc); + String activeBookmark = "bm1"; + eh.run("hg", "bookmarks", activeBookmark); + assertEquals("Bookmarks has to reload", activeBookmark, hgRepo.getBookmarks().getActiveBookmarkName()); + Nodeid initialBookmarkRevision = hgRepo.getBookmarks().getRevision(activeBookmark); // c1 + assertEquals("[sanity]", initialBookmarkRevision, hgRepo.getWorkingCopyParents().first()); + + File fileD = new File(repoLoc, "d"); + assertTrue("[sanity]", fileD.canRead()); + RepoUtils.modifyFileAppend(fileD, " 1 \n"); + HgCommitCommand cmd = new HgCommitCommand(hgRepo).message("FIRST"); + Outcome r = cmd.execute(); + errorCollector.assertTrue(r.isOk()); + Nodeid c2 = cmd.getCommittedRevision(); + errorCollector.assertEquals(c2, hgRepo.getBookmarks().getRevision(activeBookmark)); + // + eh.run("hg", "bookmark", activeBookmark, "--force", "--rev", initialBookmarkRevision.toString()); + // + RepoUtils.modifyFileAppend(fileD, " 2 \n"); + cmd = new HgCommitCommand(hgRepo).message("SECOND"); + r = cmd.execute(); + errorCollector.assertTrue(r.isOk()); + //Nodeid c3 = cmd.getCommittedRevision(); + errorCollector.assertEquals(initialBookmarkRevision, hgRepo.getBookmarks().getRevision(activeBookmark)); + } + @Test public void testRefreshTagsAndBranches() throws Exception { File repoLoc = RepoUtils.cloneRepoToTempLocation("log-branches", "test-refresh-after-commit", false);
--- a/test/org/tmatesoft/hg/test/TestDiffHelper.java Mon May 20 18:35:13 2013 +0200 +++ b/test/org/tmatesoft/hg/test/TestDiffHelper.java Mon May 20 20:34:33 2013 +0200 @@ -118,6 +118,42 @@ diff.findMatchingBlocks(mc = new MatchCollector<CharSequence>()); assertEquals(3, mc.matchCount()); // bc, e, g } + + @Test + public void testChangedEOL() { + DiffHelper<LineSequence> diffHelper = new DiffHelper<LineSequence>(); + MatchCollector<LineSequence> mc; DeltaCollector dc; + // all lines changed + diffHelper.init(newlines("one\ntwo\nthree\n".getBytes()), newlines("one\r\ntwo\r\nthree\r\n".getBytes())); + diffHelper.findMatchingBlocks(mc = new MatchCollector<LineSequence>()); + assertEquals(0, mc.matchCount()); + diffHelper.findMatchingBlocks(dc = new DeltaCollector()); + assertEquals(0, dc.unchangedCount()); + assertEquals(1, dc.deletedCount()); + assertTrue(dc.deletedLine(0)); + assertTrue(dc.deletedLine(1)); + assertTrue(dc.deletedLine(2)); + assertEquals(1, dc.addedCount()); + assertTrue(dc.addedLine(0)); + assertTrue(dc.addedLine(1)); + assertTrue(dc.addedLine(2)); + // one line changed + diffHelper.init(newlines("one\ntwo\nthree\n".getBytes()), newlines("one\ntwo\r\nthree\n".getBytes())); + diffHelper.findMatchingBlocks(mc = new MatchCollector<LineSequence>()); + assertEquals(2, mc.matchCount()); + assertTrue(mc.originLineMatched(0)); + assertTrue(mc.targetLineMatched(0)); + assertFalse(mc.originLineMatched(1)); + assertFalse(mc.targetLineMatched(1)); + assertTrue(mc.originLineMatched(2)); + assertTrue(mc.targetLineMatched(2)); + diffHelper.findMatchingBlocks(dc = new DeltaCollector()); + assertEquals(2, dc.unchangedCount()); + assertEquals(1, dc.deletedCount()); + assertTrue(dc.deletedLine(1)); + assertEquals(1, dc.addedCount()); + assertTrue(dc.addedLine(1)); + } // range is comprised of 3 values, range length always last, range start comes at index o (either 0 or 1) static boolean includes(IntVector ranges, int o, int ln) { @@ -188,20 +224,24 @@ same.add(s1From, s2From, length); } + // return number of regions that didn't change int unchangedCount() { return same.size() / 3; } + // return number of added regions int addedCount() { return added.size() / 3; } - + // return number of deleted regions int deletedCount() { return deleted.size() / 3; } + // answer if 0-based line is marked as added boolean addedLine(int ln) { return includes(added, 1, ln); } + // answer if 0-based line is marked as deleted boolean deletedLine(int ln) { return includes(deleted, 1, ln); }