Commit e918ae240ee860c9126f537bbee38a1e92cb7508
Exists in
spb-stable
and in
2 other branches
Merge branch 'no_link_inside_code_block' into 'master'
No link inside code block
Showing
4 changed files
with
69 additions
and
83 deletions
Show diff stats
app/helpers/gitlab_markdown_helper.rb
| ... | ... | @@ -59,90 +59,67 @@ module GitlabMarkdownHelper |
| 59 | 59 | end |
| 60 | 60 | end |
| 61 | 61 | |
| 62 | - # text - whole text from a markdown file | |
| 63 | - # project_path_with_namespace - namespace/projectname, eg. gitlabhq/gitlabhq | |
| 64 | - # ref - name of the branch or reference, eg. stable | |
| 65 | - # requested_path - path of request, eg. doc/api/README.md, used in special case when path is pointing to the .md file were the original request is coming from | |
| 66 | - def create_relative_links(text, project, ref, requested_path) | |
| 67 | - @path_to_satellite = project.satellite.path | |
| 68 | - project_path_with_namespace = project.path_with_namespace | |
| 62 | + def create_relative_links(text) | |
| 69 | 63 | paths = extract_paths(text) |
| 70 | - paths.each do |file_path| | |
| 71 | - original_file_path = extract(file_path) | |
| 72 | - new_path = rebuild_path(project_path_with_namespace, original_file_path, requested_path, ref) | |
| 73 | - if reference_path?(file_path) | |
| 74 | - # Replacing old string with a new one that contains updated path | |
| 75 | - # eg. [some document]: document.md will be replaced with [some document] /namespace/project/master/blob/document.md | |
| 76 | - text.gsub!(file_path, file_path.gsub(original_file_path, "/#{new_path}")) | |
| 77 | - else | |
| 78 | - # Replacing old string with a new one with brackets ]() to prevent replacing occurence of a word | |
| 79 | - # e.g. If we have a markdown like [test](test) this will replace ](test) and not the word test | |
| 80 | - text.gsub!("](#{file_path})", "](/#{new_path})") | |
| 81 | - end | |
| 82 | - end | |
| 83 | - text | |
| 84 | - end | |
| 85 | 64 | |
| 86 | - def extract_paths(markdown_text) | |
| 87 | - all_markdown_paths = pick_out_paths(markdown_text) | |
| 88 | - paths = remove_empty(all_markdown_paths) | |
| 89 | - select_relative(paths) | |
| 90 | - end | |
| 65 | + paths.uniq.each do |file_path| | |
| 66 | + new_path = rebuild_path(file_path) | |
| 67 | + # Finds quoted path so we don't replace other mentions of the string | |
| 68 | + # eg. "doc/api" will be replaced and "/home/doc/api/text" won't | |
| 69 | + text.gsub!("\"#{file_path}\"", "\"/#{new_path}\"") | |
| 70 | + end | |
| 91 | 71 | |
| 92 | - # Split the markdown text to each line and find all paths, this will match anything with - ]("some_text") and [some text]: file.md | |
| 93 | - def pick_out_paths(markdown_text) | |
| 94 | - inline_paths = markdown_text.split("\n").map { |text| text.scan(/\]\(([^(]+)\)/) } | |
| 95 | - reference_paths = markdown_text.split("\n").map { |text| text.scan(/\[.*\]:.*/) } | |
| 96 | - inline_paths + reference_paths | |
| 72 | + text | |
| 97 | 73 | end |
| 98 | 74 | |
| 99 | - # Removes any empty result produced by not matching the regexp | |
| 100 | - def remove_empty(paths) | |
| 101 | - paths.reject{|l| l.empty? }.flatten | |
| 75 | + def extract_paths(text) | |
| 76 | + links = substitute_links(text) | |
| 77 | + image_links = substitute_image_links(text) | |
| 78 | + links + image_links | |
| 102 | 79 | end |
| 103 | 80 | |
| 104 | - # If a path is a reference style link we need to omit ]: | |
| 105 | - def extract(path) | |
| 106 | - path.split("]: ").last | |
| 81 | + def substitute_links(text) | |
| 82 | + links = text.scan(/<a href=\"([^"]*)\">/) | |
| 83 | + relative_links = links.flatten.reject{ |link| link_to_ignore? link } | |
| 84 | + relative_links | |
| 107 | 85 | end |
| 108 | 86 | |
| 109 | - # Reject any path that contains ignored protocol | |
| 110 | - # eg. reject "https://gitlab.org} but accept "doc/api/README.md" | |
| 111 | - def select_relative(paths) | |
| 112 | - paths.reject{|path| ignored_protocols.map{|protocol| path.include?(protocol)}.any?} | |
| 87 | + def substitute_image_links(text) | |
| 88 | + links = text.scan(/<img src=\"([^"]*)\"/) | |
| 89 | + relative_links = links.flatten.reject{ |link| link_to_ignore? link } | |
| 90 | + relative_links | |
| 113 | 91 | end |
| 114 | 92 | |
| 115 | - # Check whether a path is a reference-style link | |
| 116 | - def reference_path?(path) | |
| 117 | - path.include?("]: ") | |
| 93 | + def link_to_ignore?(link) | |
| 94 | + ignored_protocols.map{ |protocol| link.include?(protocol) }.any? | |
| 118 | 95 | end |
| 119 | 96 | |
| 120 | 97 | def ignored_protocols |
| 121 | 98 | ["http://","https://", "ftp://", "mailto:"] |
| 122 | 99 | end |
| 123 | 100 | |
| 124 | - def rebuild_path(path_with_namespace, path, requested_path, ref) | |
| 101 | + def rebuild_path(path) | |
| 125 | 102 | path.gsub!(/(#.*)/, "") |
| 126 | 103 | id = $1 || "" |
| 127 | - file_path = relative_file_path(path, requested_path) | |
| 104 | + file_path = relative_file_path(path) | |
| 105 | + file_path = sanitize_slashes(file_path) | |
| 106 | + | |
| 128 | 107 | [ |
| 129 | - path_with_namespace, | |
| 130 | - path_with_ref(file_path, ref), | |
| 108 | + Gitlab.config.gitlab.relative_url_root, | |
| 109 | + @project.path_with_namespace, | |
| 110 | + path_with_ref(file_path), | |
| 131 | 111 | file_path |
| 132 | - ].compact.join("/").gsub(/\/*$/, '') + id | |
| 112 | + ].compact.join("/").gsub(/^\/*|\/*$/, '') + id | |
| 133 | 113 | end |
| 134 | 114 | |
| 135 | - # Checks if the path exists in the repo | |
| 136 | - # eg. checks if doc/README.md exists, if not then link to blob | |
| 137 | - def path_with_ref(path, ref) | |
| 138 | - if file_exists?(path) | |
| 139 | - "#{local_path(path)}/#{correct_ref(ref)}" | |
| 140 | - else | |
| 141 | - "blob/#{correct_ref(ref)}" | |
| 142 | - end | |
| 115 | + def sanitize_slashes(path) | |
| 116 | + path[0] = "" if path.start_with?("/") | |
| 117 | + path.chop if path.end_with?("/") | |
| 118 | + path | |
| 143 | 119 | end |
| 144 | 120 | |
| 145 | - def relative_file_path(path, requested_path) | |
| 121 | + def relative_file_path(path) | |
| 122 | + requested_path = @path | |
| 146 | 123 | nested_path = build_nested_path(path, requested_path) |
| 147 | 124 | return nested_path if file_exists?(nested_path) |
| 148 | 125 | path |
| ... | ... | @@ -166,6 +143,16 @@ module GitlabMarkdownHelper |
| 166 | 143 | end |
| 167 | 144 | end |
| 168 | 145 | |
| 146 | + # Checks if the path exists in the repo | |
| 147 | + # eg. checks if doc/README.md exists, if not then link to blob | |
| 148 | + def path_with_ref(path) | |
| 149 | + if file_exists?(path) | |
| 150 | + "#{local_path(path)}/#{correct_ref}" | |
| 151 | + else | |
| 152 | + "blob/#{correct_ref}" | |
| 153 | + end | |
| 154 | + end | |
| 155 | + | |
| 169 | 156 | def file_exists?(path) |
| 170 | 157 | return false if path.nil? |
| 171 | 158 | return @repository.blob_at(current_sha, path).present? || @repository.tree(current_sha, path).entries.any? |
| ... | ... | @@ -179,10 +166,6 @@ module GitlabMarkdownHelper |
| 179 | 166 | return "blob" |
| 180 | 167 | end |
| 181 | 168 | |
| 182 | - def current_ref | |
| 183 | - @commit.nil? ? "master" : @commit.id | |
| 184 | - end | |
| 185 | - | |
| 186 | 169 | def current_sha |
| 187 | 170 | if @commit |
| 188 | 171 | @commit.id |
| ... | ... | @@ -192,7 +175,7 @@ module GitlabMarkdownHelper |
| 192 | 175 | end |
| 193 | 176 | |
| 194 | 177 | # We will assume that if no ref exists we can point to master |
| 195 | - def correct_ref(ref) | |
| 196 | - ref ? ref : "master" | |
| 178 | + def correct_ref | |
| 179 | + @ref ? @ref : "master" | |
| 197 | 180 | end |
| 198 | 181 | end | ... | ... |
features/project/issues/issues.feature
| ... | ... | @@ -68,6 +68,12 @@ Feature: Project Issues |
| 68 | 68 | And I leave a comment with a header containing "Comment with a header" |
| 69 | 69 | Then The comment with the header should not have an ID |
| 70 | 70 | |
| 71 | + @javascript | |
| 72 | + Scenario: Blocks inside comments should not build relative links | |
| 73 | + Given I visit issue page "Release 0.4" | |
| 74 | + And I leave a comment with code block | |
| 75 | + Then The code block should be unchanged | |
| 76 | + | |
| 71 | 77 | Scenario: Issues on empty project |
| 72 | 78 | Given empty project "Empty Project" |
| 73 | 79 | When I visit empty project page | ... | ... |
features/steps/project/issues.rb
| ... | ... | @@ -163,4 +163,16 @@ class ProjectIssues < Spinach::FeatureSteps |
| 163 | 163 | project = Project.find_by(name: 'Empty Project') |
| 164 | 164 | visit project_issues_path(project) |
| 165 | 165 | end |
| 166 | + | |
| 167 | + step 'I leave a comment with code block' do | |
| 168 | + within(".js-main-target-form") do | |
| 169 | + fill_in "note[note]", with: "```\nCommand [1]: /usr/local/bin/git , see [text](doc/text)\n```" | |
| 170 | + click_button "Add Comment" | |
| 171 | + sleep 0.05 | |
| 172 | + end | |
| 173 | + end | |
| 174 | + | |
| 175 | + step 'The code block should be unchanged' do | |
| 176 | + page.should have_content("```\nCommand [1]: /usr/local/bin/git , see [text](doc/text)\n```") | |
| 177 | + end | |
| 166 | 178 | end | ... | ... |
lib/redcarpet/render/gitlab_html.rb
| ... | ... | @@ -6,8 +6,6 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML |
| 6 | 6 | def initialize(template, options = {}) |
| 7 | 7 | @template = template |
| 8 | 8 | @project = @template.instance_variable_get("@project") |
| 9 | - @ref = @template.instance_variable_get("@ref") | |
| 10 | - @request_path = @template.instance_variable_get("@path") | |
| 11 | 9 | @options = options.dup |
| 12 | 10 | super options |
| 13 | 11 | end |
| ... | ... | @@ -45,23 +43,10 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML |
| 45 | 43 | end |
| 46 | 44 | end |
| 47 | 45 | |
| 48 | - def preprocess(full_document) | |
| 49 | - if is_wiki? | |
| 50 | - full_document | |
| 51 | - elsif @project | |
| 52 | - h.create_relative_links(full_document, @project, @ref, @request_path) | |
| 53 | - else | |
| 54 | - full_document | |
| 55 | - end | |
| 56 | - end | |
| 57 | - | |
| 58 | 46 | def postprocess(full_document) |
| 59 | - h.gfm(full_document) | |
| 60 | - end | |
| 61 | - | |
| 62 | - def is_wiki? | |
| 63 | - if @template.instance_variable_get("@project_wiki") | |
| 64 | - @template.instance_variable_get("@page") | |
| 47 | + unless @template.instance_variable_get("@project_wiki") || @project.nil? | |
| 48 | + full_document = h.create_relative_links(full_document) | |
| 65 | 49 | end |
| 50 | + h.gfm(full_document) | |
| 66 | 51 | end |
| 67 | 52 | end | ... | ... |