From dbfa58e2da7f939734eee5a599b4014d6095dde3 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 16 Jan 2017 15:27:05 -0500 Subject: [PATCH 01/20] Copying a rendered issue/comment will paste into GFM textareas as actual GFM --- app/assets/javascripts/copy_as_gfm.js.es6 | 319 ++++++++++++++++++ app/assets/javascripts/shortcuts_issuable.js | 48 +-- changelogs/unreleased/copy-as-md.yml | 4 + .../filter/abstract_reference_filter.rb | 5 +- lib/banzai/filter/issue_reference_filter.rb | 2 +- lib/banzai/filter/syntax_highlight_filter.rb | 5 +- lib/banzai/filter/video_link_filter.rb | 3 +- lib/banzai/pipeline/gfm_pipeline.rb | 3 + spec/features/copy_as_gfm_spec.rb | 234 +++++++++++++ 9 files changed, 595 insertions(+), 28 deletions(-) create mode 100644 app/assets/javascripts/copy_as_gfm.js.es6 create mode 100644 changelogs/unreleased/copy-as-md.yml create mode 100644 spec/features/copy_as_gfm_spec.rb diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 new file mode 100644 index 00000000000..4827f7b7420 --- /dev/null +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -0,0 +1,319 @@ +/* eslint-disable class-methods-use-this */ + +(() => { + const gfmRules = { + // Should have an entry for every filter in lib/banzai/pipeline/gfm_pipeline.rb, + // in reverse order. + // Should have test coverage in spec/features/copy_as_gfm_spec.rb. + "InlineDiffFilter": { + "span.idiff.addition": function(el, text) { + return "{+" + text + "+}"; + }, + "span.idiff.deletion": function(el, text) { + return "{-" + text + "-}"; + }, + }, + "TaskListFilter": { + "input[type=checkbox].task-list-item-checkbox": function(el, text) { + return '[' + (el.checked ? 'x' : ' ') + ']'; + } + }, + "ReferenceFilter": { + "a.gfm:not([data-link=true])": function(el, text) { + return el.getAttribute('data-original') || text; + }, + }, + "AutolinkFilter": { + "a": function(el, text) { + if (text != el.getAttribute("href")) { + // Fall back to handler for MarkdownFilter + return false; + } + + return text; + }, + }, + "TableOfContentsFilter": { + "ul.section-nav": function(el, text) { + return "[[_TOC_]]"; + }, + }, + "EmojiFilter": { + "img.emoji": function(el, text) { + return el.getAttribute("alt"); + }, + }, + "ImageLinkFilter": { + "a.no-attachment-icon": function(el, text) { + return text; + }, + }, + "VideoLinkFilter": { + ".video-container": function(el, text) { + var videoEl = el.querySelector('video'); + if (!videoEl) { + return false; + } + + return CopyAsGFM.nodeToGFM(videoEl); + }, + "video": function(el, text) { + return "![" + el.getAttribute('data-title') + "](" + el.getAttribute("src") + ")"; + }, + }, + "MathFilter": { + "pre.code.math[data-math-style='display']": function(el, text) { + return "```math\n" + text.trim() + "\n```"; + }, + "code.code.math[data-math-style='inline']": function(el, text) { + return "$`" + text + "`$"; + }, + "span.katex-display span.katex-mathml": function(el, text) { + var mathAnnotation = el.querySelector('annotation[encoding="application/x-tex"]'); + if (!mathAnnotation) { + return false; + } + + return "```math\n" + CopyAsGFM.nodeToGFM(mathAnnotation) + "\n```"; + }, + "span.katex-mathml": function(el, text) { + var mathAnnotation = el.querySelector('annotation[encoding="application/x-tex"]'); + if (!mathAnnotation) { + return false; + } + + return "$`" + CopyAsGFM.nodeToGFM(mathAnnotation) + "`$"; + }, + "span.katex-html": function(el, text) { + return ""; + }, + 'annotation[encoding="application/x-tex"]': function(el, text) { + return text.trim(); + } + }, + "SyntaxHighlightFilter": { + "pre.code.highlight": function(el, text) { + var lang = el.getAttribute("lang"); + if (lang == "text") { + lang = ""; + } + return "```" + lang + "\n" + text.trim() + "\n```"; + }, + "pre > code": function(el, text) { + // Don't wrap code blocks in `` + return text; + }, + }, + "MarkdownFilter": { + "code": function(el, text) { + var backtickCount = 1; + var backtickMatch = text.match(/`+/); + if (backtickMatch) { + backtickCount = backtickMatch[0].length + 1; + } + + var backticks = new Array(backtickCount + 1).join('`'); + var spaceOrNoSpace = backtickCount > 1 ? " " : ""; + + return backticks + spaceOrNoSpace + text + spaceOrNoSpace + backticks; + }, + "blockquote": function(el, text) { + return text.trim().split('\n').map(function(s) { return ('> ' + s).trim(); }).join('\n'); + }, + "img": function(el, text) { + return "![" + el.getAttribute("alt") + "](" + el.getAttribute("src") + ")"; + }, + "a.anchor": function(el, text) { + return text; + }, + "a": function(el, text) { + return "[" + text + "](" + el.getAttribute("href") + ")"; + }, + "li": function(el, text) { + var lines = text.trim().split('\n'); + var firstLine = '- ' + lines.shift(); + var nextLines = lines.map(function(s) { return (' ' + s).replace(/\s+$/, ''); }); + + return firstLine + '\n' + nextLines.join('\n'); + }, + "ul": function(el, text) { + return text; + }, + "ol": function(el, text) { + return text.replace(/^- /mg, '1. '); + }, + "h1": function(el, text) { + return '# ' + text.trim(); + }, + "h2": function(el, text) { + return '## ' + text.trim(); + }, + "h3": function(el, text) { + return '### ' + text.trim(); + }, + "h4": function(el, text) { + return '#### ' + text.trim(); + }, + "h5": function(el, text) { + return '##### ' + text.trim(); + }, + "h6": function(el, text) { + return '###### ' + text.trim(); + }, + "strong": function(el, text) { + return '**' + text + '**'; + }, + "em": function(el, text) { + return '_' + text + '_'; + }, + "del": function(el, text) { + return '~~' + text + '~~'; + }, + "sup": function(el, text) { + return '^' + text; + }, + "hr": function(el, text) { + return '-----'; + }, + "table": function(el, text) { + var theadText = CopyAsGFM.nodeToGFM(el.querySelector('thead')); + var tbodyText = CopyAsGFM.nodeToGFM(el.querySelector('tbody')); + + return theadText + tbodyText; + }, + "thead": function(el, text) { + var cells = _.map(el.querySelectorAll('th'), function(cell) { + var chars = CopyAsGFM.nodeToGFM(cell).trim().length; + + var before = ''; + var after = ''; + switch (cell.style.textAlign) { + case 'center': + before = ':'; + after = ':'; + chars -= 2; + break; + case 'right': + after = ':'; + chars -= 1; + break; + } + + chars = Math.max(chars, 0); + + var middle = new Array(chars + 1).join('-'); + + return before + middle + after; + }); + return text + '| ' + cells.join(' | ') + ' |'; + }, + "tr": function(el, text) { + var cells = _.map(el.querySelectorAll('td, th'), function(cell) { + return CopyAsGFM.nodeToGFM(cell).trim(); + }); + return '| ' + cells.join(' | ') + ' |'; + }, + } + }; + + class CopyAsGFM { + constructor() { + $(document).on('copy', '.md, .wiki', this.handleCopy.bind(this)); + $(document).on('paste', '.js-gfm-input', this.handlePaste.bind(this)); + } + + handleCopy(e) { + var clipboardData = e.originalEvent.clipboardData; + if (!clipboardData) return; + + if (!window.getSelection) return; + + var selection = window.getSelection(); + if (!selection.rangeCount || selection.rangeCount === 0) return; + + var selectedDocument = selection.getRangeAt(0).cloneContents(); + if (!selectedDocument) return; + + e.preventDefault(); + clipboardData.setData('text/plain', selectedDocument.textContent); + + var gfm = CopyAsGFM.nodeToGFM(selectedDocument); + clipboardData.setData('text/x-gfm', gfm); + } + + handlePaste(e) { + var clipboardData = e.originalEvent.clipboardData; + if (!clipboardData) return; + + var gfm = clipboardData.getData('text/x-gfm'); + if (!gfm) return; + + e.preventDefault(); + + this.insertText(e.target, gfm); + } + + insertText(target, text) { + // Firefox doesn't support `document.execCommand('insertText', false, text);` on textareas + var selectionStart = target.selectionStart; + var selectionEnd = target.selectionEnd; + var value = target.value; + var textBefore = value.substring(0, selectionStart); + var textAfter = value.substring(selectionEnd, value.length); + var newText = textBefore + text + textAfter; + target.value = newText; + target.selectionStart = target.selectionEnd = selectionStart + text.length; + } + + static nodeToGFM(node) { + if (node.nodeType == Node.TEXT_NODE) { + return node.textContent; + } + + var text = this.innerGFM(node); + + if (node.nodeType == Node.DOCUMENT_FRAGMENT_NODE) { + return text; + } + + for (var filter in gfmRules) { + var rules = gfmRules[filter]; + + for (var selector in rules) { + var func = rules[selector]; + + if (!node.matches(selector)) continue; + + var result = func(node, text); + if (result === false) continue; + + return result; + } + } + + return text; + } + + static innerGFM(parentNode) { + var nodes = parentNode.childNodes; + + var clonedParentNode = parentNode.cloneNode(true); + var clonedNodes = Array.prototype.slice.call(clonedParentNode.childNodes, 0); + + for (var i = 0; i < nodes.length; i++) { + var node = nodes[i]; + var clonedNode = clonedNodes[i]; + + var text = this.nodeToGFM(node); + clonedNode.parentNode.replaceChild(document.createTextNode(text), clonedNode); + } + + return clonedParentNode.innerText || clonedParentNode.textContent; + } + } + + window.gl = window.gl || {}; + window.gl.CopyAsGFM = CopyAsGFM; + + new CopyAsGFM(); +})(); diff --git a/app/assets/javascripts/shortcuts_issuable.js b/app/assets/javascripts/shortcuts_issuable.js index b892fbc3393..426821873d7 100644 --- a/app/assets/javascripts/shortcuts_issuable.js +++ b/app/assets/javascripts/shortcuts_issuable.js @@ -39,29 +39,33 @@ } ShortcutsIssuable.prototype.replyWithSelectedText = function() { - var quote, replyField, selected, separator; - if (window.getSelection) { - selected = window.getSelection().toString(); - replyField = $('.js-main-target-form #note_note'); - if (selected.trim() === "") { - return; - } - // Put a '>' character before each non-empty line in the selection - quote = _.map(selected.split("\n"), function(val) { - if (val.trim() !== '') { - return "> " + val + "\n"; - } - }); - // If replyField already has some content, add a newline before our quote - separator = replyField.val().trim() !== "" && "\n" || ''; - replyField.val(function(_, current) { - return current + separator + quote.join('') + "\n"; - }); - // Trigger autosave for the added text - replyField.trigger('input'); - // Focus the input field - return replyField.focus(); + var quote, replyField, selectedDocument, selected, selection, separator; + if (!window.getSelection) return; + + selection = window.getSelection(); + if (!selection.rangeCount || selection.rangeCount === 0) return; + + selectedDocument = selection.getRangeAt(0).cloneContents(); + if (!selectedDocument) return; + + selected = window.gl.CopyAsGFM.nodeToGFM(selectedDocument); + + replyField = $('.js-main-target-form #note_note'); + if (selected.trim() === "") { + return; } + quote = _.map(selected.split("\n"), function(val) { + return "> " + val + "\n"; + }); + // If replyField already has some content, add a newline before our quote + separator = replyField.val().trim() !== "" && "\n" || ''; + replyField.val(function(_, current) { + return current + separator + quote.join('') + "\n"; + }); + // Trigger autosave for the added text + replyField.trigger('input'); + // Focus the input field + return replyField.focus(); }; ShortcutsIssuable.prototype.editIssue = function() { diff --git a/changelogs/unreleased/copy-as-md.yml b/changelogs/unreleased/copy-as-md.yml new file mode 100644 index 00000000000..637e9dc36e2 --- /dev/null +++ b/changelogs/unreleased/copy-as-md.yml @@ -0,0 +1,4 @@ +--- +title: Copying a rendered issue/comment will paste into GFM textareas as actual GFM +merge_request: +author: diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index 6d04f68c8f9..a3d495a5da0 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -153,7 +153,7 @@ module Banzai title = object_link_title(object) klass = reference_class(object_sym) - data = data_attributes_for(link_content || match, project, object) + data = data_attributes_for(link_content || match, project, object, link: !!link_content) if matches.names.include?("url") && matches[:url] url = matches[:url] @@ -172,9 +172,10 @@ module Banzai end end - def data_attributes_for(text, project, object) + def data_attributes_for(text, project, object, link: false) data_attribute( original: text, + link: link, project: project.id, object_sym => object.id ) diff --git a/lib/banzai/filter/issue_reference_filter.rb b/lib/banzai/filter/issue_reference_filter.rb index 4d1bc687696..fd6b9704132 100644 --- a/lib/banzai/filter/issue_reference_filter.rb +++ b/lib/banzai/filter/issue_reference_filter.rb @@ -62,7 +62,7 @@ module Banzai end end - def data_attributes_for(text, project, object) + def data_attributes_for(text, project, object, link: false) if object.is_a?(ExternalIssue) data_attribute( project: project.id, diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb index 026b81ac175..933103abb92 100644 --- a/lib/banzai/filter/syntax_highlight_filter.rb +++ b/lib/banzai/filter/syntax_highlight_filter.rb @@ -20,17 +20,18 @@ module Banzai code = node.text css_classes = "code highlight" lexer = lexer_for(language) + lang = lexer.tag begin code = format(lex(lexer, code)) - css_classes << " js-syntax-highlight #{lexer.tag}" + css_classes << " js-syntax-highlight #{lang}" rescue # Gracefully handle syntax highlighter bugs/errors to ensure # users can still access an issue/comment/etc. end - highlighted = %(
#{code}
) + highlighted = %(
#{code}
) # Extracted to a method to measure it replace_parent_pre_element(node, highlighted) diff --git a/lib/banzai/filter/video_link_filter.rb b/lib/banzai/filter/video_link_filter.rb index ac7bbcb0d10..b64a1287d4d 100644 --- a/lib/banzai/filter/video_link_filter.rb +++ b/lib/banzai/filter/video_link_filter.rb @@ -35,7 +35,8 @@ module Banzai src: element['src'], width: '400', controls: true, - 'data-setup' => '{}') + 'data-setup' => '{}', + 'data-title' => element['title'] || element['alt']) link = doc.document.create_element( 'a', diff --git a/lib/banzai/pipeline/gfm_pipeline.rb b/lib/banzai/pipeline/gfm_pipeline.rb index 5a1f873496c..09038d38b1f 100644 --- a/lib/banzai/pipeline/gfm_pipeline.rb +++ b/lib/banzai/pipeline/gfm_pipeline.rb @@ -1,6 +1,9 @@ module Banzai module Pipeline class GfmPipeline < BasePipeline + # Every filter should have an entry in app/assets/javascripts/copy_as_gfm.js.es6, + # in reverse order. + # Should have test coverage in spec/features/copy_as_gfm_spec.rb. def self.filters @filters ||= FilterArray[ Filter::SyntaxHighlightFilter, diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/copy_as_gfm_spec.rb new file mode 100644 index 00000000000..8e8c0ecb282 --- /dev/null +++ b/spec/features/copy_as_gfm_spec.rb @@ -0,0 +1,234 @@ +require 'spec_helper' + +describe 'Copy as GFM', feature: true, js: true do + include GitlabMarkdownHelper + include ActionView::Helpers::JavaScriptHelper + + before do + @feat = MarkdownFeature.new + + # `markdown` helper expects a `@project` variable + @project = @feat.project + + visit namespace_project_issue_path(@project.namespace, @project, @feat.issue) + end + + # Should have an entry for every filter in lib/banzai/pipeline/gfm_pipeline.rb + # and app/assets/javascripts/copy_as_gfm.js.es6 + filters = { + 'any filter' => [ + [ + 'crazy nesting', + '> 1. [x] **[$`2 + 2`$ {-=-}{+=+} 2^2 ~~:thumbsup:~~](http://google.com)**' + ], + [ + 'real world example from the gitlab-ce README', + <<-GFM.strip_heredoc + # GitLab + + [![Build status](https://gitlab.com/gitlab-org/gitlab-ce/badges/master/build.svg)](https://gitlab.com/gitlab-org/gitlab-ce/commits/master) + [![CE coverage report](https://gitlab.com/gitlab-org/gitlab-ce/badges/master/coverage.svg?job=coverage)](https://gitlab-org.gitlab.io/gitlab-ce/coverage-ruby) + [![Code Climate](https://codeclimate.com/github/gitlabhq/gitlabhq.svg)](https://codeclimate.com/github/gitlabhq/gitlabhq) + [![Core Infrastructure Initiative Best Practices](https://bestpractices.coreinfrastructure.org/projects/42/badge)](https://bestpractices.coreinfrastructure.org/projects/42) + + ## Canonical source + + The canonical source of GitLab Community Edition is [hosted on GitLab.com](https://gitlab.com/gitlab-org/gitlab-ce/). + + ## Open source software to collaborate on code + + To see how GitLab looks please see the [features page on our website](https://about.gitlab.com/features/). + + + - Manage Git repositories with fine grained access controls that keep your code secure + + - Perform code reviews and enhance collaboration with merge requests + + - Complete continuous integration (CI) and CD pipelines to builds, test, and deploy your applications + + - Each project can also have an issue tracker, issue board, and a wiki + + - Used by more than 100,000 organizations, GitLab is the most popular solution to manage Git repositories on-premises + + - Completely free and open source (MIT Expat license) + GFM + ] + ], + 'InlineDiffFilter' => [ + '{-Deleted text-}', + '{+Added text+}' + ], + 'TaskListFilter' => [ + '- [ ] Unchecked task', + '- [x] Checked task', + '1. [ ] Unchecked numbered task', + '1. [x] Checked numbered task' + ], + 'ReferenceFilter' => [ + ['issue reference', -> { @feat.issue.to_reference }], + ['full issue reference', -> { @feat.issue.to_reference(full: true) }], + ['issue URL', -> { namespace_project_issue_url(@project.namespace, @project, @feat.issue) }], + ['issue URL with note anchor', -> { namespace_project_issue_url(@project.namespace, @project, @feat.issue, anchor: 'note_123') }], + ['issue link', -> { "[Issue](#{namespace_project_issue_url(@project.namespace, @project, @feat.issue)})" }], + ['issue link with note anchor', -> { "[Issue](#{namespace_project_issue_url(@project.namespace, @project, @feat.issue, anchor: 'note_123')})" }], + ], + 'AutolinkFilter' => [ + 'https://example.com' + ], + 'TableOfContentsFilter' => [ + '[[_TOC_]]' + ], + 'EmojiFilter' => [ + ':thumbsup:' + ], + 'ImageLinkFilter' => [ + '![Image](https://example.com/image.png)' + ], + 'VideoLinkFilter' => [ + '![Video](https://example.com/video.mp4)' + ], + 'MathFilter' => [ + '$`c = \pm\sqrt{a^2 + b^2}`$', + [ + 'math block', + <<-GFM.strip_heredoc + ```math + c = \pm\sqrt{a^2 + b^2} + ``` + GFM + ] + ], + 'SyntaxHighlightFilter' => [ + [ + 'code block', + <<-GFM.strip_heredoc + ```ruby + def foo + bar + end + ``` + GFM + ] + ], + 'MarkdownFilter' => [ + '`code`', + '`` code with ` ticks ``', + + '> Quote', + [ + 'multiline quote', + <<-GFM.strip_heredoc, + > Multiline + > Quote + > + > With multiple paragraphs + GFM + ], + + '![Image](https://example.com/image.png)', + + '# Heading with no anchor link', + + '[Link](https://example.com)', + + '- List item', + [ + 'multiline list item', + <<-GFM.strip_heredoc, + - Multiline + List item + GFM + ], + [ + 'nested lists', + <<-GFM.strip_heredoc, + - Nested + + + - Lists + GFM + ], + '1. Numbered list item', + [ + 'multiline numbered list item', + <<-GFM.strip_heredoc, + 1. Multiline + Numbered list item + GFM + ], + [ + 'nested numbered list', + <<-GFM.strip_heredoc, + 1. Nested + + + 1. Numbered lists + GFM + ], + + '# Heading', + '## Heading', + '### Heading', + '#### Heading', + '##### Heading', + '###### Heading', + + '**Bold**', + + '_Italics_', + + '~~Strikethrough~~', + + '2^2', + + '-----', + + [ + 'table', + <<-GFM.strip_heredoc, + | Centered | Right | Left | + | :------: | ----: | ---- | + | Foo | Bar | **Baz** | + | Foo | Bar | **Baz** | + GFM + ] + ] + } + + filters.each do |filter, examples| + context filter do + examples.each do |ex| + if ex.is_a?(String) + desc = "'#{ex}'" + gfm = ex + else + desc, gfm = ex + end + + it "transforms #{desc} to HTML and back to GFM" do + gfm = instance_exec(&gfm) if gfm.is_a?(Proc) + + html = markdown(gfm) + gfm2 = html_to_gfm(html) + expect(gfm2.strip).to eq(gfm.strip) + end + end + end + end + + def html_to_gfm(html) + js = <<-JS.strip_heredoc + (function(html) { + var node = document.createElement('div'); + node.innerHTML = html; + return window.gl.CopyAsGFM.nodeToGFM(node); + })("#{escape_javascript(html)}") + JS + page.evaluate_script(js) + end + + # Fake a `current_user` helper + def current_user + @feat.user + end +end From 37cabebe1661ab09eec6dfb74b7b09bc03e5f454 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 16 Jan 2017 16:44:46 -0500 Subject: [PATCH 02/20] Address style feedback --- app/assets/javascripts/copy_as_gfm.js.es6 | 291 +++++++++++----------- 1 file changed, 149 insertions(+), 142 deletions(-) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index 4827f7b7420..f9098c7041c 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -1,192 +1,195 @@ /* eslint-disable class-methods-use-this */ +/*jshint esversion: 6 */ (() => { const gfmRules = { // Should have an entry for every filter in lib/banzai/pipeline/gfm_pipeline.rb, // in reverse order. // Should have test coverage in spec/features/copy_as_gfm_spec.rb. - "InlineDiffFilter": { - "span.idiff.addition": function(el, text) { - return "{+" + text + "+}"; + InlineDiffFilter: { + 'span.idiff.addition'(el, text) { + return `{+${text}+}`; }, - "span.idiff.deletion": function(el, text) { - return "{-" + text + "-}"; + 'span.idiff.deletion'(el, text) { + return `{-${text}-}`; }, }, - "TaskListFilter": { - "input[type=checkbox].task-list-item-checkbox": function(el, text) { - return '[' + (el.checked ? 'x' : ' ') + ']'; + TaskListFilter: { + 'input[type=checkbox].task-list-item-checkbox'(el, text) { + return `[${el.checked ? 'x' : ' '}]`; } }, - "ReferenceFilter": { - "a.gfm:not([data-link=true])": function(el, text) { - return el.getAttribute('data-original') || text; + ReferenceFilter: { + 'a.gfm:not([data-link=true])'(el, text) { + return el.dataset.original || text; }, }, - "AutolinkFilter": { - "a": function(el, text) { - if (text != el.getAttribute("href")) { - // Fall back to handler for MarkdownFilter - return false; - } + AutolinkFilter: { + 'a'(el, text) { + // Fallback on the regular MarkdownFilter's `a` handler. + if (text !== el.getAttribute('href')) return false; return text; }, }, - "TableOfContentsFilter": { - "ul.section-nav": function(el, text) { - return "[[_TOC_]]"; + TableOfContentsFilter: { + 'ul.section-nav'(el, text) { + return '[[_TOC_]]'; }, }, - "EmojiFilter": { - "img.emoji": function(el, text) { - return el.getAttribute("alt"); + EmojiFilter: { + 'img.emoji'(el, text) { + return el.getAttribute('alt'); }, }, - "ImageLinkFilter": { - "a.no-attachment-icon": function(el, text) { + ImageLinkFilter: { + 'a.no-attachment-icon'(el, text) { return text; }, }, - "VideoLinkFilter": { - ".video-container": function(el, text) { - var videoEl = el.querySelector('video'); - if (!videoEl) { - return false; - } + VideoLinkFilter: { + '.video-container'(el, text) { + let videoEl = el.querySelector('video'); + if (!videoEl) return false; return CopyAsGFM.nodeToGFM(videoEl); }, - "video": function(el, text) { - return "![" + el.getAttribute('data-title') + "](" + el.getAttribute("src") + ")"; + 'video'(el, text) { + return `![${el.dataset.title}](${el.getAttribute('src')})`; }, }, - "MathFilter": { - "pre.code.math[data-math-style='display']": function(el, text) { - return "```math\n" + text.trim() + "\n```"; + MathFilter: { + 'pre.code.math[data-math-style=display]'(el, text) { + return '```math\n' + text.trim() + '\n```'; }, - "code.code.math[data-math-style='inline']": function(el, text) { - return "$`" + text + "`$"; + 'code.code.math[data-math-style=inline]'(el, text) { + return '$`' + text + '`$'; }, - "span.katex-display span.katex-mathml": function(el, text) { - var mathAnnotation = el.querySelector('annotation[encoding="application/x-tex"]'); - if (!mathAnnotation) { - return false; - } + 'span.katex-display span.katex-mathml'(el, text) { + let mathAnnotation = el.querySelector('annotation[encoding="application/x-tex"]'); + if (!mathAnnotation) return false; - return "```math\n" + CopyAsGFM.nodeToGFM(mathAnnotation) + "\n```"; + return '```math\n' + CopyAsGFM.nodeToGFM(mathAnnotation) + '\n```'; }, - "span.katex-mathml": function(el, text) { - var mathAnnotation = el.querySelector('annotation[encoding="application/x-tex"]'); - if (!mathAnnotation) { - return false; - } + 'span.katex-mathml'(el, text) { + let mathAnnotation = el.querySelector('annotation[encoding="application/x-tex"]'); + if (!mathAnnotation) return false; - return "$`" + CopyAsGFM.nodeToGFM(mathAnnotation) + "`$"; + return '$`' + CopyAsGFM.nodeToGFM(mathAnnotation) + '`$'; }, - "span.katex-html": function(el, text) { - return ""; + 'span.katex-html'(el, text) { + // We don't want to include the content of this element in the copied text. + return ''; }, - 'annotation[encoding="application/x-tex"]': function(el, text) { + 'annotation[encoding="application/x-tex"]'(el, text) { return text.trim(); } }, - "SyntaxHighlightFilter": { - "pre.code.highlight": function(el, text) { - var lang = el.getAttribute("lang"); - if (lang == "text") { - lang = ""; + SyntaxHighlightFilter: { + 'pre.code.highlight'(el, text) { + let lang = el.getAttribute('lang'); + if (lang === 'text') { + lang = ''; } - return "```" + lang + "\n" + text.trim() + "\n```"; + return '```' + lang + '\n' + text.trim() + '\n```'; }, - "pre > code": function(el, text) { + 'pre > code'(el, text) { // Don't wrap code blocks in `` return text; }, }, - "MarkdownFilter": { - "code": function(el, text) { - var backtickCount = 1; - var backtickMatch = text.match(/`+/); + MarkdownFilter: { + 'code'(el, text) { + let backtickCount = 1; + let backtickMatch = text.match(/`+/); if (backtickMatch) { backtickCount = backtickMatch[0].length + 1; } - var backticks = new Array(backtickCount + 1).join('`'); - var spaceOrNoSpace = backtickCount > 1 ? " " : ""; + let backticks = new Array(backtickCount + 1).join('`'); + let spaceOrNoSpace = backtickCount > 1 ? ' ' : ''; return backticks + spaceOrNoSpace + text + spaceOrNoSpace + backticks; }, - "blockquote": function(el, text) { - return text.trim().split('\n').map(function(s) { return ('> ' + s).trim(); }).join('\n'); + 'blockquote'(el, text) { + return text.trim().split('\n').map((s) => (`> ${s}`).trim()).join('\n'); }, - "img": function(el, text) { - return "![" + el.getAttribute("alt") + "](" + el.getAttribute("src") + ")"; + 'img'(el, text) { + return `![${el.getAttribute('alt')}](${el.getAttribute('src')})`; }, - "a.anchor": function(el, text) { + 'a.anchor'(el, text) { + // Don't render a Markdown link for the anchor link inside a heading return text; }, - "a": function(el, text) { - return "[" + text + "](" + el.getAttribute("href") + ")"; + 'a'(el, text) { + return `[${text}](${el.getAttribute('href')})`; }, - "li": function(el, text) { - var lines = text.trim().split('\n'); - var firstLine = '- ' + lines.shift(); - var nextLines = lines.map(function(s) { return (' ' + s).replace(/\s+$/, ''); }); + 'li'(el, text) { + let lines = text.trim().split('\n'); + let firstLine = '- ' + lines.shift(); + // Add two spaces to the front of subsequent list items lines, or leave the line entirely blank. + let nextLines = lines.map(function(s) { + if (s.trim().length === 0) { + return ''; + } else { + return ` ${s}`; + } + }); - return firstLine + '\n' + nextLines.join('\n'); + return `${firstLine}\n${nextLines.join('\n')}`; }, - "ul": function(el, text) { + 'ul'(el, text) { return text; }, - "ol": function(el, text) { + 'ol'(el, text) { + // LIs get a `- ` prefix by default, which we replace by `1. ` for ordered lists. return text.replace(/^- /mg, '1. '); }, - "h1": function(el, text) { - return '# ' + text.trim(); + 'h1'(el, text) { + return `# ${text.trim()}`; }, - "h2": function(el, text) { - return '## ' + text.trim(); + 'h2'(el, text) { + return `## ${text.trim()}`; }, - "h3": function(el, text) { - return '### ' + text.trim(); + 'h3'(el, text) { + return `### ${text.trim()}`; }, - "h4": function(el, text) { - return '#### ' + text.trim(); + 'h4'(el, text) { + return `#### ${text.trim()}`; }, - "h5": function(el, text) { - return '##### ' + text.trim(); + 'h5'(el, text) { + return `##### ${text.trim()}`; }, - "h6": function(el, text) { - return '###### ' + text.trim(); + 'h6'(el, text) { + return `###### ${text.trim()}`; }, - "strong": function(el, text) { - return '**' + text + '**'; + 'strong'(el, text) { + return `**${text}**`; }, - "em": function(el, text) { - return '_' + text + '_'; + 'em'(el, text) { + return `_${text}_`; }, - "del": function(el, text) { - return '~~' + text + '~~'; + 'del'(el, text) { + return `~~${text}~~`; }, - "sup": function(el, text) { - return '^' + text; + 'sup'(el, text) { + return `^${text}`; }, - "hr": function(el, text) { + 'hr'(el, text) { return '-----'; }, - "table": function(el, text) { - var theadText = CopyAsGFM.nodeToGFM(el.querySelector('thead')); - var tbodyText = CopyAsGFM.nodeToGFM(el.querySelector('tbody')); + 'table'(el, text) { + let theadText = CopyAsGFM.nodeToGFM(el.querySelector('thead')); + let tbodyText = CopyAsGFM.nodeToGFM(el.querySelector('tbody')); return theadText + tbodyText; }, - "thead": function(el, text) { - var cells = _.map(el.querySelectorAll('th'), function(cell) { - var chars = CopyAsGFM.nodeToGFM(cell).trim().length; + 'thead'(el, text) { + let cells = _.map(el.querySelectorAll('th'), function(cell) { + let chars = CopyAsGFM.nodeToGFM(cell).trim().length; - var before = ''; - var after = ''; + let before = ''; + let after = ''; switch (cell.style.textAlign) { case 'center': before = ':'; @@ -201,17 +204,18 @@ chars = Math.max(chars, 0); - var middle = new Array(chars + 1).join('-'); + let middle = new Array(chars + 1).join('-'); return before + middle + after; }); - return text + '| ' + cells.join(' | ') + ' |'; + + return text + `| ${cells.join(' | ')} |`; }, - "tr": function(el, text) { - var cells = _.map(el.querySelectorAll('td, th'), function(cell) { + 'tr'(el, text) { + let cells = _.map(el.querySelectorAll('td, th'), function(cell) { return CopyAsGFM.nodeToGFM(cell).trim(); }); - return '| ' + cells.join(' | ') + ' |'; + return `| ${cells.join(' | ')} |`; }, } }; @@ -223,29 +227,29 @@ } handleCopy(e) { - var clipboardData = e.originalEvent.clipboardData; + let clipboardData = e.originalEvent.clipboardData; if (!clipboardData) return; if (!window.getSelection) return; - var selection = window.getSelection(); + let selection = window.getSelection(); if (!selection.rangeCount || selection.rangeCount === 0) return; - var selectedDocument = selection.getRangeAt(0).cloneContents(); + let selectedDocument = selection.getRangeAt(0).cloneContents(); if (!selectedDocument) return; e.preventDefault(); clipboardData.setData('text/plain', selectedDocument.textContent); - var gfm = CopyAsGFM.nodeToGFM(selectedDocument); + let gfm = CopyAsGFM.nodeToGFM(selectedDocument); clipboardData.setData('text/x-gfm', gfm); } handlePaste(e) { - var clipboardData = e.originalEvent.clipboardData; + let clipboardData = e.originalEvent.clipboardData; if (!clipboardData) return; - - var gfm = clipboardData.getData('text/x-gfm'); + + let gfm = clipboardData.getData('text/x-gfm'); if (!gfm) return; e.preventDefault(); @@ -255,36 +259,39 @@ insertText(target, text) { // Firefox doesn't support `document.execCommand('insertText', false, text);` on textareas - var selectionStart = target.selectionStart; - var selectionEnd = target.selectionEnd; - var value = target.value; - var textBefore = value.substring(0, selectionStart); - var textAfter = value.substring(selectionEnd, value.length); - var newText = textBefore + text + textAfter; + + let selectionStart = target.selectionStart; + let selectionEnd = target.selectionEnd; + let value = target.value; + + let textBefore = value.substring(0, selectionStart); + let textAfter = value.substring(selectionEnd, value.length); + let newText = textBefore + text + textAfter; + target.value = newText; target.selectionStart = target.selectionEnd = selectionStart + text.length; } static nodeToGFM(node) { - if (node.nodeType == Node.TEXT_NODE) { + if (node.nodeType === Node.TEXT_NODE) { return node.textContent; } - var text = this.innerGFM(node); + let text = this.innerGFM(node); - if (node.nodeType == Node.DOCUMENT_FRAGMENT_NODE) { + if (node.nodeType === Node.DOCUMENT_FRAGMENT_NODE) { return text; } - for (var filter in gfmRules) { - var rules = gfmRules[filter]; + for (let filter in gfmRules) { + let rules = gfmRules[filter]; - for (var selector in rules) { - var func = rules[selector]; + for (let selector in rules) { + let func = rules[selector]; if (!node.matches(selector)) continue; - var result = func(node, text); + let result = func(node, text); if (result === false) continue; return result; @@ -295,16 +302,16 @@ } static innerGFM(parentNode) { - var nodes = parentNode.childNodes; + let nodes = parentNode.childNodes; - var clonedParentNode = parentNode.cloneNode(true); - var clonedNodes = Array.prototype.slice.call(clonedParentNode.childNodes, 0); + let clonedParentNode = parentNode.cloneNode(true); + let clonedNodes = Array.prototype.slice.call(clonedParentNode.childNodes, 0); - for (var i = 0; i < nodes.length; i++) { - var node = nodes[i]; - var clonedNode = clonedNodes[i]; + for (let i = 0; i < nodes.length; i++) { + let node = nodes[i]; + let clonedNode = clonedNodes[i]; - var text = this.nodeToGFM(node); + let text = this.nodeToGFM(node); clonedNode.parentNode.replaceChild(document.createTextNode(text), clonedNode); } From bc3448fc48a289342948ec155fa9c7f2d49ca2bd Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 16 Jan 2017 18:11:15 -0500 Subject: [PATCH 03/20] Improve spec --- app/assets/javascripts/copy_as_gfm.js.es6 | 7 +- lib/banzai/pipeline/gfm_pipeline.rb | 7 +- spec/features/copy_as_gfm_spec.rb | 298 +++++++++++----------- 3 files changed, 159 insertions(+), 153 deletions(-) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index f9098c7041c..7c666697cdf 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -3,9 +3,10 @@ (() => { const gfmRules = { - // Should have an entry for every filter in lib/banzai/pipeline/gfm_pipeline.rb, - // in reverse order. - // Should have test coverage in spec/features/copy_as_gfm_spec.rb. + // The filters referenced in lib/banzai/pipeline/gfm_pipeline.rb convert GitLab Flavored Markdown (GFM) to HTML. + // These handlers consequently convert that same HTML to GFM to be copied to the clipboard. + // Every filter in lib/banzai/pipeline/gfm_pipeline.rb that generates HTML from GFM should have a handler here, in reverse order. + // The GFM-to-HTML-to-GFM cycle is tested in spec/features/copy_as_gfm_spec.rb. InlineDiffFilter: { 'span.idiff.addition'(el, text) { return `{+${text}+}`; diff --git a/lib/banzai/pipeline/gfm_pipeline.rb b/lib/banzai/pipeline/gfm_pipeline.rb index 09038d38b1f..7b652aa79ec 100644 --- a/lib/banzai/pipeline/gfm_pipeline.rb +++ b/lib/banzai/pipeline/gfm_pipeline.rb @@ -1,9 +1,10 @@ module Banzai module Pipeline class GfmPipeline < BasePipeline - # Every filter should have an entry in app/assets/javascripts/copy_as_gfm.js.es6, - # in reverse order. - # Should have test coverage in spec/features/copy_as_gfm_spec.rb. + # These filters convert GitLab Flavored Markdown (GFM) to HTML. + # The handlers defined in app/assets/javascripts/copy_as_gfm.js.es6 consequently convert that same HTML to GFM to be copied to the clipboard. + # Every filter that generates HTML from GFM should have a handler in app/assets/javascripts/copy_as_gfm.js.es6, in reverse order. + # The GFM-to-HTML-to-GFM cycle is tested in spec/features/copy_as_gfm_spec.rb. def self.filters @filters ||= FilterArray[ Filter::SyntaxHighlightFilter, diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/copy_as_gfm_spec.rb index 8e8c0ecb282..dd22f92c5c8 100644 --- a/spec/features/copy_as_gfm_spec.rb +++ b/spec/features/copy_as_gfm_spec.rb @@ -13,117 +13,135 @@ describe 'Copy as GFM', feature: true, js: true do visit namespace_project_issue_path(@project.namespace, @project, @feat.issue) end - # Should have an entry for every filter in lib/banzai/pipeline/gfm_pipeline.rb - # and app/assets/javascripts/copy_as_gfm.js.es6 - filters = { - 'any filter' => [ - [ - 'crazy nesting', - '> 1. [x] **[$`2 + 2`$ {-=-}{+=+} 2^2 ~~:thumbsup:~~](http://google.com)**' - ], - [ - 'real world example from the gitlab-ce README', - <<-GFM.strip_heredoc - # GitLab + # The filters referenced in lib/banzai/pipeline/gfm_pipeline.rb convert GitLab Flavored Markdown (GFM) to HTML. + # The handlers defined in app/assets/javascripts/copy_as_gfm.js.es6 consequently convert that same HTML to GFM. + # To make sure these filters and handlers are properly aligned, this spec tests the GFM-to-HTML-to-GFM cycle + # by verifying (`html_to_gfm(gfm_to_html(gfm)) == gfm`) for a number of examples of GFM for every filter. - [![Build status](https://gitlab.com/gitlab-org/gitlab-ce/badges/master/build.svg)](https://gitlab.com/gitlab-org/gitlab-ce/commits/master) - [![CE coverage report](https://gitlab.com/gitlab-org/gitlab-ce/badges/master/coverage.svg?job=coverage)](https://gitlab-org.gitlab.io/gitlab-ce/coverage-ruby) - [![Code Climate](https://codeclimate.com/github/gitlabhq/gitlabhq.svg)](https://codeclimate.com/github/gitlabhq/gitlabhq) - [![Core Infrastructure Initiative Best Practices](https://bestpractices.coreinfrastructure.org/projects/42/badge)](https://bestpractices.coreinfrastructure.org/projects/42) + it 'supports nesting' do + verify '> 1. [x] **[$`2 + 2`$ {-=-}{+=+} 2^2 ~~:thumbsup:~~](http://google.com)**' + end - ## Canonical source + it 'supports a real world example from the gitlab-ce README' do + verify <<-GFM.strip_heredoc + # GitLab - The canonical source of GitLab Community Edition is [hosted on GitLab.com](https://gitlab.com/gitlab-org/gitlab-ce/). + [![Build status](https://gitlab.com/gitlab-org/gitlab-ce/badges/master/build.svg)](https://gitlab.com/gitlab-org/gitlab-ce/commits/master) + [![CE coverage report](https://gitlab.com/gitlab-org/gitlab-ce/badges/master/coverage.svg?job=coverage)](https://gitlab-org.gitlab.io/gitlab-ce/coverage-ruby) + [![Code Climate](https://codeclimate.com/github/gitlabhq/gitlabhq.svg)](https://codeclimate.com/github/gitlabhq/gitlabhq) + [![Core Infrastructure Initiative Best Practices](https://bestpractices.coreinfrastructure.org/projects/42/badge)](https://bestpractices.coreinfrastructure.org/projects/42) - ## Open source software to collaborate on code + ## Canonical source - To see how GitLab looks please see the [features page on our website](https://about.gitlab.com/features/). + The canonical source of GitLab Community Edition is [hosted on GitLab.com](https://gitlab.com/gitlab-org/gitlab-ce/). + + ## Open source software to collaborate on code + + To see how GitLab looks please see the [features page on our website](https://about.gitlab.com/features/). - - Manage Git repositories with fine grained access controls that keep your code secure + - Manage Git repositories with fine grained access controls that keep your code secure - - Perform code reviews and enhance collaboration with merge requests + - Perform code reviews and enhance collaboration with merge requests - - Complete continuous integration (CI) and CD pipelines to builds, test, and deploy your applications + - Complete continuous integration (CI) and CD pipelines to builds, test, and deploy your applications - - Each project can also have an issue tracker, issue board, and a wiki + - Each project can also have an issue tracker, issue board, and a wiki - - Used by more than 100,000 organizations, GitLab is the most popular solution to manage Git repositories on-premises + - Used by more than 100,000 organizations, GitLab is the most popular solution to manage Git repositories on-premises - - Completely free and open source (MIT Expat license) - GFM - ] - ], - 'InlineDiffFilter' => [ + - Completely free and open source (MIT Expat license) + GFM + end + + it 'supports InlineDiffFilter' do + verify( '{-Deleted text-}', '{+Added text+}' - ], - 'TaskListFilter' => [ + ) + end + + it 'supports TaskListFilter' do + verify( '- [ ] Unchecked task', '- [x] Checked task', '1. [ ] Unchecked numbered task', '1. [x] Checked numbered task' - ], - 'ReferenceFilter' => [ - ['issue reference', -> { @feat.issue.to_reference }], - ['full issue reference', -> { @feat.issue.to_reference(full: true) }], - ['issue URL', -> { namespace_project_issue_url(@project.namespace, @project, @feat.issue) }], - ['issue URL with note anchor', -> { namespace_project_issue_url(@project.namespace, @project, @feat.issue, anchor: 'note_123') }], - ['issue link', -> { "[Issue](#{namespace_project_issue_url(@project.namespace, @project, @feat.issue)})" }], - ['issue link with note anchor', -> { "[Issue](#{namespace_project_issue_url(@project.namespace, @project, @feat.issue, anchor: 'note_123')})" }], - ], - 'AutolinkFilter' => [ - 'https://example.com' - ], - 'TableOfContentsFilter' => [ - '[[_TOC_]]' - ], - 'EmojiFilter' => [ - ':thumbsup:' - ], - 'ImageLinkFilter' => [ - '![Image](https://example.com/image.png)' - ], - 'VideoLinkFilter' => [ - '![Video](https://example.com/video.mp4)' - ], - 'MathFilter' => [ + ) + end + + it 'supports ReferenceFilter' do + verify( + # issue reference + @feat.issue.to_reference, + # full issue reference + @feat.issue.to_reference(full: true), + # issue URL + namespace_project_issue_url(@project.namespace, @project, @feat.issue), + # issue URL with note anchor + namespace_project_issue_url(@project.namespace, @project, @feat.issue, anchor: 'note_123'), + # issue link + "[Issue](#{namespace_project_issue_url(@project.namespace, @project, @feat.issue)})", + # issue link with note anchor + "[Issue](#{namespace_project_issue_url(@project.namespace, @project, @feat.issue, anchor: 'note_123')})", + ) + end + + it 'supports AutolinkFilter' do + verify 'https://example.com' + end + + it 'supports TableOfContentsFilter' do + verify '[[_TOC_]]' + end + + it 'supports EmojiFilter' do + verify ':thumbsup:' + end + + it 'supports ImageLinkFilter' do + verify '![Image](https://example.com/image.png)' + end + + it 'supports VideoLinkFilter' do + verify '![Video](https://example.com/video.mp4)' + end + + it 'supports MathFilter' do + verify( '$`c = \pm\sqrt{a^2 + b^2}`$', - [ - 'math block', - <<-GFM.strip_heredoc - ```math - c = \pm\sqrt{a^2 + b^2} - ``` - GFM - ] - ], - 'SyntaxHighlightFilter' => [ - [ - 'code block', - <<-GFM.strip_heredoc - ```ruby - def foo - bar - end - ``` - GFM - ] - ], - 'MarkdownFilter' => [ + # math block + <<-GFM.strip_heredoc + ```math + c = \pm\sqrt{a^2 + b^2} + ``` + GFM + ) + end + + it 'supports SyntaxHighlightFilter' do + verify <<-GFM.strip_heredoc + ```ruby + def foo + bar + end + ``` + GFM + end + + it 'supports MarkdownFilter' do + verify( '`code`', '`` code with ` ticks ``', '> Quote', - [ - 'multiline quote', - <<-GFM.strip_heredoc, - > Multiline - > Quote - > - > With multiple paragraphs - GFM - ], + # multiline quote + <<-GFM.strip_heredoc, + > Multiline + > Quote + > + > With multiple paragraphs + GFM '![Image](https://example.com/image.png)', @@ -132,39 +150,36 @@ describe 'Copy as GFM', feature: true, js: true do '[Link](https://example.com)', '- List item', - [ - 'multiline list item', - <<-GFM.strip_heredoc, - - Multiline - List item - GFM - ], - [ - 'nested lists', - <<-GFM.strip_heredoc, - - Nested + + # multiline list item + <<-GFM.strip_heredoc, + - Multiline + List item + GFM + + # nested lists + <<-GFM.strip_heredoc, + - Nested - - Lists - GFM - ], + - Lists + GFM + '1. Numbered list item', - [ - 'multiline numbered list item', - <<-GFM.strip_heredoc, - 1. Multiline - Numbered list item - GFM - ], - [ - 'nested numbered list', - <<-GFM.strip_heredoc, - 1. Nested + + # multiline numbered list item + <<-GFM.strip_heredoc, + 1. Multiline + Numbered list item + GFM + + # nested numbered list + <<-GFM.strip_heredoc, + 1. Nested - 1. Numbered lists - GFM - ], + 1. Numbered lists + GFM '# Heading', '## Heading', @@ -183,39 +198,18 @@ describe 'Copy as GFM', feature: true, js: true do '-----', - [ - 'table', - <<-GFM.strip_heredoc, - | Centered | Right | Left | - | :------: | ----: | ---- | - | Foo | Bar | **Baz** | - | Foo | Bar | **Baz** | - GFM - ] - ] - } - - filters.each do |filter, examples| - context filter do - examples.each do |ex| - if ex.is_a?(String) - desc = "'#{ex}'" - gfm = ex - else - desc, gfm = ex - end - - it "transforms #{desc} to HTML and back to GFM" do - gfm = instance_exec(&gfm) if gfm.is_a?(Proc) - - html = markdown(gfm) - gfm2 = html_to_gfm(html) - expect(gfm2.strip).to eq(gfm.strip) - end - end - end + # table + <<-GFM.strip_heredoc, + | Centered | Right | Left | + | :------: | ----: | ---- | + | Foo | Bar | **Baz** | + | Foo | Bar | **Baz** | + GFM + ) end + alias_method :gfm_to_html, :markdown + def html_to_gfm(html) js = <<-JS.strip_heredoc (function(html) { @@ -227,6 +221,16 @@ describe 'Copy as GFM', feature: true, js: true do page.evaluate_script(js) end + def verify(*gfms) + aggregate_failures do + gfms.each do |gfm| + html = gfm_to_html(gfm) + output_gfm = html_to_gfm(html) + expect(output_gfm.strip).to eq(gfm.strip) + end + end + end + # Fake a `current_user` helper def current_user @feat.user From 5adf080880521826b4e0f4c3c5d11ea15da458dd Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 16 Jan 2017 18:12:42 -0500 Subject: [PATCH 04/20] Don't copy if there's nothing to copy --- app/assets/javascripts/copy_as_gfm.js.es6 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index 7c666697cdf..38ec6e4732a 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -239,6 +239,8 @@ let selectedDocument = selection.getRangeAt(0).cloneContents(); if (!selectedDocument) return; + if (selectedDocument.textContent.length === 0) return; + e.preventDefault(); clipboardData.setData('text/plain', selectedDocument.textContent); From 72620ea1b725f8776087e516cdc3dd13f5f8e075 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 17 Jan 2017 13:06:12 -0500 Subject: [PATCH 05/20] Fix SyntaxHighlightFilter spec --- app/assets/javascripts/copy_as_gfm.js.es6 | 2 +- lib/banzai/filter/syntax_highlight_filter.rb | 1 + spec/lib/banzai/filter/syntax_highlight_filter_spec.rb | 8 ++++---- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index 38ec6e4732a..0d1e9b0a952 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -89,7 +89,7 @@ SyntaxHighlightFilter: { 'pre.code.highlight'(el, text) { let lang = el.getAttribute('lang'); - if (lang === 'text') { + if (lang === 'plaintext') { lang = ''; } return '```' + lang + '\n' + text.trim() + '\n```'; diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb index 933103abb92..a447e2b8bff 100644 --- a/lib/banzai/filter/syntax_highlight_filter.rb +++ b/lib/banzai/filter/syntax_highlight_filter.rb @@ -27,6 +27,7 @@ module Banzai css_classes << " js-syntax-highlight #{lang}" rescue + lang = nil # Gracefully handle syntax highlighter bugs/errors to ensure # users can still access an issue/comment/etc. end diff --git a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb index d265d29ee86..69e3c52b35a 100644 --- a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb +++ b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb @@ -6,21 +6,21 @@ describe Banzai::Filter::SyntaxHighlightFilter, lib: true do context "when no language is specified" do it "highlights as plaintext" do result = filter('
def fun end
') - expect(result.to_html).to eq('
def fun end
') + expect(result.to_html).to eq('
def fun end
') end end context "when a valid language is specified" do it "highlights as that language" do result = filter('
def fun end
') - expect(result.to_html).to eq('
def fun end
') + expect(result.to_html).to eq('
def fun end
') end end context "when an invalid language is specified" do it "highlights as plaintext" do result = filter('
This is a test
') - expect(result.to_html).to eq('
This is a test
') + expect(result.to_html).to eq('
This is a test
') end end @@ -31,7 +31,7 @@ describe Banzai::Filter::SyntaxHighlightFilter, lib: true do it "highlights as plaintext" do result = filter('
This is a test
') - expect(result.to_html).to eq('
This is a test
') + expect(result.to_html).to eq('
This is a test
') end end end From 6089ece09860e99528932ef574086c113ec08ae7 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 17 Jan 2017 13:07:20 -0500 Subject: [PATCH 06/20] Fix ShortcutsIssuable#replyWithSelectedText tests --- app/assets/javascripts/copy_as_gfm.js.es6 | 64 +++++++++++++++----- app/assets/javascripts/shortcuts_issuable.js | 16 ++--- spec/javascripts/shortcuts_issuable_spec.js | 21 ++++--- 3 files changed, 67 insertions(+), 34 deletions(-) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index 0d1e9b0a952..63291853548 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -231,20 +231,13 @@ let clipboardData = e.originalEvent.clipboardData; if (!clipboardData) return; - if (!window.getSelection) return; - - let selection = window.getSelection(); - if (!selection.rangeCount || selection.rangeCount === 0) return; - - let selectedDocument = selection.getRangeAt(0).cloneContents(); - if (!selectedDocument) return; - - if (selectedDocument.textContent.length === 0) return; + let documentFragment = CopyAsGFM.getSelectedFragment(); + if (!documentFragment) return; e.preventDefault(); - clipboardData.setData('text/plain', selectedDocument.textContent); + clipboardData.setData('text/plain', documentFragment.textContent); - let gfm = CopyAsGFM.nodeToGFM(selectedDocument); + let gfm = CopyAsGFM.nodeToGFM(documentFragment); clipboardData.setData('text/x-gfm', gfm); } @@ -257,11 +250,25 @@ e.preventDefault(); - this.insertText(e.target, gfm); + CopyAsGFM.insertText(e.target, gfm); } - insertText(target, text) { - // Firefox doesn't support `document.execCommand('insertText', false, text);` on textareas + static getSelectedFragment() { + if (!window.getSelection) return null; + + let selection = window.getSelection(); + if (!selection.rangeCount || selection.rangeCount === 0) return null; + + let documentFragment = selection.getRangeAt(0).cloneContents(); + if (!documentFragment) return null; + + if (documentFragment.textContent.length === 0) return null; + + return documentFragment; + } + + static insertText(target, text) { + // Firefox doesn't support `document.execCommand('insertText', false, text)` on textareas let selectionStart = target.selectionStart; let selectionEnd = target.selectionEnd; @@ -292,7 +299,7 @@ for (let selector in rules) { let func = rules[selector]; - if (!node.matches(selector)) continue; + if (!CopyAsGFM.nodeMatchesSelector(node, selector)) continue; let result = func(node, text); if (result === false) continue; @@ -315,11 +322,38 @@ let clonedNode = clonedNodes[i]; let text = this.nodeToGFM(node); + + // `clonedNode.replaceWith(text)` is not yet widely supported clonedNode.parentNode.replaceChild(document.createTextNode(text), clonedNode); } return clonedParentNode.innerText || clonedParentNode.textContent; } + + static nodeMatchesSelector(node, selector) { + let matches = Element.prototype.matches || + Element.prototype.matchesSelector || + Element.prototype.mozMatchesSelector || + Element.prototype.msMatchesSelector || + Element.prototype.oMatchesSelector || + Element.prototype.webkitMatchesSelector; + + if (matches) { + return matches.call(node, selector); + } + + // IE11 doesn't support `node.matches(selector)` + + let parentNode = node.parentNode; + if (!parentNode) { + parentNode = document.createElement('div'); + node = node.cloneNode(true); + parentNode.appendChild(node); + } + + let matchingNodes = parentNode.querySelectorAll(selector); + return Array.prototype.indexOf.call(matchingNodes, node) !== -1; + } } window.gl = window.gl || {}; diff --git a/app/assets/javascripts/shortcuts_issuable.js b/app/assets/javascripts/shortcuts_issuable.js index 426821873d7..e9ede122ab7 100644 --- a/app/assets/javascripts/shortcuts_issuable.js +++ b/app/assets/javascripts/shortcuts_issuable.js @@ -39,26 +39,22 @@ } ShortcutsIssuable.prototype.replyWithSelectedText = function() { - var quote, replyField, selectedDocument, selected, selection, separator; - if (!window.getSelection) return; + var quote, replyField, documentFragment, selected, separator; - selection = window.getSelection(); - if (!selection.rangeCount || selection.rangeCount === 0) return; + documentFragment = window.gl.CopyAsGFM.getSelectedFragment(); + if (!documentFragment) return; - selectedDocument = selection.getRangeAt(0).cloneContents(); - if (!selectedDocument) return; - - selected = window.gl.CopyAsGFM.nodeToGFM(selectedDocument); + selected = window.gl.CopyAsGFM.nodeToGFM(documentFragment); replyField = $('.js-main-target-form #note_note'); if (selected.trim() === "") { return; } quote = _.map(selected.split("\n"), function(val) { - return "> " + val + "\n"; + return ("> " + val).trim() + "\n"; }); // If replyField already has some content, add a newline before our quote - separator = replyField.val().trim() !== "" && "\n" || ''; + separator = replyField.val().trim() !== "" && "\n\n" || ''; replyField.val(function(_, current) { return current + separator + quote.join('') + "\n"; }); diff --git a/spec/javascripts/shortcuts_issuable_spec.js b/spec/javascripts/shortcuts_issuable_spec.js index ae5d639ad9c..7e5c0e2f144 100644 --- a/spec/javascripts/shortcuts_issuable_spec.js +++ b/spec/javascripts/shortcuts_issuable_spec.js @@ -1,6 +1,7 @@ /* eslint-disable space-before-function-paren, no-return-assign, no-var, quotes, padded-blocks */ /* global ShortcutsIssuable */ +/*= require copy_as_gfm */ /*= require shortcuts_issuable */ (function() { @@ -14,10 +15,12 @@ }); return describe('#replyWithSelectedText', function() { var stubSelection; - // Stub window.getSelection to return the provided String. - stubSelection = function(text) { - return window.getSelection = function() { - return text; + // Stub window.gl.CopyAsGFM.getSelectedFragment to return a node with the provided HTML. + stubSelection = function(html) { + window.gl.CopyAsGFM.getSelectedFragment = function() { + var node = document.createElement('div'); + node.innerHTML = html; + return node; }; }; beforeEach(function() { @@ -32,13 +35,13 @@ }); describe('with any selection', function() { beforeEach(function() { - return stubSelection('Selected text.'); + return stubSelection('

Selected text.

'); }); it('leaves existing input intact', function() { $(this.selector).val('This text was already here.'); expect($(this.selector).val()).toBe('This text was already here.'); this.shortcut.replyWithSelectedText(); - return expect($(this.selector).val()).toBe("This text was already here.\n> Selected text.\n\n"); + return expect($(this.selector).val()).toBe("This text was already here.\n\n> Selected text.\n\n"); }); it('triggers `input`', function() { var triggered; @@ -61,16 +64,16 @@ }); describe('with a one-line selection', function() { return it('quotes the selection', function() { - stubSelection('This text has been selected.'); + stubSelection('

This text has been selected.

'); this.shortcut.replyWithSelectedText(); return expect($(this.selector).val()).toBe("> This text has been selected.\n\n"); }); }); return describe('with a multi-line selection', function() { return it('quotes the selected lines as a group', function() { - stubSelection("Selected line one.\n\nSelected line two.\nSelected line three.\n"); + stubSelection("

Selected line one.

\n\n

Selected line two.

\n\n

Selected line three.

"); this.shortcut.replyWithSelectedText(); - return expect($(this.selector).val()).toBe("> Selected line one.\n> Selected line two.\n> Selected line three.\n\n"); + return expect($(this.selector).val()).toBe("> Selected line one.\n>\n> Selected line two.\n>\n> Selected line three.\n\n"); }); }); }); From 3c9e556b9e1da01df0cf6527cc3468f9fed9dfeb Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 17 Jan 2017 13:07:49 -0500 Subject: [PATCH 07/20] Add more SyntaxHighlightFilter and MathFilter tests --- app/assets/javascripts/copy_as_gfm.js.es6 | 8 +- lib/banzai/pipeline/gfm_pipeline.rb | 6 +- spec/features/copy_as_gfm_spec.rb | 157 +++++++++++++++++++--- 3 files changed, 150 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index 63291853548..8e7f4c54213 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -3,9 +3,11 @@ (() => { const gfmRules = { - // The filters referenced in lib/banzai/pipeline/gfm_pipeline.rb convert GitLab Flavored Markdown (GFM) to HTML. + // The filters referenced in lib/banzai/pipeline/gfm_pipeline.rb convert + // GitLab Flavored Markdown (GFM) to HTML. // These handlers consequently convert that same HTML to GFM to be copied to the clipboard. - // Every filter in lib/banzai/pipeline/gfm_pipeline.rb that generates HTML from GFM should have a handler here, in reverse order. + // Every filter in lib/banzai/pipeline/gfm_pipeline.rb that generates HTML + // from GFM should have a handler here, in reverse order. // The GFM-to-HTML-to-GFM cycle is tested in spec/features/copy_as_gfm_spec.rb. InlineDiffFilter: { 'span.idiff.addition'(el, text) { @@ -113,7 +115,7 @@ return backticks + spaceOrNoSpace + text + spaceOrNoSpace + backticks; }, 'blockquote'(el, text) { - return text.trim().split('\n').map((s) => (`> ${s}`).trim()).join('\n'); + return text.trim().split('\n').map((s) => `> ${s}`.trim()).join('\n'); }, 'img'(el, text) { return `![${el.getAttribute('alt')}](${el.getAttribute('src')})`; diff --git a/lib/banzai/pipeline/gfm_pipeline.rb b/lib/banzai/pipeline/gfm_pipeline.rb index 7b652aa79ec..ac95a79009b 100644 --- a/lib/banzai/pipeline/gfm_pipeline.rb +++ b/lib/banzai/pipeline/gfm_pipeline.rb @@ -2,8 +2,10 @@ module Banzai module Pipeline class GfmPipeline < BasePipeline # These filters convert GitLab Flavored Markdown (GFM) to HTML. - # The handlers defined in app/assets/javascripts/copy_as_gfm.js.es6 consequently convert that same HTML to GFM to be copied to the clipboard. - # Every filter that generates HTML from GFM should have a handler in app/assets/javascripts/copy_as_gfm.js.es6, in reverse order. + # The handlers defined in app/assets/javascripts/copy_as_gfm.js.es6 + # consequently convert that same HTML to GFM to be copied to the clipboard. + # Every filter that generates HTML from GFM should have a handler in + # app/assets/javascripts/copy_as_gfm.js.es6, in reverse order. # The GFM-to-HTML-to-GFM cycle is tested in spec/features/copy_as_gfm_spec.rb. def self.filters @filters ||= FilterArray[ diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/copy_as_gfm_spec.rb index dd22f92c5c8..cedddadb05c 100644 --- a/spec/features/copy_as_gfm_spec.rb +++ b/spec/features/copy_as_gfm_spec.rb @@ -16,7 +16,7 @@ describe 'Copy as GFM', feature: true, js: true do # The filters referenced in lib/banzai/pipeline/gfm_pipeline.rb convert GitLab Flavored Markdown (GFM) to HTML. # The handlers defined in app/assets/javascripts/copy_as_gfm.js.es6 consequently convert that same HTML to GFM. # To make sure these filters and handlers are properly aligned, this spec tests the GFM-to-HTML-to-GFM cycle - # by verifying (`html_to_gfm(gfm_to_html(gfm)) == gfm`) for a number of examples of GFM for every filter. + # by verifying (`html_to_gfm(gfm_to_html(gfm)) == gfm`) for a number of examples of GFM for every filter, using the `verify` helper. it 'supports nesting' do verify '> 1. [x] **[$`2 + 2`$ {-=-}{+=+} 2^2 ~~:thumbsup:~~](http://google.com)**' @@ -107,34 +107,159 @@ describe 'Copy as GFM', feature: true, js: true do verify '![Video](https://example.com/video.mp4)' end - it 'supports MathFilter' do + context 'MathFilter' do + it 'supports math as converted from GFM to HTML' do + verify( + '$`c = \pm\sqrt{a^2 + b^2}`$', + + # math block + <<-GFM.strip_heredoc + ```math + c = \pm\sqrt{a^2 + b^2} + ``` + GFM + ) + end + + it 'supports math as transformed from HTML to KaTeX' do + gfm = '$`c = \pm\sqrt{a^2 + b^2}`$' + + html = <<-HTML.strip_heredoc + + + + + + c + = + ± + + + + a + 2 + + + + + b + 2 + + + + + c = \\pm\\sqrt{a^2 + b^2} + + + + + + HTML + + output_gfm = html_to_gfm(html) + expect(output_gfm.strip).to eq(gfm.strip) + end + end + + + + it 'supports SyntaxHighlightFilter' do verify( - '$`c = \pm\sqrt{a^2 + b^2}`$', - # math block + <<-GFM.strip_heredoc, + ``` + Plain text + ``` + GFM + <<-GFM.strip_heredoc - ```math - c = \pm\sqrt{a^2 + b^2} + ```ruby + def foo + bar + end ``` GFM ) end - it 'supports SyntaxHighlightFilter' do - verify <<-GFM.strip_heredoc - ```ruby - def foo - bar - end - ``` - GFM - end - it 'supports MarkdownFilter' do verify( '`code`', '`` code with ` ticks ``', '> Quote', + # multiline quote <<-GFM.strip_heredoc, > Multiline From 1bf26f7aadd94e32a9cd7f78ce0b21f185f7cae3 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 18 Jan 2017 16:19:51 -0600 Subject: [PATCH 08/20] Move some functions to utils --- app/assets/javascripts/copy_as_gfm.js.es6 | 65 ++----------------- .../javascripts/lib/utils/common_utils.js.es6 | 53 +++++++++++++++ app/assets/javascripts/shortcuts_issuable.js | 2 +- spec/javascripts/shortcuts_issuable_spec.js | 4 +- 4 files changed, 63 insertions(+), 61 deletions(-) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index 8e7f4c54213..0059dc3f60f 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -1,6 +1,9 @@ /* eslint-disable class-methods-use-this */ /*jshint esversion: 6 */ +/*= require lib/utils/common_utils */ + + (() => { const gfmRules = { // The filters referenced in lib/banzai/pipeline/gfm_pipeline.rb convert @@ -233,7 +236,7 @@ let clipboardData = e.originalEvent.clipboardData; if (!clipboardData) return; - let documentFragment = CopyAsGFM.getSelectedFragment(); + let documentFragment = window.gl.utils.getSelectedFragment(); if (!documentFragment) return; e.preventDefault(); @@ -252,36 +255,7 @@ e.preventDefault(); - CopyAsGFM.insertText(e.target, gfm); - } - - static getSelectedFragment() { - if (!window.getSelection) return null; - - let selection = window.getSelection(); - if (!selection.rangeCount || selection.rangeCount === 0) return null; - - let documentFragment = selection.getRangeAt(0).cloneContents(); - if (!documentFragment) return null; - - if (documentFragment.textContent.length === 0) return null; - - return documentFragment; - } - - static insertText(target, text) { - // Firefox doesn't support `document.execCommand('insertText', false, text)` on textareas - - let selectionStart = target.selectionStart; - let selectionEnd = target.selectionEnd; - let value = target.value; - - let textBefore = value.substring(0, selectionStart); - let textAfter = value.substring(selectionEnd, value.length); - let newText = textBefore + text + textAfter; - - target.value = newText; - target.selectionStart = target.selectionEnd = selectionStart + text.length; + window.gl.utils.insertText(e.target, gfm); } static nodeToGFM(node) { @@ -301,7 +275,7 @@ for (let selector in rules) { let func = rules[selector]; - if (!CopyAsGFM.nodeMatchesSelector(node, selector)) continue; + if (!window.gl.utils.nodeMatchesSelector(node, selector)) continue; let result = func(node, text); if (result === false) continue; @@ -324,38 +298,13 @@ let clonedNode = clonedNodes[i]; let text = this.nodeToGFM(node); - + // `clonedNode.replaceWith(text)` is not yet widely supported clonedNode.parentNode.replaceChild(document.createTextNode(text), clonedNode); } return clonedParentNode.innerText || clonedParentNode.textContent; } - - static nodeMatchesSelector(node, selector) { - let matches = Element.prototype.matches || - Element.prototype.matchesSelector || - Element.prototype.mozMatchesSelector || - Element.prototype.msMatchesSelector || - Element.prototype.oMatchesSelector || - Element.prototype.webkitMatchesSelector; - - if (matches) { - return matches.call(node, selector); - } - - // IE11 doesn't support `node.matches(selector)` - - let parentNode = node.parentNode; - if (!parentNode) { - parentNode = document.createElement('div'); - node = node.cloneNode(true); - parentNode.appendChild(node); - } - - let matchingNodes = parentNode.querySelectorAll(selector); - return Array.prototype.indexOf.call(matchingNodes, node) !== -1; - } } window.gl = window.gl || {}; diff --git a/app/assets/javascripts/lib/utils/common_utils.js.es6 b/app/assets/javascripts/lib/utils/common_utils.js.es6 index 0c6a3cc3170..59b01668688 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js.es6 +++ b/app/assets/javascripts/lib/utils/common_utils.js.es6 @@ -160,6 +160,59 @@ return decodeURIComponent(results[2].replace(/\+/g, ' ')); }; + w.gl.utils.getSelectedFragment = () => { + if (!window.getSelection) return null; + + let selection = window.getSelection(); + if (!selection.rangeCount || selection.rangeCount === 0) return null; + + let documentFragment = selection.getRangeAt(0).cloneContents(); + if (!documentFragment) return null; + + if (documentFragment.textContent.length === 0) return null; + + return documentFragment; + } + + w.gl.utils.insertText = (target, text) => { + // Firefox doesn't support `document.execCommand('insertText', false, text)` on textareas + + let selectionStart = target.selectionStart; + let selectionEnd = target.selectionEnd; + let value = target.value; + + let textBefore = value.substring(0, selectionStart); + let textAfter = value.substring(selectionEnd, value.length); + let newText = textBefore + text + textAfter; + + target.value = newText; + target.selectionStart = target.selectionEnd = selectionStart + text.length; + } + + w.gl.utils.nodeMatchesSelector = (node, selector) => { + let matches = Element.prototype.matches || + Element.prototype.matchesSelector || + Element.prototype.mozMatchesSelector || + Element.prototype.msMatchesSelector || + Element.prototype.oMatchesSelector || + Element.prototype.webkitMatchesSelector; + + if (matches) { + return matches.call(node, selector); + } + + // IE11 doesn't support `node.matches(selector)` + + let parentNode = node.parentNode; + if (!parentNode) { + parentNode = document.createElement('div'); + node = node.cloneNode(true); + parentNode.appendChild(node); + } + + let matchingNodes = parentNode.querySelectorAll(selector); + return Array.prototype.indexOf.call(matchingNodes, node) !== -1; + } })(window); }).call(this); diff --git a/app/assets/javascripts/shortcuts_issuable.js b/app/assets/javascripts/shortcuts_issuable.js index e9ede122ab7..363379f49ae 100644 --- a/app/assets/javascripts/shortcuts_issuable.js +++ b/app/assets/javascripts/shortcuts_issuable.js @@ -41,7 +41,7 @@ ShortcutsIssuable.prototype.replyWithSelectedText = function() { var quote, replyField, documentFragment, selected, separator; - documentFragment = window.gl.CopyAsGFM.getSelectedFragment(); + documentFragment = window.gl.utils.getSelectedFragment(); if (!documentFragment) return; selected = window.gl.CopyAsGFM.nodeToGFM(documentFragment); diff --git a/spec/javascripts/shortcuts_issuable_spec.js b/spec/javascripts/shortcuts_issuable_spec.js index 7e5c0e2f144..c2894d6f3ea 100644 --- a/spec/javascripts/shortcuts_issuable_spec.js +++ b/spec/javascripts/shortcuts_issuable_spec.js @@ -15,9 +15,9 @@ }); return describe('#replyWithSelectedText', function() { var stubSelection; - // Stub window.gl.CopyAsGFM.getSelectedFragment to return a node with the provided HTML. + // Stub window.gl.utils.getSelectedFragment to return a node with the provided HTML. stubSelection = function(html) { - window.gl.CopyAsGFM.getSelectedFragment = function() { + window.gl.utils.getSelectedFragment = function() { var node = document.createElement('div'); node.innerHTML = html; return node; From c463dfb66d5a6da21b2c366976814f24d4d0cdbf Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 18 Jan 2017 16:21:44 -0600 Subject: [PATCH 09/20] Satisfy Rubocop --- spec/features/copy_as_gfm_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/copy_as_gfm_spec.rb index cedddadb05c..5722c595435 100644 --- a/spec/features/copy_as_gfm_spec.rb +++ b/spec/features/copy_as_gfm_spec.rb @@ -233,8 +233,6 @@ describe 'Copy as GFM', feature: true, js: true do end end - - it 'supports SyntaxHighlightFilter' do verify( <<-GFM.strip_heredoc, From e8a265465108bde34d9c78d30890f03f2d3e007d Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 18 Jan 2017 16:38:30 -0600 Subject: [PATCH 10/20] Satisfy eslint --- app/assets/javascripts/copy_as_gfm.js.es6 | 103 +++++++++--------- .../javascripts/lib/utils/common_utils.js.es6 | 26 ++--- 2 files changed, 64 insertions(+), 65 deletions(-) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index 0059dc3f60f..83096a84543 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -1,5 +1,5 @@ -/* eslint-disable class-methods-use-this */ -/*jshint esversion: 6 */ +/* eslint-disable class-methods-use-this, object-shorthand, no-unused-vars, no-use-before-define, no-new, max-len, no-restricted-syntax, guard-for-in, no-continue */ +/* jshint esversion: 6 */ /*= require lib/utils/common_utils */ @@ -23,7 +23,7 @@ TaskListFilter: { 'input[type=checkbox].task-list-item-checkbox'(el, text) { return `[${el.checked ? 'x' : ' '}]`; - } + }, }, ReferenceFilter: { 'a.gfm:not([data-link=true])'(el, text) { @@ -55,7 +55,7 @@ }, VideoLinkFilter: { '.video-container'(el, text) { - let videoEl = el.querySelector('video'); + const videoEl = el.querySelector('video'); if (!videoEl) return false; return CopyAsGFM.nodeToGFM(videoEl); @@ -66,22 +66,22 @@ }, MathFilter: { 'pre.code.math[data-math-style=display]'(el, text) { - return '```math\n' + text.trim() + '\n```'; + return `\`\`\`math\n${text.trim()}\n\`\`\``; }, 'code.code.math[data-math-style=inline]'(el, text) { - return '$`' + text + '`$'; + return `$\`${text}\`$`; }, 'span.katex-display span.katex-mathml'(el, text) { - let mathAnnotation = el.querySelector('annotation[encoding="application/x-tex"]'); + const mathAnnotation = el.querySelector('annotation[encoding="application/x-tex"]'); if (!mathAnnotation) return false; - return '```math\n' + CopyAsGFM.nodeToGFM(mathAnnotation) + '\n```'; + return `\`\`\`math\n${CopyAsGFM.nodeToGFM(mathAnnotation)}\n\`\`\``; }, 'span.katex-mathml'(el, text) { - let mathAnnotation = el.querySelector('annotation[encoding="application/x-tex"]'); + const mathAnnotation = el.querySelector('annotation[encoding="application/x-tex"]'); if (!mathAnnotation) return false; - return '$`' + CopyAsGFM.nodeToGFM(mathAnnotation) + '`$'; + return `$\`${CopyAsGFM.nodeToGFM(mathAnnotation)}\`$`; }, 'span.katex-html'(el, text) { // We don't want to include the content of this element in the copied text. @@ -89,7 +89,7 @@ }, 'annotation[encoding="application/x-tex"]'(el, text) { return text.trim(); - } + }, }, SyntaxHighlightFilter: { 'pre.code.highlight'(el, text) { @@ -97,7 +97,7 @@ if (lang === 'plaintext') { lang = ''; } - return '```' + lang + '\n' + text.trim() + '\n```'; + return `\`\`\`${lang}\n${text.trim()}\n\`\`\``; }, 'pre > code'(el, text) { // Don't wrap code blocks in `` @@ -107,18 +107,18 @@ MarkdownFilter: { 'code'(el, text) { let backtickCount = 1; - let backtickMatch = text.match(/`+/); + const backtickMatch = text.match(/`+/); if (backtickMatch) { backtickCount = backtickMatch[0].length + 1; } - let backticks = new Array(backtickCount + 1).join('`'); - let spaceOrNoSpace = backtickCount > 1 ? ' ' : ''; + const backticks = new Array(backtickCount + 1).join('`'); + const spaceOrNoSpace = backtickCount > 1 ? ' ' : ''; return backticks + spaceOrNoSpace + text + spaceOrNoSpace + backticks; }, 'blockquote'(el, text) { - return text.trim().split('\n').map((s) => `> ${s}`.trim()).join('\n'); + return text.trim().split('\n').map(s => `> ${s}`.trim()).join('\n'); }, 'img'(el, text) { return `![${el.getAttribute('alt')}](${el.getAttribute('src')})`; @@ -131,15 +131,14 @@ return `[${text}](${el.getAttribute('href')})`; }, 'li'(el, text) { - let lines = text.trim().split('\n'); - let firstLine = '- ' + lines.shift(); - // Add two spaces to the front of subsequent list items lines, or leave the line entirely blank. - let nextLines = lines.map(function(s) { - if (s.trim().length === 0) { - return ''; - } else { - return ` ${s}`; - } + const lines = text.trim().split('\n'); + const firstLine = `- ${lines.shift()}`; + // Add two spaces to the front of subsequent list items lines, + // or leave the line entirely blank. + const nextLines = lines.map((s) => { + if (s.trim().length === 0) return ''; + + return ` ${s}`; }); return `${firstLine}\n${nextLines.join('\n')}`; @@ -185,13 +184,13 @@ return '-----'; }, 'table'(el, text) { - let theadText = CopyAsGFM.nodeToGFM(el.querySelector('thead')); - let tbodyText = CopyAsGFM.nodeToGFM(el.querySelector('tbody')); + const theadText = CopyAsGFM.nodeToGFM(el.querySelector('thead')); + const tbodyText = CopyAsGFM.nodeToGFM(el.querySelector('tbody')); return theadText + tbodyText; }, 'thead'(el, text) { - let cells = _.map(el.querySelectorAll('th'), function(cell) { + const cells = _.map(el.querySelectorAll('th'), (cell) => { let chars = CopyAsGFM.nodeToGFM(cell).trim().length; let before = ''; @@ -206,24 +205,24 @@ after = ':'; chars -= 1; break; + default: + break; } chars = Math.max(chars, 0); - let middle = new Array(chars + 1).join('-'); + const middle = new Array(chars + 1).join('-'); return before + middle + after; }); - return text + `| ${cells.join(' | ')} |`; + return `${text}| ${cells.join(' | ')} |`; }, 'tr'(el, text) { - let cells = _.map(el.querySelectorAll('td, th'), function(cell) { - return CopyAsGFM.nodeToGFM(cell).trim(); - }); + const cells = _.map(el.querySelectorAll('td, th'), cell => CopyAsGFM.nodeToGFM(cell).trim()); return `| ${cells.join(' | ')} |`; }, - } + }, }; class CopyAsGFM { @@ -233,24 +232,24 @@ } handleCopy(e) { - let clipboardData = e.originalEvent.clipboardData; + const clipboardData = e.originalEvent.clipboardData; if (!clipboardData) return; - let documentFragment = window.gl.utils.getSelectedFragment(); + const documentFragment = window.gl.utils.getSelectedFragment(); if (!documentFragment) return; e.preventDefault(); clipboardData.setData('text/plain', documentFragment.textContent); - let gfm = CopyAsGFM.nodeToGFM(documentFragment); + const gfm = CopyAsGFM.nodeToGFM(documentFragment); clipboardData.setData('text/x-gfm', gfm); } handlePaste(e) { - let clipboardData = e.originalEvent.clipboardData; + const clipboardData = e.originalEvent.clipboardData; if (!clipboardData) return; - let gfm = clipboardData.getData('text/x-gfm'); + const gfm = clipboardData.getData('text/x-gfm'); if (!gfm) return; e.preventDefault(); @@ -263,21 +262,21 @@ return node.textContent; } - let text = this.innerGFM(node); + const text = this.innerGFM(node); if (node.nodeType === Node.DOCUMENT_FRAGMENT_NODE) { return text; } - for (let filter in gfmRules) { - let rules = gfmRules[filter]; + for (const filter in gfmRules) { + const rules = gfmRules[filter]; - for (let selector in rules) { - let func = rules[selector]; + for (const selector in rules) { + const func = rules[selector]; if (!window.gl.utils.nodeMatchesSelector(node, selector)) continue; - let result = func(node, text); + const result = func(node, text); if (result === false) continue; return result; @@ -288,16 +287,16 @@ } static innerGFM(parentNode) { - let nodes = parentNode.childNodes; + const nodes = parentNode.childNodes; - let clonedParentNode = parentNode.cloneNode(true); - let clonedNodes = Array.prototype.slice.call(clonedParentNode.childNodes, 0); + const clonedParentNode = parentNode.cloneNode(true); + const clonedNodes = Array.prototype.slice.call(clonedParentNode.childNodes, 0); - for (let i = 0; i < nodes.length; i++) { - let node = nodes[i]; - let clonedNode = clonedNodes[i]; + for (let i = 0; i < nodes.length; i += 1) { + const node = nodes[i]; + const clonedNode = clonedNodes[i]; - let text = this.nodeToGFM(node); + const text = this.nodeToGFM(node); // `clonedNode.replaceWith(text)` is not yet widely supported clonedNode.parentNode.replaceChild(document.createTextNode(text), clonedNode); diff --git a/app/assets/javascripts/lib/utils/common_utils.js.es6 b/app/assets/javascripts/lib/utils/common_utils.js.es6 index 59b01668688..43179e8ab88 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js.es6 +++ b/app/assets/javascripts/lib/utils/common_utils.js.es6 @@ -163,34 +163,34 @@ w.gl.utils.getSelectedFragment = () => { if (!window.getSelection) return null; - let selection = window.getSelection(); + const selection = window.getSelection(); if (!selection.rangeCount || selection.rangeCount === 0) return null; - let documentFragment = selection.getRangeAt(0).cloneContents(); + const documentFragment = selection.getRangeAt(0).cloneContents(); if (!documentFragment) return null; if (documentFragment.textContent.length === 0) return null; return documentFragment; - } + }; w.gl.utils.insertText = (target, text) => { // Firefox doesn't support `document.execCommand('insertText', false, text)` on textareas - let selectionStart = target.selectionStart; - let selectionEnd = target.selectionEnd; - let value = target.value; + const selectionStart = target.selectionStart; + const selectionEnd = target.selectionEnd; + const value = target.value; - let textBefore = value.substring(0, selectionStart); - let textAfter = value.substring(selectionEnd, value.length); - let newText = textBefore + text + textAfter; + const textBefore = value.substring(0, selectionStart); + const textAfter = value.substring(selectionEnd, value.length); + const newText = textBefore + text + textAfter; target.value = newText; target.selectionStart = target.selectionEnd = selectionStart + text.length; - } + }; w.gl.utils.nodeMatchesSelector = (node, selector) => { - let matches = Element.prototype.matches || + const matches = Element.prototype.matches || Element.prototype.matchesSelector || Element.prototype.mozMatchesSelector || Element.prototype.msMatchesSelector || @@ -210,9 +210,9 @@ parentNode.appendChild(node); } - let matchingNodes = parentNode.querySelectorAll(selector); + const matchingNodes = parentNode.querySelectorAll(selector); return Array.prototype.indexOf.call(matchingNodes, node) !== -1; - } + }; })(window); }).call(this); From 5bc467164cb103724205b21c203998e065b7970d Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 18 Jan 2017 17:22:27 -0600 Subject: [PATCH 11/20] No needed to create an array --- app/assets/javascripts/copy_as_gfm.js.es6 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index 83096a84543..8f3a96c8f5d 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -112,7 +112,7 @@ backtickCount = backtickMatch[0].length + 1; } - const backticks = new Array(backtickCount + 1).join('`'); + const backticks = Array(backtickCount + 1).join('`'); const spaceOrNoSpace = backtickCount > 1 ? ' ' : ''; return backticks + spaceOrNoSpace + text + spaceOrNoSpace + backticks; @@ -211,7 +211,7 @@ chars = Math.max(chars, 0); - const middle = new Array(chars + 1).join('-'); + const middle = Array(chars + 1).join('-'); return before + middle + after; }); From 359c41761947ea50b84718b34b327d48a689d147 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 19 Jan 2017 10:01:27 -0600 Subject: [PATCH 12/20] Properly copy/paste allowed HTML --- app/assets/javascripts/copy_as_gfm.js.es6 | 22 ++++++++++++++++ spec/features/copy_as_gfm_spec.rb | 31 +++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index 8f3a96c8f5d..6c13ba2ccb8 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -91,6 +91,28 @@ return text.trim(); }, }, + SanitizationFilter: { + 'br'(el, text) { + return '
'; + }, + 'dl'(el, text) { + let lines = text.trim().split('\n'); + // Add two spaces to the front of subsequent list items lines, + // or leave the line entirely blank. + lines = lines.map((s) => { + s = s.trim(); + if (s.length === 0) return ''; + + return ` ${s}`; + }); + + return `
\n${lines.join('\n')}\n
`; + }, + 'sub, dt, dd, kbd, q, samp, var, ruby, rt, rp, abbr'(el, text) { + const tag = el.nodeName.toLowerCase(); + return `<${tag}>${text}`; + }, + }, SyntaxHighlightFilter: { 'pre.code.highlight'(el, text) { let lang = el.getAttribute('lang'); diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/copy_as_gfm_spec.rb index 5722c595435..86cb67d275a 100644 --- a/spec/features/copy_as_gfm_spec.rb +++ b/spec/features/copy_as_gfm_spec.rb @@ -233,6 +233,37 @@ describe 'Copy as GFM', feature: true, js: true do end end + it 'supports SanitizationFilter' do + verify( + <<-GFM.strip_heredoc + BR:
+ + sub + +
+
dt
+
dd
+
+ + kbd + + q + + samp + + var + + ruby + + rt + + rp + + abbr + GFM + ) + end + it 'supports SyntaxHighlightFilter' do verify( <<-GFM.strip_heredoc, From 9b800bc6418621b601fead1c18dd0a327534c998 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 19 Jan 2017 10:01:50 -0600 Subject: [PATCH 13/20] Remove unneeded code --- app/assets/javascripts/copy_as_gfm.js.es6 | 4 ++-- app/assets/javascripts/lib/utils/common_utils.js.es6 | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index 6c13ba2ccb8..c55aee39fca 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -249,8 +249,8 @@ class CopyAsGFM { constructor() { - $(document).on('copy', '.md, .wiki', this.handleCopy.bind(this)); - $(document).on('paste', '.js-gfm-input', this.handlePaste.bind(this)); + $(document).on('copy', '.md, .wiki', this.handleCopy); + $(document).on('paste', '.js-gfm-input', this.handlePaste); } handleCopy(e) { diff --git a/app/assets/javascripts/lib/utils/common_utils.js.es6 b/app/assets/javascripts/lib/utils/common_utils.js.es6 index 43179e8ab88..24aedcae1ed 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js.es6 +++ b/app/assets/javascripts/lib/utils/common_utils.js.es6 @@ -161,8 +161,6 @@ }; w.gl.utils.getSelectedFragment = () => { - if (!window.getSelection) return null; - const selection = window.getSelection(); if (!selection.rangeCount || selection.rangeCount === 0) return null; From b6ac53322e8562e65bbed3a4decce2dd057a3d1e Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 19 Jan 2017 10:02:19 -0600 Subject: [PATCH 14/20] Don't copy as GFM when more than GFM is selected --- app/assets/javascripts/copy_as_gfm.js.es6 | 3 +++ app/assets/javascripts/shortcuts_issuable.js | 3 +++ 2 files changed, 6 insertions(+) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index c55aee39fca..80e30fdf390 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -260,6 +260,9 @@ const documentFragment = window.gl.utils.getSelectedFragment(); if (!documentFragment) return; + // If the documentFragment contains more than just Markdown, don't copy as GFM. + if (documentFragment.querySelector('.md, .wiki')) return; + e.preventDefault(); clipboardData.setData('text/plain', documentFragment.textContent); diff --git a/app/assets/javascripts/shortcuts_issuable.js b/app/assets/javascripts/shortcuts_issuable.js index 363379f49ae..6603b9679b9 100644 --- a/app/assets/javascripts/shortcuts_issuable.js +++ b/app/assets/javascripts/shortcuts_issuable.js @@ -44,6 +44,9 @@ documentFragment = window.gl.utils.getSelectedFragment(); if (!documentFragment) return; + // If the documentFragment contains more than just Markdown, don't copy as GFM. + if (documentFragment.querySelector('.md, .wiki')) return; + selected = window.gl.CopyAsGFM.nodeToGFM(documentFragment); replyField = $('.js-main-target-form #note_note'); From dd6f91cdcc015f554df0fdeb78b30fa6d627d38d Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 19 Jan 2017 10:02:39 -0600 Subject: [PATCH 15/20] Trigger autosize on the textarea after pasting --- app/assets/javascripts/lib/utils/common_utils.js.es6 | 8 ++++++++ app/assets/javascripts/shortcuts_issuable.js | 9 ++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/lib/utils/common_utils.js.es6 b/app/assets/javascripts/lib/utils/common_utils.js.es6 index 24aedcae1ed..92b2ef6e959 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js.es6 +++ b/app/assets/javascripts/lib/utils/common_utils.js.es6 @@ -185,6 +185,14 @@ target.value = newText; target.selectionStart = target.selectionEnd = selectionStart + text.length; + + // Trigger autosave + $(target).trigger('input'); + + // Trigger autosize + var event = document.createEvent('Event'); + event.initEvent('autosize:update', true, false); + target.dispatchEvent(event); }; w.gl.utils.nodeMatchesSelector = (node, selector) => { diff --git a/app/assets/javascripts/shortcuts_issuable.js b/app/assets/javascripts/shortcuts_issuable.js index 6603b9679b9..97fa68c8437 100644 --- a/app/assets/javascripts/shortcuts_issuable.js +++ b/app/assets/javascripts/shortcuts_issuable.js @@ -61,8 +61,15 @@ replyField.val(function(_, current) { return current + separator + quote.join('') + "\n"; }); - // Trigger autosave for the added text + + // Trigger autosave replyField.trigger('input'); + + // Trigger autosize + var event = document.createEvent('Event'); + event.initEvent('autosize:update', true, false); + replyField.get(0).dispatchEvent(event); + // Focus the input field return replyField.focus(); }; From f928e19cb572b34d93baaa5e3040d99fc5ba9939 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 19 Jan 2017 10:06:47 -0600 Subject: [PATCH 16/20] Remove unneeded checks --- app/assets/javascripts/lib/utils/common_utils.js.es6 | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/assets/javascripts/lib/utils/common_utils.js.es6 b/app/assets/javascripts/lib/utils/common_utils.js.es6 index 92b2ef6e959..eefb727f009 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js.es6 +++ b/app/assets/javascripts/lib/utils/common_utils.js.es6 @@ -162,11 +162,7 @@ w.gl.utils.getSelectedFragment = () => { const selection = window.getSelection(); - if (!selection.rangeCount || selection.rangeCount === 0) return null; - const documentFragment = selection.getRangeAt(0).cloneContents(); - if (!documentFragment) return null; - if (documentFragment.textContent.length === 0) return null; return documentFragment; From d89f56161297921f3f3ccdf0d186f39ff1c0a4d3 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 19 Jan 2017 13:47:54 -0600 Subject: [PATCH 17/20] Improve support for linebreaks, tables and nested blockquotes in lists --- app/assets/javascripts/copy_as_gfm.js.es6 | 17 ++++++------- spec/features/copy_as_gfm_spec.rb | 29 +++++++++++++++++------ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index 80e30fdf390..9cbeb782270 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -92,9 +92,6 @@ }, }, SanitizationFilter: { - 'br'(el, text) { - return '
'; - }, 'dl'(el, text) { let lines = text.trim().split('\n'); // Add two spaces to the front of subsequent list items lines, @@ -127,6 +124,10 @@ }, }, MarkdownFilter: { + 'br'(el, text) { + // Two spaces at the end of a line are turned into a BR + return ' '; + }, 'code'(el, text) { let backtickCount = 1; const backtickMatch = text.match(/`+/); @@ -155,12 +156,12 @@ 'li'(el, text) { const lines = text.trim().split('\n'); const firstLine = `- ${lines.shift()}`; - // Add two spaces to the front of subsequent list items lines, + // Add four spaces to the front of subsequent list items lines, // or leave the line entirely blank. const nextLines = lines.map((s) => { if (s.trim().length === 0) return ''; - return ` ${s}`; + return ` ${s}`; }); return `${firstLine}\n${nextLines.join('\n')}`; @@ -213,7 +214,7 @@ }, 'thead'(el, text) { const cells = _.map(el.querySelectorAll('th'), (cell) => { - let chars = CopyAsGFM.nodeToGFM(cell).trim().length; + let chars = CopyAsGFM.nodeToGFM(cell).trim().length + 2; let before = ''; let after = ''; @@ -231,14 +232,14 @@ break; } - chars = Math.max(chars, 0); + chars = Math.max(chars, 3); const middle = Array(chars + 1).join('-'); return before + middle + after; }); - return `${text}| ${cells.join(' | ')} |`; + return `${text}|${cells.join('|')}|`; }, 'tr'(el, text) { const cells = _.map(el.querySelectorAll('td, th'), cell => CopyAsGFM.nodeToGFM(cell).trim()); diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/copy_as_gfm_spec.rb index 86cb67d275a..6656d6a1e5d 100644 --- a/spec/features/copy_as_gfm_spec.rb +++ b/spec/features/copy_as_gfm_spec.rb @@ -236,8 +236,6 @@ describe 'Copy as GFM', feature: true, js: true do it 'supports SanitizationFilter' do verify( <<-GFM.strip_heredoc - BR:
- sub
@@ -284,6 +282,8 @@ describe 'Copy as GFM', feature: true, js: true do it 'supports MarkdownFilter' do verify( + "Line with two spaces at the end \nto insert a linebreak", + '`code`', '`` code with ` ticks ``', @@ -308,7 +308,7 @@ describe 'Copy as GFM', feature: true, js: true do # multiline list item <<-GFM.strip_heredoc, - Multiline - List item + List item GFM # nested lists @@ -316,7 +316,14 @@ describe 'Copy as GFM', feature: true, js: true do - Nested - - Lists + - Lists + GFM + + # list with blockquote + <<-GFM.strip_heredoc, + - List + + > Blockquote GFM '1. Numbered list item', @@ -324,7 +331,7 @@ describe 'Copy as GFM', feature: true, js: true do # multiline numbered list item <<-GFM.strip_heredoc, 1. Multiline - Numbered list item + Numbered list item GFM # nested numbered list @@ -332,7 +339,7 @@ describe 'Copy as GFM', feature: true, js: true do 1. Nested - 1. Numbered lists + 1. Numbered lists GFM '# Heading', @@ -355,10 +362,18 @@ describe 'Copy as GFM', feature: true, js: true do # table <<-GFM.strip_heredoc, | Centered | Right | Left | - | :------: | ----: | ---- | + |:--------:|------:|------| | Foo | Bar | **Baz** | | Foo | Bar | **Baz** | GFM + + # table with empty heading + <<-GFM.strip_heredoc, + | | x | y | + |---|---|---| + | a | 1 | 0 | + | b | 0 | 1 | + GFM ) end From bd2880bbc6238ffc4954dba2690d357e4313a34c Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 19 Jan 2017 13:54:59 -0600 Subject: [PATCH 18/20] Improve handling of code blocks containing triple backticks --- app/assets/javascripts/copy_as_gfm.js.es6 | 22 +++++++++++++++++++--- spec/features/copy_as_gfm_spec.rb | 12 +++++++++++- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index 9cbeb782270..554849feb6b 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -116,7 +116,19 @@ if (lang === 'plaintext') { lang = ''; } - return `\`\`\`${lang}\n${text.trim()}\n\`\`\``; + text = text.trim(); + + // Prefixes lines with 4 spaces if the code contains triple backticks + if (lang === '' && text.match(/^```/gm)) { + return text.split('\n').map((s) => { + s = s.trim(); + if (s.length === 0) return ''; + + return ` ${s}`; + }).join('\n'); + } + + return `\`\`\`${lang}\n${text}\n\`\`\``; }, 'pre > code'(el, text) { // Don't wrap code blocks in `` @@ -207,8 +219,12 @@ return '-----'; }, 'table'(el, text) { - const theadText = CopyAsGFM.nodeToGFM(el.querySelector('thead')); - const tbodyText = CopyAsGFM.nodeToGFM(el.querySelector('tbody')); + const theadEl = el.querySelector('thead'); + const tbodyEl = el.querySelector('tbody'); + if (!theadEl || !tbodyEl) return false; + + const theadText = CopyAsGFM.nodeToGFM(theadEl); + const tbodyText = CopyAsGFM.nodeToGFM(tbodyEl); return theadText + tbodyText; }, diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/copy_as_gfm_spec.rb index 6656d6a1e5d..f3a8447162c 100644 --- a/spec/features/copy_as_gfm_spec.rb +++ b/spec/features/copy_as_gfm_spec.rb @@ -270,13 +270,23 @@ describe 'Copy as GFM', feature: true, js: true do ``` GFM - <<-GFM.strip_heredoc + <<-GFM.strip_heredoc, ```ruby def foo bar end ``` GFM + + <<-GFM.strip_heredoc + Foo + + This is an example of GFM + + ```js + Code goes here + ``` + GFM ) end From 43dc6263526dbbb7dab44d91406c36b07ce2829b Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 24 Jan 2017 16:55:12 -0600 Subject: [PATCH 19/20] Run tests in a single browser session --- spec/features/copy_as_gfm_spec.rb | 173 ++++++++++++++++-------------- 1 file changed, 94 insertions(+), 79 deletions(-) diff --git a/spec/features/copy_as_gfm_spec.rb b/spec/features/copy_as_gfm_spec.rb index f3a8447162c..f3a5b565122 100644 --- a/spec/features/copy_as_gfm_spec.rb +++ b/spec/features/copy_as_gfm_spec.rb @@ -18,60 +18,67 @@ describe 'Copy as GFM', feature: true, js: true do # To make sure these filters and handlers are properly aligned, this spec tests the GFM-to-HTML-to-GFM cycle # by verifying (`html_to_gfm(gfm_to_html(gfm)) == gfm`) for a number of examples of GFM for every filter, using the `verify` helper. - it 'supports nesting' do - verify '> 1. [x] **[$`2 + 2`$ {-=-}{+=+} 2^2 ~~:thumbsup:~~](http://google.com)**' - end - - it 'supports a real world example from the gitlab-ce README' do - verify <<-GFM.strip_heredoc - # GitLab - - [![Build status](https://gitlab.com/gitlab-org/gitlab-ce/badges/master/build.svg)](https://gitlab.com/gitlab-org/gitlab-ce/commits/master) - [![CE coverage report](https://gitlab.com/gitlab-org/gitlab-ce/badges/master/coverage.svg?job=coverage)](https://gitlab-org.gitlab.io/gitlab-ce/coverage-ruby) - [![Code Climate](https://codeclimate.com/github/gitlabhq/gitlabhq.svg)](https://codeclimate.com/github/gitlabhq/gitlabhq) - [![Core Infrastructure Initiative Best Practices](https://bestpractices.coreinfrastructure.org/projects/42/badge)](https://bestpractices.coreinfrastructure.org/projects/42) - - ## Canonical source - - The canonical source of GitLab Community Edition is [hosted on GitLab.com](https://gitlab.com/gitlab-org/gitlab-ce/). - - ## Open source software to collaborate on code - - To see how GitLab looks please see the [features page on our website](https://about.gitlab.com/features/). - - - - Manage Git repositories with fine grained access controls that keep your code secure - - - Perform code reviews and enhance collaboration with merge requests - - - Complete continuous integration (CI) and CD pipelines to builds, test, and deploy your applications - - - Each project can also have an issue tracker, issue board, and a wiki - - - Used by more than 100,000 organizations, GitLab is the most popular solution to manage Git repositories on-premises - - - Completely free and open source (MIT Expat license) - GFM - end - - it 'supports InlineDiffFilter' do + # These are all in a single `it` for performance reasons. + it 'works', :aggregate_failures do verify( + 'nesting', + + '> 1. [x] **[$`2 + 2`$ {-=-}{+=+} 2^2 ~~:thumbsup:~~](http://google.com)**' + ) + + verify( + 'a real world example from the gitlab-ce README', + + <<-GFM.strip_heredoc + # GitLab + + [![Build status](https://gitlab.com/gitlab-org/gitlab-ce/badges/master/build.svg)](https://gitlab.com/gitlab-org/gitlab-ce/commits/master) + [![CE coverage report](https://gitlab.com/gitlab-org/gitlab-ce/badges/master/coverage.svg?job=coverage)](https://gitlab-org.gitlab.io/gitlab-ce/coverage-ruby) + [![Code Climate](https://codeclimate.com/github/gitlabhq/gitlabhq.svg)](https://codeclimate.com/github/gitlabhq/gitlabhq) + [![Core Infrastructure Initiative Best Practices](https://bestpractices.coreinfrastructure.org/projects/42/badge)](https://bestpractices.coreinfrastructure.org/projects/42) + + ## Canonical source + + The canonical source of GitLab Community Edition is [hosted on GitLab.com](https://gitlab.com/gitlab-org/gitlab-ce/). + + ## Open source software to collaborate on code + + To see how GitLab looks please see the [features page on our website](https://about.gitlab.com/features/). + + + - Manage Git repositories with fine grained access controls that keep your code secure + + - Perform code reviews and enhance collaboration with merge requests + + - Complete continuous integration (CI) and CD pipelines to builds, test, and deploy your applications + + - Each project can also have an issue tracker, issue board, and a wiki + + - Used by more than 100,000 organizations, GitLab is the most popular solution to manage Git repositories on-premises + + - Completely free and open source (MIT Expat license) + GFM + ) + + verify( + 'InlineDiffFilter', + '{-Deleted text-}', '{+Added text+}' ) - end - it 'supports TaskListFilter' do verify( + 'TaskListFilter', + '- [ ] Unchecked task', '- [x] Checked task', '1. [ ] Unchecked numbered task', '1. [x] Checked numbered task' ) - end - it 'supports ReferenceFilter' do verify( + 'ReferenceFilter', + # issue reference @feat.issue.to_reference, # full issue reference @@ -85,43 +92,51 @@ describe 'Copy as GFM', feature: true, js: true do # issue link with note anchor "[Issue](#{namespace_project_issue_url(@project.namespace, @project, @feat.issue, anchor: 'note_123')})", ) - end - it 'supports AutolinkFilter' do - verify 'https://example.com' - end + verify( + 'AutolinkFilter', - it 'supports TableOfContentsFilter' do - verify '[[_TOC_]]' - end + 'https://example.com' + ) - it 'supports EmojiFilter' do - verify ':thumbsup:' - end + verify( + 'TableOfContentsFilter', - it 'supports ImageLinkFilter' do - verify '![Image](https://example.com/image.png)' - end + '[[_TOC_]]' + ) - it 'supports VideoLinkFilter' do - verify '![Video](https://example.com/video.mp4)' - end + verify( + 'EmojiFilter', - context 'MathFilter' do - it 'supports math as converted from GFM to HTML' do - verify( - '$`c = \pm\sqrt{a^2 + b^2}`$', + ':thumbsup:' + ) - # math block - <<-GFM.strip_heredoc - ```math - c = \pm\sqrt{a^2 + b^2} - ``` - GFM - ) - end + verify( + 'ImageLinkFilter', - it 'supports math as transformed from HTML to KaTeX' do + '![Image](https://example.com/image.png)' + ) + + verify( + 'VideoLinkFilter', + + '![Video](https://example.com/video.mp4)' + ) + + verify( + 'MathFilter: math as converted from GFM to HTML', + + '$`c = \pm\sqrt{a^2 + b^2}`$', + + # math block + <<-GFM.strip_heredoc + ```math + c = \pm\sqrt{a^2 + b^2} + ``` + GFM + ) + + aggregate_failures('MathFilter: math as transformed from HTML to KaTeX') do gfm = '$`c = \pm\sqrt{a^2 + b^2}`$' html = <<-HTML.strip_heredoc @@ -231,10 +246,10 @@ describe 'Copy as GFM', feature: true, js: true do output_gfm = html_to_gfm(html) expect(output_gfm.strip).to eq(gfm.strip) end - end - it 'supports SanitizationFilter' do verify( + 'SanitizationFilter', + <<-GFM.strip_heredoc sub @@ -260,10 +275,10 @@ describe 'Copy as GFM', feature: true, js: true do abbr GFM ) - end - it 'supports SyntaxHighlightFilter' do verify( + 'SanitizationFilter', + <<-GFM.strip_heredoc, ``` Plain text @@ -280,7 +295,7 @@ describe 'Copy as GFM', feature: true, js: true do <<-GFM.strip_heredoc Foo - + This is an example of GFM ```js @@ -288,10 +303,10 @@ describe 'Copy as GFM', feature: true, js: true do ``` GFM ) - end - it 'supports MarkdownFilter' do verify( + 'MarkdownFilter', + "Line with two spaces at the end \nto insert a linebreak", '`code`', @@ -400,8 +415,8 @@ describe 'Copy as GFM', feature: true, js: true do page.evaluate_script(js) end - def verify(*gfms) - aggregate_failures do + def verify(label, *gfms) + aggregate_failures(label) do gfms.each do |gfm| html = gfm_to_html(gfm) output_gfm = html_to_gfm(html) From 6c2d8f357e141149f5b21153e540957ed48cbbab Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 25 Jan 2017 11:50:29 -0600 Subject: [PATCH 20/20] Statisfy eslint --- app/assets/javascripts/copy_as_gfm.js.es6 | 24 ++++++++++---------- app/assets/javascripts/shortcuts_issuable.js | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/copy_as_gfm.js.es6 b/app/assets/javascripts/copy_as_gfm.js.es6 index 554849feb6b..b94125a4210 100644 --- a/app/assets/javascripts/copy_as_gfm.js.es6 +++ b/app/assets/javascripts/copy_as_gfm.js.es6 @@ -3,7 +3,6 @@ /*= require lib/utils/common_utils */ - (() => { const gfmRules = { // The filters referenced in lib/banzai/pipeline/gfm_pipeline.rb convert @@ -96,11 +95,11 @@ let lines = text.trim().split('\n'); // Add two spaces to the front of subsequent list items lines, // or leave the line entirely blank. - lines = lines.map((s) => { - s = s.trim(); - if (s.length === 0) return ''; + lines = lines.map((l) => { + const line = l.trim(); + if (line.length === 0) return ''; - return ` ${s}`; + return ` ${line}`; }); return `
\n${lines.join('\n')}\n
`; @@ -111,20 +110,21 @@ }, }, SyntaxHighlightFilter: { - 'pre.code.highlight'(el, text) { + 'pre.code.highlight'(el, t) { + const text = t.trim(); + let lang = el.getAttribute('lang'); if (lang === 'plaintext') { lang = ''; } - text = text.trim(); - // Prefixes lines with 4 spaces if the code contains triple backticks + // Prefixes lines with 4 spaces if the code contains triple backticks if (lang === '' && text.match(/^```/gm)) { - return text.split('\n').map((s) => { - s = s.trim(); - if (s.length === 0) return ''; + return text.split('\n').map((l) => { + const line = l.trim(); + if (line.length === 0) return ''; - return ` ${s}`; + return ` ${line}`; }).join('\n'); } diff --git a/app/assets/javascripts/shortcuts_issuable.js b/app/assets/javascripts/shortcuts_issuable.js index 28491934c10..4ef516af8c8 100644 --- a/app/assets/javascripts/shortcuts_issuable.js +++ b/app/assets/javascripts/shortcuts_issuable.js @@ -61,7 +61,7 @@ replyField.val(function(_, current) { return current + separator + quote.join('') + "\n"; }); - + // Trigger autosave replyField.trigger('input');