From 6e5d6a64e99d6e580471199279e5dfd55e791f10 Mon Sep 17 00:00:00 2001 From: David Nelson Date: Thu, 18 Apr 2019 17:55:02 -0500 Subject: [PATCH 1/3] Add tests for currently supported log arguments --- __tests__/base.js | 26 +++++++++++++++++++++++++- __tests__/git.spec.js | 40 ++++++++++++++++++++++++++++++++++++++++ src/js/git/headless.js | 5 ++++- 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/__tests__/base.js b/__tests__/base.js index 76a6ec7f..ba3cf434 100644 --- a/__tests__/base.js +++ b/__tests__/base.js @@ -1,3 +1,5 @@ +var Q = require('q'); + var HeadlessGit = require('../src/js/git/headless').HeadlessGit; var TreeCompare = require('../src/js/graph/treeCompare.js'); @@ -95,6 +97,27 @@ var expectLevelSolved = function(levelBlob) { expectLevelAsync(headless, levelBlob); }; +var runCommand = function(command, resultHandler) { + var headless = new HeadlessGit(); + var deferred = Q.defer(); + var msg = null; + + deferred.promise.then(function(commands) { + msg = commands[commands.length - 1].get('error').get('msg'); + }); + + runs(function() { + headless.sendCommand(command, deferred); + }); + waitsFor(function() { + if(null == msg) { + return false; + } + resultHandler(msg); + return true; + }, 'commands should be finished', 500); +}; + var TIME = 150; // useful for throwing garbage and then expecting one commit var ONE_COMMIT_TREE = '{"branches":{"master":{"target":"C2","id":"master"}},"commits":{"C0":{"parents":[],"id":"C0","rootCommit":true},"C1":{"parents":["C0"],"id":"C1"},"C2":{"parents":["C1"],"id":"C2"}},"HEAD":{"target":"master","id":"HEAD"}}'; @@ -105,6 +128,7 @@ module.exports = { TIME: TIME, expectTreeAsync: expectTreeAsync, expectLevelSolved: expectLevelSolved, - ONE_COMMIT_TREE: ONE_COMMIT_TREE + ONE_COMMIT_TREE: ONE_COMMIT_TREE, + runCommand: runCommand }; diff --git a/__tests__/git.spec.js b/__tests__/git.spec.js index 656d197e..810f4e9f 100644 --- a/__tests__/git.spec.js +++ b/__tests__/git.spec.js @@ -1,5 +1,6 @@ var base = require('./base'); var expectTreeAsync = base.expectTreeAsync; +var runCommand = base.runCommand; describe('Git', function() { it('Commits', function() { @@ -275,4 +276,43 @@ describe('Git', function() { ); }); + describe('Log supports', function() { + var SETUP = 'git co -b left C0; gc; git merge master; git co -b right C0; gc; git merge master; git co -b all left; git merge right; '; + + it('implied HEAD', function() { + runCommand(SETUP + '; git co right; git log', function(commandMsg) { + expect(commandMsg).toContain('Commit: C0\n'); + expect(commandMsg).toContain('Commit: C1\n'); + expect(commandMsg).not.toContain('Commit: C2\n'); + expect(commandMsg).not.toContain('Commit: C3\n'); + expect(commandMsg).toContain('Commit: C4\n'); + expect(commandMsg).toContain('Commit: C5\n'); + expect(commandMsg).not.toContain('Commit: C6\n'); + }); + }); + + it('single included revision', function() { + runCommand(SETUP + 'git log right', function(commandMsg) { + expect(commandMsg).toContain('Commit: C0\n'); + expect(commandMsg).toContain('Commit: C1\n'); + expect(commandMsg).not.toContain('Commit: C2\n'); + expect(commandMsg).not.toContain('Commit: C3\n'); + expect(commandMsg).toContain('Commit: C4\n'); + expect(commandMsg).toContain('Commit: C5\n'); + expect(commandMsg).not.toContain('Commit: C6\n'); + }); + }); + + it('single excluded revision', function() { + runCommand(SETUP + 'git log all ^right', function(commandMsg) { + expect(commandMsg).not.toContain('Commit: C0\n'); + expect(commandMsg).not.toContain('Commit: C1\n'); + expect(commandMsg).toContain('Commit: C2\n'); + expect(commandMsg).toContain('Commit: C3\n'); + expect(commandMsg).not.toContain('Commit: C4\n'); + expect(commandMsg).not.toContain('Commit: C5\n'); + expect(commandMsg).toContain('Commit: C6\n'); + }); + }); + }); }); diff --git a/src/js/git/headless.js b/src/js/git/headless.js index a82081b9..f2b2c0bc 100644 --- a/src/js/git/headless.js +++ b/src/js/git/headless.js @@ -109,6 +109,8 @@ HeadlessGit.prototype.sendCommand = function(value, entireCommandPromise) { var chain = deferred.promise; var startTime = new Date().getTime(); + var commands = []; + util.splitTextCommand(value, function(commandStr) { chain = chain.then(function() { var commandObj = new Command({ @@ -117,6 +119,7 @@ HeadlessGit.prototype.sendCommand = function(value, entireCommandPromise) { var thisDeferred = Q.defer(); this.gitEngine.dispatch(commandObj, thisDeferred); + commands.push(commandObj); return thisDeferred.promise; }.bind(this)); }, this); @@ -124,7 +127,7 @@ HeadlessGit.prototype.sendCommand = function(value, entireCommandPromise) { chain.then(function() { var nowTime = new Date().getTime(); if (entireCommandPromise) { - entireCommandPromise.resolve(); + entireCommandPromise.resolve(commands); } }); From 8529a3aac76eece97f7dc0ebbf86152ae26ccfbb Mon Sep 17 00:00:00 2001 From: David Nelson Date: Thu, 18 Apr 2019 20:57:06 -0500 Subject: [PATCH 2/3] Implement rev-list This commit introduces the RevisionRange, which more closely follows real git in terms of identifying a range of revisions than the current implementation of log. RevisionRange is being used in the new rev-list command first because it is easy to test for ordering. A future commit will replace the existing implementation of log with RevisionRange. --- __tests__/git.spec.js | 37 ++++++++++++++++++++ src/js/git/commands.js | 12 +++++++ src/js/git/index.js | 79 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+) diff --git a/__tests__/git.spec.js b/__tests__/git.spec.js index 810f4e9f..73c46a82 100644 --- a/__tests__/git.spec.js +++ b/__tests__/git.spec.js @@ -276,6 +276,43 @@ describe('Git', function() { ); }); + describe('RevList', function() { + + it('requires at least 1 argument', function() { + runCommand('git rev-list', function(commandMsg) { + expect(commandMsg).toContain('at least 1'); + }); + }); + + describe('supports', function() { + var SETUP = 'git co -b left C0; gc; git merge master; git co -b right C0; gc; git merge master; git co -b all left; git merge right; '; + + it('single included revision', function() { + runCommand(SETUP + 'git rev-list all', function(commandMsg) { + expect(commandMsg).toBe('C6\nC5\nC4\nC3\nC2\nC1\nC0\n'); + }); + }); + + it('single excluded revision', function() { + runCommand(SETUP + 'git rev-list all ^right', function(commandMsg) { + expect(commandMsg).toBe('C6\nC3\nC2\n'); + }); + }); + + it('multiple included revisions', function() { + runCommand(SETUP + 'git rev-list right left', function(commandMsg) { + expect(commandMsg).toBe('C5\nC4\nC3\nC2\nC1\nC0\n'); + }); + }); + + it('multiple excluded revisions', function() { + runCommand(SETUP + 'git rev-list all ^right ^left', function(commandMsg) { + expect(commandMsg).toBe('C6\n'); + }); + }); + }); + }); + describe('Log supports', function() { var SETUP = 'git co -b left C0; gc; git merge master; git co -b right C0; gc; git merge master; git co -b all left; git merge right; '; diff --git a/src/js/git/commands.js b/src/js/git/commands.js index 4c083d5e..83e21e80 100644 --- a/src/js/git/commands.js +++ b/src/js/git/commands.js @@ -574,6 +574,18 @@ var commandConfig = { } }, + revlist: { + dontCountForGolf: true, + displayName: 'rev-list', + regex: /^git +rev-list($|\s)/, + execute: function(engine, command) { + var generalArgs = command.getGeneralArgs(); + command.validateArgBounds(generalArgs, 1); + + engine.revlist(generalArgs); + } + }, + log: { dontCountForGolf: true, regex: /^git +log($|\s)/, diff --git a/src/js/git/index.js b/src/js/git/index.js index d03a2075..ff811f8e 100644 --- a/src/js/git/index.js +++ b/src/js/git/index.js @@ -2740,6 +2740,20 @@ GitEngine.prototype.logWithout = function(ref, omitBranch) { this.log(ref, Graph.getUpstreamSet(this, omitBranch)); }; +GitEngine.prototype.revlist = function(refs) { + var range = new RevisionRange(this, refs); + + // now go through and collect ids + var bigLogStr = ''; + range.revisions.forEach(function(c) { + bigLogStr += c.id + '\n'; + }); + + throw new CommandResult({ + msg: bigLogStr + }); +}; + GitEngine.prototype.log = function(ref, omitSet) { // omit set is for doing stuff like git log branchA ^branchB omitSet = omitSet || {}; @@ -3079,6 +3093,71 @@ var Tag = Ref.extend({ } }); +function RevisionRange(engine, specifiers) { + this.engine = engine; + this.included = {}; + this.excluded = {}; + this.revisions = []; + + this.processSpecifiers(specifiers); +} + +RevisionRange.prototype.isExclusion = function(specifier) { + return specifier.startsWith('^'); +}; + +RevisionRange.prototype.processSpecifiers = function(specifiers) { + var self = this; + var inclusions = []; + var exclusions = []; + + specifiers.forEach(function(specifier) { + if(self.isExclusion(specifier)) { + exclusions.push(specifier.slice(1)); + } else { + inclusions.push(specifier); + } + }); + + exclusions.forEach(function(exclusion) { + self.addExcluded(Graph.getUpstreamSet(self.engine, exclusion)); + }); + + inclusions.forEach(function(inclusion) { + self.addIncluded(Graph.getUpstreamSet(self.engine, inclusion)); + }); + + var includedKeys = Array.from(Object.keys(self.included)); + + self.revisions = includedKeys.map(function(revision) { + return self.engine.resolveStringRef(revision); + }); + self.revisions.sort(self.engine.dateSortFunc); + self.revisions.reverse(); +}; + +RevisionRange.prototype.isExcluded = function(revision) { + return this.excluded.hasOwnProperty(revision); +}; + +RevisionRange.prototype.addExcluded = function(setToExclude) { + var self = this; + Object.keys(setToExclude).forEach(function(toExclude) { + if(!self.isExcluded(toExclude)) { + self.excluded[toExclude] = true; + } + }); +}; + +RevisionRange.prototype.addIncluded = function(setToInclude) { + var self = this; + Object.keys(setToInclude).forEach(function(toInclude) { + if(!self.isExcluded(toInclude)) { + self.included[toInclude] = true; + } + }); +}; + exports.GitEngine = GitEngine; exports.Commit = Commit; exports.Branch = Branch; From b9f8e191628693b94c0fa4ee96b32442ea6c6878 Mon Sep 17 00:00:00 2001 From: David Nelson Date: Thu, 18 Apr 2019 20:57:29 -0500 Subject: [PATCH 3/3] Implement log using RevisionRange This changes the implementation of the "log" command to use the RevisionRange functionality. RevisionRange sorts the results in order of reverse create time, to match real git. This is a change from the previous implementation of log, which essentially produced a breadth-first ordering. --- __tests__/git.spec.js | 24 ++++++++++++++++++ src/js/git/commands.js | 15 ++--------- src/js/git/index.js | 47 +++++++++++------------------------ src/js/models/commandModel.js | 14 ++++++----- 4 files changed, 49 insertions(+), 51 deletions(-) diff --git a/__tests__/git.spec.js b/__tests__/git.spec.js index 73c46a82..fd10ad41 100644 --- a/__tests__/git.spec.js +++ b/__tests__/git.spec.js @@ -351,5 +351,29 @@ describe('Git', function() { expect(commandMsg).toContain('Commit: C6\n'); }); }); + + it('multiple included revisions', function() { + runCommand(SETUP + 'git log right left', function(commandMsg) { + expect(commandMsg).toContain('Commit: C0\n'); + expect(commandMsg).toContain('Commit: C1\n'); + expect(commandMsg).toContain('Commit: C2\n'); + expect(commandMsg).toContain('Commit: C3\n'); + expect(commandMsg).toContain('Commit: C4\n'); + expect(commandMsg).toContain('Commit: C5\n'); + expect(commandMsg).not.toContain('Commit: C6\n'); + }); + }); + + it('multiple excluded revisions', function() { + runCommand(SETUP + 'git log all ^right ^left', function(commandMsg) { + expect(commandMsg).not.toContain('Commit: C0\n'); + expect(commandMsg).not.toContain('Commit: C1\n'); + expect(commandMsg).not.toContain('Commit: C2\n'); + expect(commandMsg).not.toContain('Commit: C3\n'); + expect(commandMsg).not.toContain('Commit: C4\n'); + expect(commandMsg).not.toContain('Commit: C5\n'); + expect(commandMsg).toContain('Commit: C6\n'); + }); + }); }); }); diff --git a/src/js/git/commands.js b/src/js/git/commands.js index 83e21e80..50e93d5a 100644 --- a/src/js/git/commands.js +++ b/src/js/git/commands.js @@ -592,19 +592,8 @@ var commandConfig = { execute: function(engine, command) { var generalArgs = command.getGeneralArgs(); - if (generalArgs.length == 2) { - // do fancy git log branchA ^branchB - if (generalArgs[1][0] == '^') { - engine.logWithout(generalArgs[0], generalArgs[1]); - } else { - throw new GitError({ - msg: intl.str('git-error-options') - }); - } - } - - command.oneArgImpliedHead(generalArgs); - engine.log(generalArgs[0]); + command.impliedHead(generalArgs, 0); + engine.log(generalArgs); } }, diff --git a/src/js/git/index.js b/src/js/git/index.js index ff811f8e..c0c64024 100644 --- a/src/js/git/index.js +++ b/src/js/git/index.js @@ -2744,9 +2744,8 @@ GitEngine.prototype.revlist = function(refs) { var range = new RevisionRange(this, refs); // now go through and collect ids - var bigLogStr = ''; - range.revisions.forEach(function(c) { - bigLogStr += c.id + '\n'; + var bigLogStr = range.formatRevisions(function(c) { + return c.id + '\n'; }); throw new CommandResult({ @@ -2754,37 +2753,13 @@ GitEngine.prototype.revlist = function(refs) { }); }; -GitEngine.prototype.log = function(ref, omitSet) { - // omit set is for doing stuff like git log branchA ^branchB - omitSet = omitSet || {}; - // first get the commit we referenced - var commit = this.getCommitFromRef(ref); - - // then get as many far back as we can from here, order by commit date - var toDump = []; - var pQueue = [commit]; - - var seen = {}; - - while (pQueue.length) { - var popped = pQueue.shift(0); - if (seen[popped.get('id')] || omitSet[popped.get('id')]) { - continue; - } - seen[popped.get('id')] = true; - - toDump.push(popped); - - if (popped.get('parents') && popped.get('parents').length) { - pQueue = pQueue.concat(popped.get('parents')); - } - } +GitEngine.prototype.log = function(refs) { + var range = new RevisionRange(this, refs); // now go through and collect logs - var bigLogStr = ''; - toDump.forEach(function (c) { - bigLogStr += c.getLogEntry(); - }, this); + var bigLogStr = range.formatRevisions(function(c) { + return c.getLogEntry(); + }); throw new CommandResult({ msg: bigLogStr @@ -3158,6 +3133,14 @@ RevisionRange.prototype.addIncluded = function(setToInclude) { }); }; +RevisionRange.prototype.formatRevisions = function(revisionFormatter) { + var output = ""; + this.revisions.forEach(function(c) { + output += revisionFormatter(c); + }); + return output; +}; + exports.GitEngine = GitEngine; exports.Commit = Commit; exports.Branch = Branch; diff --git a/src/js/models/commandModel.js b/src/js/models/commandModel.js index 76e94d61..138f9024 100644 --- a/src/js/models/commandModel.js +++ b/src/js/models/commandModel.js @@ -126,18 +126,14 @@ var Command = Backbone.Model.extend({ oneArgImpliedHead: function(args, option) { this.validateArgBounds(args, 0, 1, option); // and if it's one, add a HEAD to the back - if (args.length === 0) { - args.push('HEAD'); - } + this.impliedHead(args, 0); }, twoArgsImpliedHead: function(args, option) { // our args we expect to be between 1 and 2 this.validateArgBounds(args, 1, 2, option); // and if it's one, add a HEAD to the back - if (args.length == 1) { - args.push('HEAD'); - } + this.impliedHead(args, 1); }, oneArgImpliedOrigin: function(args) { @@ -151,6 +147,12 @@ var Command = Backbone.Model.extend({ this.validateArgBounds(args, 0, 2); }, + impliedHead: function(args, min) { + if(args.length == min) { + args.push('HEAD'); + } + }, + // this is a little utility class to help arg validation that happens over and over again validateArgBounds: function(args, lower, upper, option) { var what = (option === undefined) ?