Merge branch 'commit-range-reference' into 'master'

Automatically link commit ranges to compare page.

Closes #2103

Implemented as proposed in the last paragraph of the issue:

> We could do something similar to Ruby, where `1..5` means `1,2,3,4,5` and `1...5` means `1,2,3,4`: `..` means inclusive, `...` means exclusive. In our case, `sha1...sha4` would mean `sha2,sha3,sha4` (exclusive with regards to `sha1`) and `sha1..sha4` would mean `sha1^...sha4`, i.e. `sha1,sha2,sha3,sha4` (inclusive to `sha1`).

- `sha1...sha4` now links to `compare/sha1...sha4`
- `sha1..sha4` now links to `compare/sha1^...sha4`.

See merge request !1649

2 files changed, 67 insertions, 0 deletions

diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb
index 76fcf888a6a..74a42932fe8 100644
--- a/spec/helpers/gitlab_markdown_helper_spec.rb
+++ b/spec/helpers/gitlab_markdown_helper_spec.rb
@@ -9,6 +9,7 @@ describe GitlabMarkdownHelper do
 
   let(:user)          { create(:user, username: 'gfm') }
   let(:commit)        { project.repository.commit }
+  let(:earlier_commit){ project.repository.commit("HEAD~2") }
   let(:issue)         { create(:issue, project: project) }
   let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
   let(:snippet)       { create(:project_snippet, project: project) }
@@ -53,6 +54,53 @@ describe GitlabMarkdownHelper do
           to have_selector('a.gfm.foo')
     end
 
+    describe "referencing a commit range" do
+      let(:expected) { namespace_project_compare_path(project.namespace, project, from: earlier_commit.id, to: commit.id) }
+
+      it "should link using a full id" do
+        actual = "What happened in #{earlier_commit.id}...#{commit.id}"
+        expect(gfm(actual)).to match(expected)
+      end
+
+      it "should link using a short id" do
+        actual = "What happened in #{earlier_commit.short_id}...#{commit.short_id}"
+        expected = namespace_project_compare_path(project.namespace, project, from: earlier_commit.short_id, to: commit.short_id)
+        expect(gfm(actual)).to match(expected)
+      end
+
+      it "should link inclusively" do
+        actual = "What happened in #{earlier_commit.id}..#{commit.id}"
+        expected = namespace_project_compare_path(project.namespace, project, from: "#{earlier_commit.id}^", to: commit.id)
+        expect(gfm(actual)).to match(expected)
+      end
+
+      it "should link with adjacent text" do
+        actual = "(see #{earlier_commit.id}...#{commit.id})"
+        expect(gfm(actual)).to match(expected)
+      end
+
+      it "should keep whitespace intact" do
+        actual   = "Changes #{earlier_commit.id}...#{commit.id} dramatically"
+        expected = /Changes <a.+>#{earlier_commit.id}...#{commit.id}<\/a> dramatically/
+        expect(gfm(actual)).to match(expected)
+      end
+
+      it "should not link with an invalid id" do
+        actual = expected = "What happened in #{earlier_commit.id.reverse}...#{commit.id.reverse}"
+        expect(gfm(actual)).to eq(expected)
+      end
+
+      it "should include a title attribute" do
+        actual = "What happened in #{earlier_commit.id}...#{commit.id}"
+        expect(gfm(actual)).to match(/title="Commits #{earlier_commit.id} through #{commit.id}"/)
+      end
+
+      it "should include standard gfm classes" do
+        actual = "What happened in #{earlier_commit.id}...#{commit.id}"
+        expect(gfm(actual)).to match(/class="\s?gfm gfm-commit_range\s?"/)
+      end
+    end
+
     describe "referencing a commit" do
       let(:expected) { namespace_project_commit_path(project.namespace, project, commit) }
 
diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb
index 0847c31258c..034f8ee7c45 100644
--- a/spec/lib/gitlab/reference_extractor_spec.rb
+++ b/spec/lib/gitlab/reference_extractor_spec.rb
@@ -31,6 +31,11 @@ describe Gitlab::ReferenceExtractor do
     expect(subject.commits).to eq([{ project: nil, id: '98cf0ae3' }])
   end
 
+  it 'extracts commit ranges' do
+    subject.analyze('here you go, a commit range: 98cf0ae3...98cf0ae4', nil)
+    expect(subject.commit_ranges).to eq([{ project: nil, id: '98cf0ae3...98cf0ae4' }])
+  end
+
   it 'extracts multiple references and preserves their order' do
     subject.analyze('@me and @you both care about this', nil)
     expect(subject.users).to eq([
@@ -100,5 +105,19 @@ describe Gitlab::ReferenceExtractor do
       expect(extracted[0].sha).to eq(commit.sha)
       expect(extracted[0].message).to eq(commit.message)
     end
+
+    it 'accesses valid commit ranges' do
+      commit = project.repository.commit('master')
+      earlier_commit = project.repository.commit('master~2')
+
+      subject.analyze("this references commits #{earlier_commit.sha[0..6]}...#{commit.sha[0..6]}",
+                      project)
+      extracted = subject.commit_ranges_for(project)
+      expect(extracted.size).to eq(1)
+      expect(extracted[0][0].sha).to eq(earlier_commit.sha)
+      expect(extracted[0][0].message).to eq(earlier_commit.message)
+      expect(extracted[0][1].sha).to eq(commit.sha)
+      expect(extracted[0][1].message).to eq(commit.message)
+    end
   end
 end