diff options
author | Brett Walker <bwalker@gitlab.com> | 2018-09-02 18:24:27 +0300 |
---|---|---|
committer | Brett Walker <bwalker@gitlab.com> | 2018-09-05 17:19:16 +0300 |
commit | a963721f797b451efec15702e73752c8b8830631 (patch) | |
tree | 46b920a9b5fd0078d5cfdcdecb8e5c16be852f21 | |
parent | e41b99943230240403bdca7b19d5f379892cd819 (diff) |
render using RedCarpet if legacy_render parameter is set
-rw-r--r-- | app/helpers/markup_helper.rb | 6 | ||||
-rw-r--r-- | app/services/preview_markdown_service.rb | 6 | ||||
-rw-r--r-- | app/views/projects/_wiki.html.haml | 3 | ||||
-rw-r--r-- | app/views/projects/blob/edit.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/blob/preview.html.haml | 3 | ||||
-rw-r--r-- | app/views/projects/blob/viewers/_markup.html.haml | 8 | ||||
-rw-r--r-- | app/views/projects/wikis/_form.html.haml | 4 | ||||
-rw-r--r-- | app/views/projects/wikis/_sidebar.html.haml | 3 | ||||
-rw-r--r-- | app/views/projects/wikis/show.html.haml | 3 | ||||
-rw-r--r-- | app/views/search/results/_snippet_blob.html.haml | 3 | ||||
-rw-r--r-- | changelogs/unreleased/bw-commonmark-for-files.yml | 5 | ||||
-rw-r--r-- | spec/features/projects/blobs/blob_show_spec.rb | 50 | ||||
-rw-r--r-- | spec/features/projects/blobs/edit_spec.rb | 34 | ||||
-rw-r--r-- | spec/features/projects/wiki/markdown_preview_spec.rb | 28 | ||||
-rw-r--r-- | spec/features/snippets/show_spec.rb | 44 | ||||
-rw-r--r-- | spec/services/preview_markdown_service_spec.rb | 7 |
16 files changed, 181 insertions, 28 deletions
diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb index 39e7a7fd396..a73f962e4ff 100644 --- a/app/helpers/markup_helper.rb +++ b/app/helpers/markup_helper.rb @@ -111,17 +111,17 @@ module MarkupHelper prepare_for_rendering(html, context) end - def render_wiki_content(wiki_page) + def render_wiki_content(wiki_page, context = {}) text = wiki_page.content return '' unless text.present? - context = { + context.merge!({ pipeline: :wiki, project: @project, project_wiki: @project_wiki, page_slug: wiki_page.slug, issuable_state_filter_enabled: true - } + }) html = case wiki_page.format diff --git a/app/services/preview_markdown_service.rb b/app/services/preview_markdown_service.rb index 11b996ed4b6..de8757006f1 100644 --- a/app/services/preview_markdown_service.rb +++ b/app/services/preview_markdown_service.rb @@ -43,6 +43,10 @@ class PreviewMarkdownService < BaseService end def markdown_engine - CacheMarkdownField::MarkdownEngine.from_version(params[:markdown_version].to_i) + if params[:legacy_render] + :redcarpet + else + CacheMarkdownField::MarkdownEngine.from_version(params[:markdown_version].to_i) + end end end diff --git a/app/views/projects/_wiki.html.haml b/app/views/projects/_wiki.html.haml index 5646dc464f8..0737f94e766 100644 --- a/app/views/projects/_wiki.html.haml +++ b/app/views/projects/_wiki.html.haml @@ -2,7 +2,8 @@ %div{ class: container_class } .prepend-top-default.append-bottom-default .wiki - = render_wiki_content(@wiki_home) + - context = params[:legacy_render] ? { markdown_engine: :redcarpet} : {} + = render_wiki_content(@wiki_home, context) - else - can_create_wiki = can?(current_user, :create_wiki, @project) .project-home-empty{ class: [('row-content-block' if can_create_wiki), ('content-block' unless can_create_wiki)] } diff --git a/app/views/projects/blob/edit.html.haml b/app/views/projects/blob/edit.html.haml index 27cf040da7c..fdab8a53b41 100644 --- a/app/views/projects/blob/edit.html.haml +++ b/app/views/projects/blob/edit.html.haml @@ -21,7 +21,7 @@ Write %li - = link_to '#preview', 'data-preview-url' => project_preview_blob_path(@project, @id) do + = link_to '#preview', 'data-preview-url' => project_preview_blob_path(@project, @id, legacy_render: params[:legacy_render]) do = editing_preview_title(@blob.name) = form_tag(project_update_blob_path(@project, @id), method: :put, class: 'js-quick-submit js-requires-input js-edit-blob-form', data: blob_editor_paths) do diff --git a/app/views/projects/blob/preview.html.haml b/app/views/projects/blob/preview.html.haml index da2cef17e8a..da29eb7a133 100644 --- a/app/views/projects/blob/preview.html.haml +++ b/app/views/projects/blob/preview.html.haml @@ -2,7 +2,8 @@ .diff-content - if markup?(@blob.name) .file-content.wiki - = markup(@blob.name, @content) + - context = params[:legacy_render] ? { markdown_engine: :redcarpet} : {} + = markup(@blob.name, @content, context) - else .file-content.code.js-syntax-highlight - unless @diff_lines.empty? diff --git a/app/views/projects/blob/viewers/_markup.html.haml b/app/views/projects/blob/viewers/_markup.html.haml index 230305b488d..ced08c659a9 100644 --- a/app/views/projects/blob/viewers/_markup.html.haml +++ b/app/views/projects/blob/viewers/_markup.html.haml @@ -1,4 +1,8 @@ - blob = viewer.blob -- rendered_markup = blob.rendered_markup if blob.respond_to?(:rendered_markup) +- context = {} +- if params[:legacy_render] + - context[:markdown_engine] = :redcarpet +- else + - context[:rendered] = blob.rendered_markup if blob.respond_to?(:rendered_markup) .file-content.wiki - = markup(blob.name, blob.data, rendered: rendered_markup) + = markup(blob.name, blob.data, context) diff --git a/app/views/projects/wikis/_form.html.haml b/app/views/projects/wikis/_form.html.haml index 05791e9d4fc..707fc1dc842 100644 --- a/app/views/projects/wikis/_form.html.haml +++ b/app/views/projects/wikis/_form.html.haml @@ -1,8 +1,10 @@ - commit_message = @page.persisted? ? s_("WikiPageEdit|Update %{page_title}") : s_("WikiPageCreate|Create %{page_title}") - commit_message = commit_message % { page_title: @page.title } +- markdown_version = params[:legacy_render] ? CacheMarkdownField::CACHE_REDCARPET_VERSION : 0 = form_for [@project.namespace.becomes(Namespace), @project, @page], method: @page.persisted? ? :put : :post, - html: { class: 'wiki-form common-note-form prepend-top-default js-quick-submit' } do |f| + html: { class: 'wiki-form common-note-form prepend-top-default js-quick-submit' }, + data: { markdown_version: markdown_version } do |f| = form_errors(@page) - if @page.persisted? diff --git a/app/views/projects/wikis/_sidebar.html.haml b/app/views/projects/wikis/_sidebar.html.haml index 28353927135..6542ef3b4e0 100644 --- a/app/views/projects/wikis/_sidebar.html.haml +++ b/app/views/projects/wikis/_sidebar.html.haml @@ -12,7 +12,8 @@ .blocks-container .block.block-first - if @sidebar_page - = render_wiki_content(@sidebar_page) + - context = params[:legacy_render] ? { markdown_engine: :redcarpet} : {} + = render_wiki_content(@sidebar_page, context) - else %ul.wiki-pages = render @sidebar_wiki_entries, context: 'sidebar' diff --git a/app/views/projects/wikis/show.html.haml b/app/views/projects/wikis/show.html.haml index a08973c7f32..eb1410fc87d 100644 --- a/app/views/projects/wikis/show.html.haml +++ b/app/views/projects/wikis/show.html.haml @@ -26,6 +26,7 @@ .prepend-top-default.append-bottom-default .wiki - = render_wiki_content(@page) + - context = params[:legacy_render] ? { markdown_engine: :redcarpet} : {} + = render_wiki_content(@page, context) = render 'sidebar' diff --git a/app/views/search/results/_snippet_blob.html.haml b/app/views/search/results/_snippet_blob.html.haml index 57a0b64bfd5..6527d70134e 100644 --- a/app/views/search/results/_snippet_blob.html.haml +++ b/app/views/search/results/_snippet_blob.html.haml @@ -21,7 +21,8 @@ .file-content.wiki - snippet_chunks.each do |chunk| - unless chunk[:data].empty? - = markup(snippet.file_name, chunk[:data]) + - context = params[:legacy_render] ? { markdown_engine: :redcarpet} : {} + = markup(snippet.file_name, chunk[:data], context) - else .file-content.code .nothing-here-block Empty file diff --git a/changelogs/unreleased/bw-commonmark-for-files.yml b/changelogs/unreleased/bw-commonmark-for-files.yml new file mode 100644 index 00000000000..f932ccb704f --- /dev/null +++ b/changelogs/unreleased/bw-commonmark-for-files.yml @@ -0,0 +1,5 @@ +--- +title: Render files (`.md`) and wikis using CommonMark +merge_request: 21228 +author: +type: changed diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 1064f72c271..2ba4d4918ff 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -5,8 +5,8 @@ describe 'File blob', :js do let(:project) { create(:project, :public, :repository) } - def visit_blob(path, anchor: nil, ref: 'master') - visit project_blob_path(project, File.join(ref, path), anchor: anchor) + def visit_blob(path, anchor: nil, ref: 'master', legacy_render: nil) + visit project_blob_path(project, File.join(ref, path), anchor: anchor, legacy_render: legacy_render) wait_for_requests end @@ -142,6 +142,52 @@ describe 'File blob', :js do end end + context 'Markdown rendering' do + before do + project.add_maintainer(project.creator) + + Files::CreateService.new( + project, + project.creator, + start_branch: 'master', + branch_name: 'master', + commit_message: "Add RedCarpet and CommonMark Markdown ", + file_path: 'files/commonmark/file.md', + file_content: "1. one\n - sublist\n" + ).execute + end + + context 'when rendering default markdown' do + before do + visit_blob('files/commonmark/file.md') + + wait_for_requests + end + + it 'renders using CommonMark' do + aggregate_failures do + expect(page).to have_content("sublist") + expect(page).not_to have_xpath("//ol//li//ul") + end + end + end + + context 'when rendering legacy markdown' do + before do + visit_blob('files/commonmark/file.md', legacy_render: 1) + + wait_for_requests + end + + it 'renders using RedCarpet' do + aggregate_failures do + expect(page).to have_content("sublist") + expect(page).to have_xpath("//ol//li//ul") + end + end + end + end + context 'Markdown file (stored in LFS)' do before do project.add_maintainer(project.creator) diff --git a/spec/features/projects/blobs/edit_spec.rb b/spec/features/projects/blobs/edit_spec.rb index 0e036b4ea68..d5b20605860 100644 --- a/spec/features/projects/blobs/edit_spec.rb +++ b/spec/features/projects/blobs/edit_spec.rb @@ -7,6 +7,7 @@ describe 'Editing file blob', :js do let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') } let(:branch) { 'master' } let(:file_path) { project.repository.ls_files(project.repository.root_ref)[1] } + let(:readme_file_path) { 'README.md' } context 'as a developer' do let(:user) { create(:user) } @@ -20,14 +21,19 @@ describe 'Editing file blob', :js do def edit_and_commit(commit_changes: true) wait_for_requests find('.js-edit-blob').click - find('#editor') - execute_script('ace.edit("editor").setValue("class NextFeature\nend\n")') + fill_editor(content: "class NextFeature\\nend\\n") if commit_changes click_button 'Commit changes' end end + def fill_editor(content: "class NextFeature\\nend\\n") + wait_for_requests + find('#editor') + execute_script("ace.edit('editor').setValue('#{content}')") + end + context 'from MR diff' do before do visit diffs_project_merge_request_path(project, merge_request) @@ -63,6 +69,30 @@ describe 'Editing file blob', :js do expect(new_line_count).to be > 0 end end + + context 'when rendering the preview' do + it 'renders content with CommonMark' do + visit project_edit_blob_path(project, tree_join(branch, readme_file_path)) + fill_editor(content: "1. one\\n - sublist\\n") + click_link 'Preview' + wait_for_requests + + # the above generates two seperate lists (not embedded) in CommonMark + expect(page).to have_content("sublist") + expect(page).not_to have_xpath("//ol//li//ul") + end + + it 'renders content with RedCarpet when legacy_render is set' do + visit project_edit_blob_path(project, tree_join(branch, readme_file_path), legacy_render: 1) + fill_editor(content: "1. one\\n - sublist\\n") + click_link 'Preview' + wait_for_requests + + # the above generates a sublist list in RedCarpet + expect(page).to have_content("sublist") + expect(page).to have_xpath("//ol//li//ul") + end + end end context 'visit blob edit' do diff --git a/spec/features/projects/wiki/markdown_preview_spec.rb b/spec/features/projects/wiki/markdown_preview_spec.rb index ed5f8105487..f505023d1d0 100644 --- a/spec/features/projects/wiki/markdown_preview_spec.rb +++ b/spec/features/projects/wiki/markdown_preview_spec.rb @@ -162,6 +162,34 @@ describe 'Projects > Wiki > User previews markdown changes', :js do expect(page.html).to include("<a href=\"/#{project.full_path}/wikis/title%20with%20spaces\">spaced link</a>") end end + + context 'when rendering the preview' do + it 'renders content with CommonMark' do + create_wiki_page 'a-page/b-page/c-page/common-mark' + click_link 'Edit' + + fill_in :wiki_content, with: "1. one\n - sublist\n" + click_on "Preview" + + # the above generates two seperate lists (not embedded) in CommonMark + expect(page).to have_content("sublist") + expect(page).not_to have_xpath("//ol//li//ul") + end + + it 'renders content with RedCarpet when legacy_render is set' do + wiki_page = create(:wiki_page, + wiki: project.wiki, + attrs: { title: 'home', content: "Empty content" }) + visit(project_wiki_edit_path(project, wiki_page, legacy_render: 1)) + + fill_in :wiki_content, with: "1. one\n - sublist\n" + click_on "Preview" + + # the above generates a sublist list in RedCarpet + expect(page).to have_content("sublist") + expect(page).to have_xpath("//ol//li//ul") + end + end end it "does not linkify double brackets inside code blocks as expected" do diff --git a/spec/features/snippets/show_spec.rb b/spec/features/snippets/show_spec.rb index 3fe0b60b18f..367a479f62a 100644 --- a/spec/features/snippets/show_spec.rb +++ b/spec/features/snippets/show_spec.rb @@ -68,23 +68,45 @@ describe 'Snippet', :js do end end - context 'with cached Redcarpet html' do - let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_REDCARPET_VERSION) } + context 'Markdown rendering' do + let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content) } let(:file_name) { 'test.md' } let(:content) { "1. one\n - sublist\n" } - it 'renders correctly' do - expect(page).to have_xpath("//ol//li//ul") + context 'when rendering default markdown' do + it 'renders using CommonMark' do + expect(page).to have_content("sublist") + expect(page).not_to have_xpath("//ol//li//ul") + end end - end - context 'with cached CommonMark html' do - let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) } - let(:file_name) { 'test.md' } - let(:content) { "1. one\n - sublist\n" } + context 'when rendering legacy markdown' do + before do + visit snippet_path(snippet, legacy_render: 1) - it 'renders correctly' do - expect(page).not_to have_xpath("//ol//li//ul") + wait_for_requests + end + + it 'renders using RedCarpet' do + expect(page).to have_content("sublist") + expect(page).to have_xpath("//ol//li//ul") + end + end + + context 'with cached CommonMark html' do + let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION) } + + it 'renders correctly' do + expect(page).not_to have_xpath("//ol//li//ul") + end + end + + context 'with cached Redcarpet html' do + let(:snippet) { create(:personal_snippet, :public, file_name: file_name, content: content, cached_markdown_version: CacheMarkdownField::CACHE_REDCARPET_VERSION) } + + it 'renders correctly' do + expect(page).to have_xpath("//ol//li//ul") + end end end diff --git a/spec/services/preview_markdown_service_spec.rb b/spec/services/preview_markdown_service_spec.rb index 507909d9231..b69977c812a 100644 --- a/spec/services/preview_markdown_service_spec.rb +++ b/spec/services/preview_markdown_service_spec.rb @@ -101,4 +101,11 @@ describe PreviewMarkdownService do expect(result[:markdown_engine]).to eq :common_mark end + + it 'honors the legacy_render parameter' do + service = described_class.new(project, user, { legacy_render: '1' }) + result = service.execute + + expect(result[:markdown_engine]).to eq :redcarpet + end end |