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

CHANGELOG

@@ -30,6 +30,7 @@ v 7.9.0 (unreleased)
   - Support referencing issues to a project whose name starts with a digit
   - Condense commits already in target branch when updating merge request source branch.
   - Send notifications and leave system comments when bulk updating issues.
+  - Automatically link commit ranges to compare page: sha1...sha4 or sha1..sha4 (includes sha1 in comparison)
 
 v 7.8.2
   - Fix service migration issue when upgrading from versions prior to 7.3

lib/gitlab/markdown.rb

@@ -14,6 +14,7 @@ module Gitlab
   #   * !123 for merge requests
   #   * $123 for snippets
   #   * 123456 for commits
+  #   * 123456...7890123 for commit ranges (comparisons)
   #
   # It also parses Emoji codes to insert images. See
   # http://www.emoji-cheat-sheet.com/ for a list of the supported icons.
@@ -133,13 +134,14 @@ module Gitlab
         |#{PROJ_STR}?\#(?<issue>([a-zA-Z\-]+-)?\d+) # Issue ID
         |#{PROJ_STR}?!(?<merge_request>\d+)  # MR ID
         |\$(?<snippet>\d+)                   # Snippet ID
+        |(#{PROJ_STR}@)?(?<commit_range>[\h]{6,40}\.{2,3}[\h]{6,40}) # Commit range
         |(#{PROJ_STR}@)?(?<commit>[\h]{6,40}) # Commit ID
         |(?<skip>gfm-extraction-[\h]{6,40})  # Skip gfm extractions. Otherwise will be parsed as commit
       )
       (?<suffix>\W)?                         # Suffix
     }x.freeze
 
-    TYPES = [:user, :issue, :label, :merge_request, :snippet, :commit].freeze
+    TYPES = [:user, :issue, :label, :merge_request, :snippet, :commit, :commit_range].freeze
 
     def parse_references(text, project = @project)
       # parse reference links
@@ -290,6 +292,30 @@ module Gitlab
       end
     end
 
+    def reference_commit_range(identifier, project = @project, prefix_text = nil)
+      from_id, to_id = identifier.split(/\.{2,3}/, 2)
+
+      inclusive = identifier !~ /\.{3}/
+      from_id << "^" if inclusive
+
+      if project.valid_repo? && 
+          from = project.repository.commit(from_id) && 
+          to = project.repository.commit(to_id)
+
+        options = html_options.merge(
+          title: "Commits #{from_id} through #{to_id}",
+          class: "gfm gfm-commit_range #{html_options[:class]}"
+        )
+        prefix_text = "#{prefix_text}@" if prefix_text
+
+        link_to(
+          "#{prefix_text}#{identifier}",
+          namespace_project_compare_url(project.namespace, project, from: from_id, to: to_id),
+          options
+        )
+      end
+    end
+
     def reference_external_issue(identifier, project = @project,
                                  prefix_text = nil)
       url = url_for_issue(identifier, project)

lib/gitlab/reference_extractor.rb

 module Gitlab
   # Extract possible GFM references from an arbitrary String for further processing.
   class ReferenceExtractor
-    attr_accessor :users, :labels, :issues, :merge_requests, :snippets, :commits
+    attr_accessor :users, :labels, :issues, :merge_requests, :snippets, :commits, :commit_ranges
 
     include Markdown
 
     def initialize
-      @users, @labels, @issues, @merge_requests, @snippets, @commits =
-        [], [], [], [], [], []
+      @users, @labels, @issues, @merge_requests, @snippets, @commits, @commit_ranges =
+        [], [], [], [], [], [], []
     end
 
     def analyze(string, project)
@@ -60,6 +60,16 @@ module Gitlab
       end.reject(&:nil?)
     end
 
+    def commit_ranges_for(project = nil)
+      commit_ranges.map do |entry|
+        repo = entry[:project].repository if entry[:project]
+        if repo && should_lookup?(project, entry[:project])
+          from_id, to_id = entry[:id].split(/\.{2,3}/, 2)
+          [repo.commit(from_id), repo.commit(to_id)]
+        end
+      end.reject(&:nil?)
+    end
+
     private
 
     def reference_link(type, identifier, project, _)

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) }
 

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