Merge branch 'header-anchors' of github.com:cirosantilli/gitlab-elearn into...

Merge branch 'header-anchors' of github.com:cirosantilli/gitlab-elearn into cirosantilli-header-anchors
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

Conflicts:
	CHANGELOG

CHANGELOG

@@ -14,6 +14,7 @@ v 6.6.0
   - Fix block/remove UI for admin::users#show page
   - Show users' group membership on users' activity page
   - User pages are visible without login if user is authorized to a public project
+  - Markdown rendered headers have id derived from their name and link to their id
 
 v 6.5.1
   - Fix branch selectbox when create merge request from fork

app/assets/stylesheets/generic/files.scss

@@ -50,7 +50,6 @@
     }
 
     &.wiki {
-      padding: 20px;
       font-size: 14px;
       line-height: 1.6;
 

app/assets/stylesheets/generic/issue_box.scss

@@ -23,11 +23,12 @@
     line-height: 28px;
     margin: 0;
     color: #444;
+    border-bottom: 1px solid #eee;
   }
 
   .context {
     border: none;
-    border-top: 1px solid #eee;
+    border-bottom: 1px solid #eee;
   }
 
   .description {
@@ -35,7 +36,7 @@
   }
 
   .title, .context, .description {
-    padding: 15px;
+    padding: 15px 15px 15px 30px;
 
     .clearfix {
       margin: 0;

app/assets/stylesheets/generic/typography.scss

@@ -90,6 +90,27 @@ a:focus {
 
   font-size: 14px;
   line-height: 1.6;
+
+  /* Link to current header. */
+  h1, h2, h3, h4, h5, h6 {
+    position: relative;
+    &:hover > :last-child {
+      $size: 16px;
+      position: absolute;
+      right: 100%;
+      top: 50%;
+      margin-top: -$size/2;
+      margin-right: 0px;
+      padding-right: 20px;
+      display: inline-block;
+      width: $size;
+      height: $size;
+      background-image: url("icon-link.png");
+      background-size: contain;
+      background-repeat: no-repeat;
+    }
+  }
+
   ul {
     padding: 0;
     margin: 0 0 9px 25px !important;

app/assets/stylesheets/main/mixins.scss

@@ -114,6 +114,10 @@
     font-size: 1.2em;
   }
 
+  // Larger 30px left margin is required for the header link icon.
+  // Use on all markdown including those without header links for uniformity.
+  margin: 20px 20px 20px 30px;
+
   blockquote p {
     color: #888;
     font-size: 14px;

app/helpers/gitlab_markdown_helper.rb

@@ -28,14 +28,16 @@ module GitlabMarkdownHelper
     link_to(gfm_body.html_safe, url, html_options)
   end
 
-  def markdown(text)
-    unless @markdown
-      gitlab_renderer = Redcarpet::Render::GitlabHTML.new(self,
-                          # see https://github.com/vmg/redcarpet#darling-i-packed-you-a-couple-renderers-for-lunch-
-                          filter_html: true,
-                          with_toc_data: true,
-                          hard_wrap: true,
-                          safe_links_only: true)
+  def markdown(text, options={})
+    unless (@markdown and options == @options)
+      @options = options
+      gitlab_renderer = Redcarpet::Render::GitlabHTML.new(self, {
+                            # see https://github.com/vmg/redcarpet#darling-i-packed-you-a-couple-renderers-for-lunch-
+                            filter_html: true,
+                            with_toc_data: true,
+                            hard_wrap: true,
+                            safe_links_only: true
+                          }.merge(options))
       @markdown = Redcarpet::Markdown.new(gitlab_renderer,
                       # see https://github.com/vmg/redcarpet#and-its-like-really-simple-to-use
                       no_intra_emphasis: true,
@@ -47,7 +49,6 @@ module GitlabMarkdownHelper
                       space_after_headers: true,
                       superscript: true)
     end
-
     @markdown.render(text).html_safe
   end
 

app/views/help/_layout.html.haml

@@ -8,4 +8,5 @@
           = link_to title, path
 
   .col-md-9
-    = yield
+    .wiki
+      = yield

app/views/projects/issues/show.html.haml

@@ -46,10 +46,9 @@
       = render partial: 'issue_context', locals: { issue: @issue }
 
   - if @issue.description.present?
-    .description
-      .wiki
-        = preserve do
-          = markdown @issue.description
+    .wiki
+      = preserve do
+        = markdown @issue.description
 
 - content_for :note_actions do
   - if can?(current_user, :modify_issue, @issue)

app/views/projects/merge_requests/show/_mr_box.html.haml

@@ -15,10 +15,9 @@
 
 
   - if @merge_request.description.present?
-    .description
-      .wiki
-        = preserve do
-          = markdown @merge_request.description
+    .wiki
+      = preserve do
+        = markdown @merge_request.description
 
   - if @merge_request.closed?
     .description.alert-danger

app/views/projects/milestones/show.html.haml

@@ -42,13 +42,11 @@
     .progress.progress-info
       .progress-bar{style: "width: #{@milestone.percent_complete}%;"}
 
-
   - if @milestone.description.present?
-    .description
+    .wiki
       = preserve do
         = markdown @milestone.description
 
-
 %ul.nav.nav-tabs.append-bottom-10
   %li.active
     = link_to '#tab-issues', 'data-toggle' => 'tab' do

app/views/projects/notes/_note.html.haml

@@ -31,7 +31,7 @@
   .note-body
     .note-text
       = preserve do
-        = markdown(note.note)
+        = markdown(note.note, {no_header_anchors: true})
 
     .note-edit-form
       = form_for note, url: project_note_path(@project, note), method: :put, remote: true, authenticity_token: true do |f|

features/dashboard/help.feature

+Feature: Help
+  Background:
+    Given I sign in as a user
+    And I visit the "Rake Tasks" help page
+
+  Scenario: The markdown should be rendered correctly
+    Then I should see "Rake Tasks" page markdown rendered
+    And Header "Rebuild project satellites" should have correct ids and links

features/project/issues/issues.feature

@@ -55,3 +55,15 @@ Feature: Project Issues
     And I fill in issue search with ".3"
     Then I should see "Release 0.3" in issues
     And I should not see "Release 0.4" in issues
+
+  # Markdown
+
+  Scenario: Headers inside the description should have ids generated for them.
+    Given I visit issue page "Release 0.4"
+    Then Header "Description header" should have correct id and link
+
+  @javascript
+  Scenario: Headers inside comments should not have ids generated for them.
+    Given I visit issue page "Release 0.4"
+    And I leave a comment with a header containing "Comment with a header"
+    Then The comment with the header should not have an ID

features/project/issues/milestones.feature

@@ -22,3 +22,9 @@ Feature: Project Milestones
     Given the milestone has open and closed issues
     And I click link "v2.2"
     Then I should see 3 issues
+
+  # Markdown
+
+  Scenario: Headers inside the description should have ids generated for them.
+    Given I click link "v2.2"
+    Then Header "Description header" should have correct id and link

features/project/merge_requests.feature

@@ -77,3 +77,15 @@ Feature: Project Merge Requests
     Then I modify merge commit message
     And I accept this merge request
     Then I should see merged request
+
+  # Markdown
+
+  Scenario: Headers inside the description should have ids generated for them.
+    When I visit merge request page "Bug NS-04"
+    Then Header "Description header" should have correct id and link
+
+  @javascript
+  Scenario: Headers inside comments should not have ids generated for them.
+    Given I visit merge request page "Bug NS-04"
+    And I leave a comment with a header containing "Comment with a header"
+    Then The comment with the header should not have an ID

features/project/source/markdown_render.feature

@@ -4,6 +4,15 @@ Feature: Project markdown render
     And I own project "Delta"
     Given I visit project source page
 
+  # -------------------------------------------
+  # README
+  # -------------------------------------------
+
+  Scenario: Tree view should have correct links in README
+    Given I go directory which contains README file
+    And I click on a relative link in README
+    Then I should see the correct markdown
+
   Scenario: I browse files from master branch
     Then I should see files from repository in master
     And I should see rendered README which contains correct links
@@ -28,6 +37,14 @@ Feature: Project markdown render
     And I click on Maintenance in README
     Then I should see correct maintenance file rendered
 
+  Scenario: README headers should have header links
+    Then I should see rendered README which contains correct links
+    And Header "Application details" should have correct id and link
+
+  # -------------------------------------------
+  # File content
+  # -------------------------------------------
+
   Scenario: I navigate to doc directory to view documentation in master
     And I navigate to the doc/api/README
     And I see correct file rendered
@@ -40,6 +57,14 @@ Feature: Project markdown render
     And I click on raketasks in doc/api/README
     Then I should see correct directory rendered
 
+  Scenario: I navigate to doc directory to view user doc in master
+    And I navigate to the doc/api/README
+    And Header "GitLab API" should have correct id and link
+
+  # -------------------------------------------
+  # Markdown branch README
+  # -------------------------------------------
+
   Scenario: I browse files from markdown branch
     When I visit markdown branch
     Then I should see files from repository in markdown branch
@@ -68,6 +93,10 @@ Feature: Project markdown render
     And I click on raketasks in doc/api/README
     Then I should see correct directory rendered for markdown branch
 
+  # -------------------------------------------
+  # Wiki
+  # -------------------------------------------
+
   Scenario: I create a wiki page with different links
     Given I go to wiki page
     And I add various links to the wiki page
@@ -81,12 +110,7 @@ Feature: Project markdown render
     And I click on Rake tasks link
     Then I see Rake tasks directory
 
-  Scenario: I visit the help page with markdown
-    Given I visit to the help page
-    And I select a page with markdown
-    Then I should see a help page with markdown
-
-  Scenario: Tree view should have correct links in README
-    Given I go directory which contains README file
-    And I click on a relative link in README
-    Then I should see the correct markdown
+  Scenario: Wiki headers should have should have ids generated for them.
+    Given I go to wiki page
+    And I add a header to the wiki page
+    Then Wiki header should have correct id and link

features/steps/help.rb

+class Spinach::Features::Help < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedPaths
+  include SharedMarkdown
+
+  step 'I visit the help page' do
+    visit help_path
+  end
+
+  step 'I visit the "Rake Tasks" help page' do
+    visit help_raketasks_path
+  end
+
+  step 'I should see "Rake Tasks" page markdown rendered' do
+    page.should have_content "GitLab provides some specific rake tasks to enable special features or perform maintenance tasks"
+  end
+
+  step 'Header "Rebuild project satellites" should have correct ids and links' do
+    header_should_have_correct_id_and_link(3, 'Rebuild project satellites', 'rebuild-project-satellites')
+  end
+end

features/steps/project/project_issues.rb

@@ -3,6 +3,7 @@ class ProjectIssues < Spinach::FeatureSteps
   include SharedProject
   include SharedNote
   include SharedPaths
+  include SharedMarkdown
 
   Given 'I should see "Release 0.4" in issues' do
     page.should have_content "Release 0.4"
@@ -121,7 +122,9 @@ class ProjectIssues < Spinach::FeatureSteps
     create(:issue,
            title: "Release 0.4",
            project: project,
-           author: project.users.first)
+           author: project.users.first,
+           description: "# Description header"
+          )
   end
 
   And 'project "Shop" have "Tweet control" open issue' do

features/steps/project/project_markdown_render.rb

 class Spinach::Features::ProjectMarkdownRender < Spinach::FeatureSteps
   include SharedAuthentication
   include SharedPaths
+  include SharedMarkdown
 
   And 'I own project "Delta"' do
     @project = Project.find_by(name: "Delta")
@@ -44,7 +45,6 @@ class Spinach::Features::ProjectMarkdownRender < Spinach::FeatureSteps
     page.should have_content "maintenance.md"
   end
 
-
   And 'I click on GitLab API doc directory in README' do
     click_link "GitLab API doc directory"
   end
@@ -140,6 +140,16 @@ class Spinach::Features::ProjectMarkdownRender < Spinach::FeatureSteps
     page.should have_content "test GitLab API doc Rake tasks"
   end
 
+  step 'I add a header to the wiki page' do
+    fill_in "wiki[content]", with: "# Wiki header\n"
+    fill_in "wiki[message]", with: "Add header to wiki"
+    click_button "Create page"
+  end
+
+  step 'Wiki header should have correct id and link' do
+    header_should_have_correct_id_and_link(1, 'Wiki header', 'wiki-header')
+  end
+
   And 'I click on test link' do
     click_link "test"
   end
@@ -173,18 +183,6 @@ class Spinach::Features::ProjectMarkdownRender < Spinach::FeatureSteps
     page.should have_content "maintenance.md"
   end
 
-  Given 'I visit to the help page' do
-    visit help_path
-  end
-
-  And 'I select a page with markdown' do
-    click_link "Rake Tasks"
-  end
-
-  Then 'I should see a help page with markdown' do
-    page.should have_content "GitLab provides some specific rake tasks to enable special features or perform maintenance tasks"
-  end
-
   Given 'I go directory which contains README file' do
     visit project_tree_path(@project, "master/doc/api")
     current_path.should == project_tree_path(@project, "master/doc/api")
@@ -198,4 +196,12 @@ class Spinach::Features::ProjectMarkdownRender < Spinach::FeatureSteps
     current_path.should == project_blob_path(@project, "master/doc/api/users.md")
     page.should have_content "List users"
   end
+
+  step 'Header "Application details" should have correct id and link' do
+    header_should_have_correct_id_and_link(2, 'Application details', 'application-details')
+  end
+
+  step 'Header "GitLab API" should have correct id and link' do
+    header_should_have_correct_id_and_link(1, 'GitLab API', 'gitlab-api')
+  end
 end

features/steps/project/project_merge_requests.rb

@@ -3,6 +3,7 @@ class ProjectMergeRequests < Spinach::FeatureSteps
   include SharedProject
   include SharedNote
   include SharedPaths
+  include SharedMarkdown
 
   step 'I click link "New Merge Request"' do
     click_link "New Merge Request"
@@ -83,7 +84,9 @@ class ProjectMergeRequests < Spinach::FeatureSteps
            target_project: project,
            source_branch: 'stable',
            target_branch: 'master',
-           author: project.users.first)
+           author: project.users.first,
+           description: "# Description header"
+          )
   end
 
   step 'project "Shop" have "Bug NS-05" open merge request with diffs inside' do

features/steps/project/project_milestones.rb

@@ -2,6 +2,7 @@ class ProjectMilestones < Spinach::FeatureSteps
   include SharedAuthentication
   include SharedProject
   include SharedPaths
+  include SharedMarkdown
 
   Then 'I should see milestone "v2.2"' do
     milestone = @project.milestones.find_by(title: "v2.2")
@@ -32,8 +33,11 @@ class ProjectMilestones < Spinach::FeatureSteps
 
   And 'project "Shop" has milestone "v2.2"' do
     project = Project.find_by(name: "Shop")
-    milestone = create(:milestone, title: "v2.2", project: project)
-
+    milestone = create(:milestone,
+                       title: "v2.2",
+                       project: project,
+                       description: "# Description header"
+                      )
     3.times { create(:issue, project: project, milestone: milestone) }
   end
 

features/steps/shared/markdown.rb

+module SharedMarkdown
+  include Spinach::DSL
+
+  def header_should_have_correct_id_and_link(level, text, id, parent = ".wiki")
+    page.find(:css, "#{parent} h#{level}##{id}").text.should == text
+    page.find(:css, "#{parent} h#{level}##{id} > :last-child")[:href].should =~ /##{id}$/
+  end
+
+  step 'Header "Description header" should have correct id and link' do
+    header_should_have_correct_id_and_link(1, 'Description header', 'description-header')
+  end
+end

features/steps/shared/note.rb

@@ -102,4 +102,21 @@ module SharedNote
       page.should have_content("XML attached")
     end
   end
+
+  # Markdown
+
+  step 'I leave a comment with a header containing "Comment with a header"' do
+    within(".js-main-target-form") do
+      fill_in "note[note]", with: "# Comment with a header"
+      click_button "Add Comment"
+      sleep 0.05
+    end
+  end
+
+  step 'The comment with the header should not have an ID' do
+    within(".note-text") do
+      page.should     have_content("Comment with a header")
+      page.should_not have_css("#comment-with-a-header")
+    end
+  end
 end

lib/redcarpet/render/gitlab_html.rb

@@ -8,6 +8,7 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML
     @project = @template.instance_variable_get("@project")
     @ref = @template.instance_variable_get("@ref")
     @request_path = @template.instance_variable_get("@path")
+    @options = options.dup
     super options
   end
 
@@ -34,6 +35,16 @@ class Redcarpet::Render::GitlabHTML < Redcarpet::Render::HTML
     h.link_to_gfm(content, link, title: title)
   end
 
+  def header(text, level)
+    if @options[:no_header_anchors]
+      "<h#{level}>#{text}</h#{level}>"
+    else
+      id = ActionController::Base.helpers.strip_tags(h.gfm(text)).downcase() \
+          .gsub(/[^a-z0-9_-]/, '-').gsub(/-+/, '-').gsub(/^-/, '').gsub(/-$/, '')
+      "<h#{level} id=\"#{id}\">#{text}<a href=\"\##{id}\"></a></h#{level}>"
+    end
+  end
+
   def preprocess(full_document)
     if @project
       h.create_relative_links(full_document, @project, @ref, @request_path, is_wiki?)

spec/helpers/gitlab_markdown_helper_spec.rb

@@ -348,8 +348,21 @@ describe GitlabMarkdownHelper do
     it "should handle references in headers" do
       actual = "\n# Working around ##{issue.iid}\n## Apply !#{merge_request.iid}"
 
-      markdown(actual).should match(%r{<h1[^<]*>Working around <a.+>##{issue.iid}</a></h1>})
-      markdown(actual).should match(%r{<h2[^<]*>Apply <a.+>!#{merge_request.iid}</a></h2>})
+      markdown(actual, {no_header_anchors:true}).should match(%r{<h1[^<]*>Working around <a.+>##{issue.iid}</a></h1>})
+      markdown(actual, {no_header_anchors:true}).should match(%r{<h2[^<]*>Apply <a.+>!#{merge_request.iid}</a></h2>})
+    end
+
+    it "should add ids and links to headers" do
+      # Test every rule except nested tags.
+      text = '..Ab_c-d. e..'
+      id = 'ab_c-d-e'
+      markdown("# #{text}").should match(%r{<h1 id="#{id}">#{text}<a href="[^"]*##{id}"></a></h1>})
+      markdown("# #{text}", {no_header_anchors:true}).should == "<h1>#{text}</h1>"
+
+      id = 'link-text'
+      markdown("# [link text](url) ![img alt](url)").should match(
+        %r{<h1 id="#{id}"><a href="[^"]*url">link text</a> <img[^>]*><a href="[^"]*##{id}"></a></h1>}
+      )
     end
 
     it "should handle references in lists" do