From 0189be0831350a5d473884a5b454a10509ff58ce Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 10 Oct 2014 15:39:48 +0300 Subject: [PATCH 1/8] Use short_id instead of [0..N] for short version of commit sha Signed-off-by: Dmitriy Zaporozhets --- app/helpers/commits_helper.rb | 4 ++++ app/models/commit.rb | 6 +++++- app/models/event.rb | 2 +- app/models/merge_request_diff.rb | 2 +- app/views/events/_commit.html.haml | 2 +- app/views/events/_event_push.atom.haml | 2 +- app/views/events/event/_push.html.haml | 2 +- app/views/projects/commit/_commit_box.html.haml | 2 +- app/views/projects/tree/_submodule_item.html.haml | 4 ++-- app/views/projects/wikis/history.html.haml | 2 +- app/views/search/results/_note.html.haml | 2 +- features/steps/project/commits/commits.rb | 2 +- spec/models/commit_spec.rb | 2 +- spec/models/note_spec.rb | 4 ++-- spec/support/mentionable_shared_examples.rb | 11 +++++------ 15 files changed, 28 insertions(+), 21 deletions(-) diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index cab2984a4c4..0e0532b65b2 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -120,4 +120,8 @@ module CommitsHelper class: 'commit-short-id') end end + + def truncate_sha(sha) + Commit.truncate_sha(sha) + end end diff --git a/app/models/commit.rb b/app/models/commit.rb index a1343b65c72..61551df9e27 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -26,6 +26,10 @@ class Commit def diff_line_count(diffs) diffs.reduce(0) { |sum, d| sum + d.diff.lines.count } end + + def truncate_sha(sha) + sha[0..10] + end end attr_accessor :raw @@ -111,7 +115,7 @@ class Commit # Mentionable override. def gfm_reference - "commit #{sha[0..5]}" + "commit #{short_id}" end def method_missing(m, *args, &block) diff --git a/app/models/event.rb b/app/models/event.rb index 9e296c00281..c0b126713a6 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -266,7 +266,7 @@ class Event < ActiveRecord::Base end def note_short_commit_id - note_commit_id[0..8] + Commit.truncate_sha(note_commit_id) end def note_commit? diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 409e82ed1ef..a71122d5e07 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -55,7 +55,7 @@ class MergeRequestDiff < ActiveRecord::Base end def last_commit_short_sha - @last_commit_short_sha ||= last_commit.sha[0..10] + @last_commit_short_sha ||= last_commit.short_id end private diff --git a/app/views/events/_commit.html.haml b/app/views/events/_commit.html.haml index 0e03e116e7d..f0c34def145 100644 --- a/app/views/events/_commit.html.haml +++ b/app/views/events/_commit.html.haml @@ -1,5 +1,5 @@ %li.commit .commit-row-title - = link_to commit[:id][0..8], project_commit_path(project, commit[:id]), class: "commit_short_id", alt: '' + = link_to truncate_sha(commit[:id]), project_commit_path(project, commit[:id]), class: "commit_short_id", alt: ''   = gfm event_commit_title(commit[:message]), project diff --git a/app/views/events/_event_push.atom.haml b/app/views/events/_event_push.atom.haml index 17228c430ca..2b63519edac 100644 --- a/app/views/events/_event_push.atom.haml +++ b/app/views/events/_event_push.atom.haml @@ -2,7 +2,7 @@ - event.commits.first(15).each do |commit| %p %strong= commit[:author][:name] - = link_to "(##{commit[:id][0...8]})", project_commit_path(event.project, id: commit[:id]) + = link_to "(##{truncate_sha(commit[:id])})", project_commit_path(event.project, id: commit[:id]) %i at = commit[:timestamp].to_time.to_s(:short) diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml index 1bca64c7d50..b912b5e092f 100644 --- a/app/views/events/event/_push.html.haml +++ b/app/views/events/event/_push.html.haml @@ -22,4 +22,4 @@ - if event.commits_count > 2 %span ... and #{event.commits_count - 2} more commits. = link_to project_compare_path(event.project, from: event.commit_from, to: event.commit_to) do - %strong Compare → #{event.commit_from[0..7]}...#{event.commit_to[0..7]} + %strong Compare → #{truncate_sha(event.commit_from)}...#{truncate_sha(event.commit_to)} diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 0b6b6af4f90..e149f017f84 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -35,7 +35,7 @@ .commit-info-row %span.cgray= pluralize(@commit.parents.count, "parent") - @commit.parents.each do |parent| - = link_to parent.id[0...10], project_commit_path(@project, parent) + = link_to parent.short_id, project_commit_path(@project, parent) - if @branches.any? .commit-info-row diff --git a/app/views/projects/tree/_submodule_item.html.haml b/app/views/projects/tree/_submodule_item.html.haml index a8ec9df2c8f..46e9be4af83 100644 --- a/app/views/projects/tree/_submodule_item.html.haml +++ b/app/views/projects/tree/_submodule_item.html.haml @@ -7,8 +7,8 @@ @ %span.monospace - if commit.nil? - #{submodule_item.id[0..10]} + #{truncate_sha(submodule_item.id)} - else - = link_to "#{submodule_item.id[0..10]}", commit + = link_to "#{truncate_sha(submodule_item.id)}", commit %td %td.hidden-xs diff --git a/app/views/projects/wikis/history.html.haml b/app/views/projects/wikis/history.html.haml index d3a66c48c9b..ef4b8f74714 100644 --- a/app/views/projects/wikis/history.html.haml +++ b/app/views/projects/wikis/history.html.haml @@ -17,7 +17,7 @@ %tr %td = link_to project_wiki_path(@project, @page, version_id: commit.id) do - = commit.id[0..10] + = truncate_sha(commit.id) %td = commit.author.name %td diff --git a/app/views/search/results/_note.html.haml b/app/views/search/results/_note.html.haml index f2327cd69cc..a44a4542df5 100644 --- a/app/views/search/results/_note.html.haml +++ b/app/views/search/results/_note.html.haml @@ -10,7 +10,7 @@ = project.name_with_namespace · = link_to project_commit_path(project, note.commit_id, anchor: dom_id(note)) do - Commit #{note.commit_id[0..8]} + Commit #{truncate_sha(note.commit_id)} - else = link_to project do = project.name_with_namespace diff --git a/features/steps/project/commits/commits.rb b/features/steps/project/commits/commits.rb index c054e0e8282..935f313e298 100644 --- a/features/steps/project/commits/commits.rb +++ b/features/steps/project/commits/commits.rb @@ -8,7 +8,7 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps commit = @project.repository.commit page.should have_content(@project.name) page.should have_content(commit.message[0..20]) - page.should have_content(commit.id.to_s[0..5]) + page.should have_content(commit.short_id) end step 'I click atom feed link' do diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 6f201adc4e8..24bbf4f57da 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -75,7 +75,7 @@ eos it_behaves_like 'a mentionable' do let(:subject) { commit } let(:mauthor) { create :user, email: commit.author_email } - let(:backref_text) { "commit #{subject.sha[0..5]}" } + let(:backref_text) { "commit #{subject.short_id}" } let(:set_mentionable_text) { ->(txt){ subject.stub(safe_message: txt) } } # Include the subject in the repository stub. diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index eeecd714a28..d8b4a27eb07 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -228,7 +228,7 @@ describe Note do it { should be_valid } its(:noteable) { should == issue } - its(:note) { should == "_mentioned in commit #{commit.sha[0..5]}_" } + its(:note) { should == "_mentioned in commit #{commit.sha[0..10]}_" } end context 'merge request from an issue' do @@ -267,7 +267,7 @@ describe Note do its(:noteable_type) { should == "Commit" } its(:noteable_id) { should be_nil } its(:commit_id) { should == commit.id } - its(:note) { should == "_mentioned in commit #{parent_commit.id[0...6]}_" } + its(:note) { should == "_mentioned in commit #{parent_commit.id[0..10]}_" } end end diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb index 692834c9f29..ebd74206699 100644 --- a/spec/support/mentionable_shared_examples.rb +++ b/spec/support/mentionable_shared_examples.rb @@ -30,15 +30,15 @@ def common_mentionable_setup "!#{mentioned_mr.iid}, " + "#{ext_proj.path_with_namespace}##{ext_issue.iid}, " + "#{ext_proj.path_with_namespace}!#{ext_mr.iid}, " + - "#{ext_proj.path_with_namespace}@#{ext_commit.id[0..5]}, " + - "#{mentioned_commit.sha[0..5]} and itself as #{backref_text}" + "#{ext_proj.path_with_namespace}@#{ext_commit.short_id}, " + + "#{mentioned_commit.sha[0..10]} and itself as #{backref_text}" end before do # Wire the project's repository to return the mentioned commit, and +nil+ for any # unrecognized commits. - commitmap = { '123456' => mentioned_commit } - extra_commits.each { |c| commitmap[c.sha[0..5]] = c } + commitmap = { '1234567890a' => mentioned_commit } + extra_commits.each { |c| commitmap[c.short_id] = c } mproject.repository.stub(:commit) { |sha| commitmap[sha] } set_mentionable_text.call(ref_string) end @@ -54,7 +54,6 @@ shared_examples 'a mentionable' do it "extracts references from its reference property" do # De-duplicate and omit itself refs = subject.references(mproject) - refs.should have(6).items refs.should include(mentioned_issue) refs.should include(mentioned_mr) @@ -90,7 +89,7 @@ shared_examples 'an editable mentionable' do it 'creates new cross-reference notes when the mentionable text is edited' do new_text = "still mentions ##{mentioned_issue.iid}, " + - "#{mentioned_commit.sha[0..5]}, " + + "#{mentioned_commit.sha[0..10]}, " + "#{ext_issue.iid}, " + "new refs: ##{other_issue.iid}, " + "#{ext_proj.path_with_namespace}##{other_ext_issue.iid}" From 200118357de47c6db69a48eac2b488bfb46e9026 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 10 Oct 2014 15:44:27 +0300 Subject: [PATCH 2/8] Use full commit sha width for reference in note body to prevent Ambiguous SHA1 prefix problem Signed-off-by: Dmitriy Zaporozhets --- app/models/commit.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/commit.rb b/app/models/commit.rb index 61551df9e27..c30a630429a 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -115,7 +115,7 @@ class Commit # Mentionable override. def gfm_reference - "commit #{short_id}" + "commit #{id}" end def method_missing(m, *args, &block) From daa55f31d899819069ddbaa9596769a233b5a729 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 10 Oct 2014 15:58:11 +0300 Subject: [PATCH 3/8] Dont raise exception when wrong commit id passed Signed-off-by: Dmitriy Zaporozhets --- app/models/repository.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/repository.rb b/app/models/repository.rb index 339e485e6d2..93994123a90 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -30,6 +30,8 @@ class Repository commit = Gitlab::Git::Commit.find(raw_repository, id) commit = Commit.new(commit) if commit commit + rescue Rugged::OdbError => ex + nil end def commits(ref, path = nil, limit = nil, offset = nil, skip_merges = false) From ea04ed7879ad7177bef7a6dbe3bf90d76ebb8b45 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 10 Oct 2014 16:30:14 +0300 Subject: [PATCH 4/8] Use 8chars short sha for commit in views Signed-off-by: Dmitriy Zaporozhets --- app/models/commit.rb | 8 +++++++- app/views/projects/blame/show.html.haml | 2 +- app/views/projects/commits/_commit.html.haml | 2 +- app/views/projects/commits/_inline_commit.html.haml | 2 +- spec/models/commit_spec.rb | 2 +- 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/app/models/commit.rb b/app/models/commit.rb index c30a630429a..cbe0a39bc70 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -27,8 +27,9 @@ class Commit diffs.reduce(0) { |sum, d| sum + d.diff.lines.count } end + # Truncate sha to 8 characters def truncate_sha(sha) - sha[0..10] + sha[0..7] end end @@ -128,6 +129,11 @@ class Commit super end + # Truncate sha to 8 characters + def short_id + @raw.short_id(7) + end + def parents @parents ||= Commit.decorate(super) end diff --git a/app/views/projects/blame/show.html.haml b/app/views/projects/blame/show.html.haml index e5cde488c3c..bdf02c6285d 100644 --- a/app/views/projects/blame/show.html.haml +++ b/app/views/projects/blame/show.html.haml @@ -15,7 +15,7 @@ %tr %td.blame-commit %span.commit - = link_to commit.short_id(8), project_commit_path(@project, commit), class: "commit_short_id" + = link_to commit.short_id, project_commit_path(@project, commit), class: "commit_short_id"   = commit_author_link(commit, avatar: true, size: 16)   diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 68852ba973f..1eb17f760dc 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -1,6 +1,6 @@ %li.commit.js-toggle-container .commit-row-title - = link_to commit.short_id(8), project_commit_path(project, commit), class: "commit_short_id" + = link_to commit.short_id, project_commit_path(project, commit), class: "commit_short_id"   %span.str-truncated = link_to_gfm commit.title, project_commit_path(project, commit.id), class: "commit-row-message" diff --git a/app/views/projects/commits/_inline_commit.html.haml b/app/views/projects/commits/_inline_commit.html.haml index b36369b4285..574599aa2d2 100644 --- a/app/views/projects/commits/_inline_commit.html.haml +++ b/app/views/projects/commits/_inline_commit.html.haml @@ -1,6 +1,6 @@ %li.commit.inline-commit .commit-row-title - = link_to commit.short_id(8), project_commit_path(project, commit), class: "commit_short_id" + = link_to commit.short_id, project_commit_path(project, commit), class: "commit_short_id"   %span.str-truncated = link_to_gfm commit.title, project_commit_path(project, commit.id), class: "commit-row-message" diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 24bbf4f57da..a6ec44da4be 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -75,7 +75,7 @@ eos it_behaves_like 'a mentionable' do let(:subject) { commit } let(:mauthor) { create :user, email: commit.author_email } - let(:backref_text) { "commit #{subject.short_id}" } + let(:backref_text) { "commit #{subject.id}" } let(:set_mentionable_text) { ->(txt){ subject.stub(safe_message: txt) } } # Include the subject in the repository stub. From 0852d5e480e2789dcc6b5cce08fc0875f97af4bf Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 10 Oct 2014 16:36:06 +0300 Subject: [PATCH 5/8] Fix tests Signed-off-by: Dmitriy Zaporozhets --- spec/models/note_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index d8b4a27eb07..2d839e9611b 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -228,7 +228,7 @@ describe Note do it { should be_valid } its(:noteable) { should == issue } - its(:note) { should == "_mentioned in commit #{commit.sha[0..10]}_" } + its(:note) { should == "_mentioned in commit #{commit.sha}_" } end context 'merge request from an issue' do @@ -267,7 +267,7 @@ describe Note do its(:noteable_type) { should == "Commit" } its(:noteable_id) { should be_nil } its(:commit_id) { should == commit.id } - its(:note) { should == "_mentioned in commit #{parent_commit.id[0..10]}_" } + its(:note) { should == "_mentioned in commit #{parent_commit.id}_" } end end From 8c01448cf9ffb3662ebd22e02077e48ba59c65ca Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 10 Oct 2014 17:39:29 +0300 Subject: [PATCH 6/8] Dontr decoarate already decorated stuff Signed-off-by: Dmitriy Zaporozhets --- app/models/commit.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/models/commit.rb b/app/models/commit.rb index cbe0a39bc70..212229649fc 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -19,7 +19,13 @@ class Commit class << self def decorate(commits) - commits.map { |c| self.new(c) } + commits.map do |commit| + if commit.kind_of?(Commit) + commit + else + self.new(commit) + end + end end # Calculate number of lines to render for diffs From b2d1e97df99dfdda65d5411de76dd34091d6be3e Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 10 Oct 2014 18:42:05 +0300 Subject: [PATCH 7/8] Fix spinach tests Signed-off-by: Dmitriy Zaporozhets --- features/steps/project/merge_requests.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index c009568977d..fae0cec53a6 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -111,7 +111,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps step 'I click on the commit in the merge request' do within '.mr-commits' do - click_link sample_commit.id[0..8] + click_link Commit.truncate_sha(sample_commit.id) end end From b02c21df5cc0dcc795f71f8c11d05d61dc2ad897 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Sun, 12 Oct 2014 21:49:20 +0300 Subject: [PATCH 8/8] Fix tests Signed-off-by: Dmitriy Zaporozhets --- spec/helpers/gitlab_markdown_helper_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index 15033f07432..246bb535fc1 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -60,7 +60,7 @@ describe GitlabMarkdownHelper do end it "should link using a short id" do - actual = "Backported from #{commit.short_id(6)}" + actual = "Backported from #{commit.short_id}" gfm(actual).should match(expected) end