diff --git a/lib/gitlab/markdown/label_reference_filter.rb b/lib/gitlab/markdown/label_reference_filter.rb
index 9753d0ac0f9..3bb84c50b93 100644
--- a/lib/gitlab/markdown/label_reference_filter.rb
+++ b/lib/gitlab/markdown/label_reference_filter.rb
@@ -13,25 +13,34 @@ module Gitlab
class LabelReferenceFilter < HTML::Pipeline::Filter
# Public: Find label references in text
#
- # LabelReferenceFilter.references_in(text) do |match, label|
- # "#{label} "
+ # LabelReferenceFilter.references_in(text) do |match, id, name|
+ # "#{Label.find(id)} "
# end
#
# text - String text to search.
#
- # Yields the String match and the Integer label ID.
+ # Yields the String match, an optional Integer label ID, and an optional
+ # String label name.
#
# Returns a String replaced with the return of the block.
def self.references_in(text)
text.gsub(LABEL_PATTERN) do |match|
- yield match, $~[:label].to_i
+ yield match, $~[:label_id].to_i, $~[:label_name]
end
end
# Pattern used to extract label references from text
#
- # This pattern supports cross-project references.
- LABEL_PATTERN = /~(?\d+)/
+ # TODO (rspeicher): Limit to double quotes (meh) or disallow single quotes in label names (bad).
+ LABEL_PATTERN = %r{
+ ~(
+ (?\d+) | # Integer-based label ID, or
+ (?
+ [^'"&\?,\s]+ | # String-based single-word label title
+ ['"][^&\?,]+['"] # String-based multi-word label surrounded in quotes
+ )
+ )
+ }x
# Don't look for references in text nodes that are children of these
# elements.
@@ -68,8 +77,10 @@ module Gitlab
def label_link_filter(text)
project = context[:project]
- self.class.references_in(text) do |match, id|
- if label = project.labels.find_by(id: id)
+ self.class.references_in(text) do |match, id, name|
+ params = label_params(id, name)
+
+ if label = project.labels.find_by(params)
url = url_for_label(project, label)
klass = "gfm gfm-label #{context[:reference_class]}".strip
@@ -92,6 +103,22 @@ module Gitlab
LabelsHelper.render_colored_label(label)
end
+ # Parameters to pass to `Label.find_by` based on the given arguments
+ #
+ # id - Integer ID to pass. If present, returns {id: id}
+ # name - String name to pass. If `id` is absent, finds by name without
+ # surrounding quotes.
+ #
+ # Returns a Hash.
+ def label_params(id, name)
+ if id > 0
+ { id: id }
+ else
+ # TODO (rspeicher): Don't strip single quotes if we decide to only use double quotes for surrounding.
+ { name: name.tr('\'"', '') }
+ end
+ end
+
def project
context[:project]
end
diff --git a/spec/lib/gitlab/markdown/label_reference_filter_spec.rb b/spec/lib/gitlab/markdown/label_reference_filter_spec.rb
index 49243484c95..2258889cb51 100644
--- a/spec/lib/gitlab/markdown/label_reference_filter_spec.rb
+++ b/spec/lib/gitlab/markdown/label_reference_filter_spec.rb
@@ -21,24 +21,6 @@ module Gitlab::Markdown
end
end
- it 'links to a valid reference' do
- doc = filter("See #{reference}")
-
- expect(doc.css('a').first.attr('href')).to eq urls.
- namespace_project_issues_url(project.namespace, project, label_name: label.name)
- end
-
- it 'links with adjacent text' do
- doc = filter("Label (#{reference}.)")
- expect(doc.to_html).to match(%r(\(#{label.name}\.\)))
- end
-
- it 'ignores invalid label IDs' do
- exp = act = "Label ~#{label.id + 1}"
-
- expect(filter(act).to_html).to eq exp
- end
-
it 'includes default classes' do
doc = filter("Label #{reference}")
expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-label'
@@ -68,5 +50,100 @@ module Gitlab::Markdown
expect(doc.css('a span').first.attr('style')).to match(/\Abackground-color: #\h{6}; color: #\h{6}\z/)
end
end
+
+ context 'Integer-based references' do
+ it 'links to a valid reference' do
+ doc = filter("See #{reference}")
+
+ expect(doc.css('a').first.attr('href')).to eq urls.
+ namespace_project_issues_url(project.namespace, project, label_name: label.name)
+ end
+
+ it 'links with adjacent text' do
+ doc = filter("Label (#{reference}.)")
+ expect(doc.to_html).to match(%r(\(#{label.name}\.\)))
+ end
+
+ it 'ignores invalid label IDs' do
+ exp = act = "Label ~#{label.id + 1}"
+
+ expect(filter(act).to_html).to eq exp
+ end
+ end
+
+ context 'String-based single-word references' do
+ let(:label) { create(:label, name: 'gfm', project: project) }
+ let(:reference) { "~#{label.name}" }
+
+ it 'links to a valid reference' do
+ doc = filter("See #{reference}")
+
+ expect(doc.css('a').first.attr('href')).to eq urls.
+ namespace_project_issues_url(project.namespace, project, label_name: label.name)
+ expect(doc.text).to eq 'See gfm'
+ end
+
+ it 'links with adjacent text' do
+ skip 'FIXME (rspeicher): This will fail, because a period and parentheses are both currently valid in label names.'
+ doc = filter("Label (#{reference}.)")
+ expect(doc.to_html).to match(%r(\(#{label.name}\.\)))
+ end
+
+ it 'ignores invalid label names' do
+ exp = act = "Label ~#{label.name.reverse}"
+
+ expect(filter(act).to_html).to eq exp
+ end
+ end
+
+ context 'String-based multi-word references in quotes' do
+ let(:label) { create(:label, name: 'gfm references', project: project) }
+
+ context 'in single quotes' do
+ let(:reference) { "~'#{label.name}'" }
+
+ it 'links to a valid reference' do
+ doc = filter("See #{reference}")
+
+ expect(doc.css('a').first.attr('href')).to eq urls.
+ namespace_project_issues_url(project.namespace, project, label_name: label.name)
+ expect(doc.text).to eq 'See gfm references'
+ end
+
+ it 'links with adjacent text' do
+ doc = filter("Label (#{reference}.)")
+ expect(doc.to_html).to match(%r(\(#{label.name}\.\)))
+ end
+
+ it 'ignores invalid label names' do
+ exp = act = "Label ~'#{label.name.reverse}'"
+
+ expect(filter(act).to_html).to eq exp
+ end
+ end
+
+ context 'in double quotes' do
+ let(:reference) { %(~"#{label.name}") }
+
+ it 'links to a valid reference' do
+ doc = filter("See #{reference}")
+
+ expect(doc.css('a').first.attr('href')).to eq urls.
+ namespace_project_issues_url(project.namespace, project, label_name: label.name)
+ expect(doc.text).to eq 'See gfm references'
+ end
+
+ it 'links with adjacent text' do
+ doc = filter("Label (#{reference}.)")
+ expect(doc.to_html).to match(%r(\(#{label.name}\.\)))
+ end
+
+ it 'ignores invalid label names' do
+ exp = act = %(Label ~"#{label.name.reverse}")
+
+ expect(filter(act).to_html).to eq exp
+ end
+ end
+ end
end
end