From ead0692c0292aa52331071c4ccad82d941c31742 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 14 Mar 2019 12:53:06 -0400 Subject: [PATCH 1/5] Asciidoctor: BWC for // CONSOLE then callouts Our backwards compatibility support for "OPEN IN CONSOLE"-style links did not work with snippets followed by a callout list. That looks like this: ``` [source,js] ---- GET / <1> ---- // CONSOLE <1> words go here ``` These is actually *super* common and I'm surprised I didn't catch it before! This adds tests for it so it won't sneak through again. The reason this didn't work is that our backwards compatibility worked by transforming the `// CONSOLE` comment in `pass:[// CONSOLE]` and then catching that when walking the tree. We do this in two passes because our preprocessor works line by line and reading ahead can cause issues. So we do the "action at a distance" in a tree processor where it is quite simple. The trouble is that the `pass:[// CONSOLE]` construct is *inline* so when it is immediately followed by the `<1> words go here` line it gets sucked into the same block which Asciidoctor thinks of as a paragraph. This causes the parsing for the callout list not to happen, causing all kinds of trouble. The fix for this is to introduce a block macro that functions like `pass:[// CONSOLE]` and to use that instead. This causes the callout list to be parsed properly. --- README.asciidoc | 12 +- .../elastic_compat_preprocessor/extension.rb | 4 +- .../extension.rb | 10 +- resources/asciidoctor/lib/extensions.rb | 2 + .../lib/lang_override/extension.rb | 15 +++ .../spec/elastic_compat_preprocessor_spec.rb | 2 + .../elastic_compat_tree_processor_spec.rb | 121 +++++++++++------- 7 files changed, 105 insertions(+), 61 deletions(-) create mode 100644 resources/asciidoctor/lib/lang_override/extension.rb diff --git a/README.asciidoc b/README.asciidoc index 870528ffad360..b0d48d85b2e30 100644 --- a/README.asciidoc +++ b/README.asciidoc @@ -1022,19 +1022,9 @@ the Elaticsearch repository because it can recognize it to make tests. The "Asciidoctor". Try it first and if it works then use it. Otherwise, use the "AsciiDoc" way. -//// - -The tricks that we pull to make ascidoctor support // CONSOLE are windy -and force us to add subs=+macros when we render an asciidoc snippet. -We *don't* require that for normal snippets, just those that contain -asciidoc. - -//// - .Code block with CONSOLE link (AsciiDoc way) ================================== -ifdef::asciidoctor[[source,asciidoc,subs=+macros]] -ifndef::asciidoctor[[source,asciidoc]] +[source,asciidoc] -- [source,js] ---------------------------------- diff --git a/resources/asciidoctor/lib/elastic_compat_preprocessor/extension.rb b/resources/asciidoctor/lib/elastic_compat_preprocessor/extension.rb index 7d77719d1f95e..1b26b38907460 100644 --- a/resources/asciidoctor/lib/elastic_compat_preprocessor/extension.rb +++ b/resources/asciidoctor/lib/elastic_compat_preprocessor/extension.rb @@ -113,7 +113,7 @@ class ElasticCompatPreprocessor < Asciidoctor::Extensions::Preprocessor INCLUDE_TAGGED_DIRECTIVE_RX = /^include-tagged::([^\[][^\[]*)\[(#{Asciidoctor::CC_ANY}+)?\]$/ SOURCE_WITH_SUBS_RX = /^\["source", ?"[^"]+", ?subs="(#{Asciidoctor::CC_ANY}+)"\]$/ CODE_BLOCK_RX = /^-----*$/ - SNIPPET_RX = %r{//\s*(?:AUTOSENSE|KIBANA|CONSOLE|SENSE:[^\n<]+)} + SNIPPET_RX = %r{^//\s*(AUTOSENSE|KIBANA|CONSOLE|SENSE:[^\n<]+)$} # NOCOMMIT handle trailing spaces LEGACY_MACROS = 'added|beta|coming|deprecated|experimental' LEGACY_BLOCK_MACRO_RX = /^(#{LEGACY_MACROS})\[([^\]]*)\]/ LEGACY_INLINE_MACRO_RX = /(#{LEGACY_MACROS})\[([^\]]*)\]/ @@ -178,7 +178,7 @@ def reader.process_line(line) # CONSOLE snippet. Asciidoctor really doesn't recommend this sort of # thing but we have thousands of them and it'll take us some time to # stop doing it. - line&.gsub!(SNIPPET_RX, 'pass:[\0]') + line&.gsub!(SNIPPET_RX, 'lang_override::[\1]') end end reader diff --git a/resources/asciidoctor/lib/elastic_compat_tree_processor/extension.rb b/resources/asciidoctor/lib/elastic_compat_tree_processor/extension.rb index 4f5848c681455..41eb6ba49ebd7 100644 --- a/resources/asciidoctor/lib/elastic_compat_tree_processor/extension.rb +++ b/resources/asciidoctor/lib/elastic_compat_tree_processor/extension.rb @@ -74,11 +74,13 @@ def process_lang_override(block) return unless my_index next_block = block.parent.blocks[my_index + 1] - return unless next_block && next_block.context == :paragraph - return unless next_block.source =~ %r{pass:\[//\s*([^:\]]+)(?::\s*([^\]]+))?\]} + return unless next_block && next_block.context == :pass - lang = LANG_MAPPING[$1] - snippet = $2 + m = %r{^//\s*([^:\]]+)(?::\s*([^\]]+))?$}.match(next_block.source) + return unless m + + lang = LANG_MAPPING[m[1]] + snippet = m[2] return unless lang # Not a language we handle block.set_attr 'language', lang diff --git a/resources/asciidoctor/lib/extensions.rb b/resources/asciidoctor/lib/extensions.rb index 2636161689d39..0ebc88604831c 100644 --- a/resources/asciidoctor/lib/extensions.rb +++ b/resources/asciidoctor/lib/extensions.rb @@ -8,6 +8,7 @@ require_relative 'elastic_compat_tree_processor/extension' require_relative 'elastic_compat_preprocessor/extension' require_relative 'elastic_include_tagged/extension' +require_relative 'lang_override/extension' require_relative 'open_in_widget/extension' Asciidoctor::Extensions.register CareAdmonition @@ -16,6 +17,7 @@ # Enable storing the source locations so we can look at them. This is required # for EditMe to get a nice location. document.sourcemap = true + block_macro LangOverride preprocessor CrampedInclude preprocessor ElasticCompatPreprocessor treeprocessor CopyImages::CopyImages diff --git a/resources/asciidoctor/lib/lang_override/extension.rb b/resources/asciidoctor/lib/lang_override/extension.rb new file mode 100644 index 0000000000000..ebf6066b8b6b8 --- /dev/null +++ b/resources/asciidoctor/lib/lang_override/extension.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +## +# Block macro that exists entirely to mark language overrides in a clean way +# without additing additional lines to the input file which would throw off +# line numbers. +# +class LangOverride < Asciidoctor::Extensions::BlockMacroProcessor + use_dsl + named :lang_override + name_positional_attributes :override + def process(parent, _target, attrs) + Asciidoctor::Block.new(parent, :pass, :source => "// #{attrs[:override]}") + end +end diff --git a/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb b/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb index ec53f011f16d5..3fddb9ccc4406 100644 --- a/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb +++ b/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb @@ -5,6 +5,7 @@ require 'elastic_compat_preprocessor/extension' require 'elastic_compat_tree_processor/extension' require 'elastic_include_tagged/extension' +require 'lang_override/extension' require 'open_in_widget/extension' require 'shared_examples/does_not_break_line_numbers' @@ -13,6 +14,7 @@ Asciidoctor::Extensions.register CareAdmonition Asciidoctor::Extensions.register ChangeAdmonition Asciidoctor::Extensions.register do + block_macro LangOverride preprocessor ElasticCompatPreprocessor include_processor ElasticIncludeTagged treeprocessor ElasticCompatTreeProcessor diff --git a/resources/asciidoctor/spec/elastic_compat_tree_processor_spec.rb b/resources/asciidoctor/spec/elastic_compat_tree_processor_spec.rb index 98efeb0a9c5fc..6cde69688804f 100644 --- a/resources/asciidoctor/spec/elastic_compat_tree_processor_spec.rb +++ b/resources/asciidoctor/spec/elastic_compat_tree_processor_spec.rb @@ -1,11 +1,13 @@ # frozen_string_literal: true require 'elastic_compat_tree_processor/extension' +require 'lang_override/extension' RSpec.describe ElasticCompatTreeProcessor do before(:each) do Asciidoctor::Extensions.register do treeprocessor ElasticCompatTreeProcessor + block_macro LangOverride end end @@ -72,51 +74,82 @@ expect(actual).to eq(expected.strip) end - [ - %w[CONSOLE console], - %w[AUTOSENSE sense], - %w[KIBANA kibana], - %w[SENSE:path/to/snippet.sense sense], - ].each do |command, lang| - it "transforms legacy // #{command} commands into the #{lang} language" do - actual = convert <<~ASCIIDOC - == Example - [source,js] - ---- - GET / - ---- - pass:[// #{command}] - ASCIIDOC - expected = <<~DOCBOOK - - Example - GET / - - DOCBOOK - expect(actual).to eq(expected.strip) - end - end + shared_examples 'snippet language' do |override, lang| + name = override ? " the #{override} lang override" : 'out a lang override' + context "for a snippet with#{name}" do + let(:snippet) do + snippet = <<~ASCIIDOC + [source,js] + ---- + GET / <1> + ---- + ASCIIDOC + snippet += "lang_override::[#{override}]" if override + snippet + end + let(:has_lang) do + // + end + shared_examples 'has the expected language' do + it "has the #{lang} language" do + expect(converted).to match(has_lang) + end + end + context 'when it is alone' do + let(:converted) do + convert <<~ASCIIDOC + == Example + #{snippet} + ASCIIDOC + end + include_examples 'has the expected language' + end + context 'when it is followed by a paragraph' do + let(:converted) do + convert <<~ASCIIDOC + == Example + #{snippet} - context 'a snippet is inside of a definition list' do - let(:converted) do - convert <<~ASCIIDOC - == Example - Term:: - Definition - + - -- - [source,js] - ---- - GET / - ---- - -- - ASCIIDOC - end - let(:has_original_language) do - %r{GET /} - end - it "doesn't break" do - expect(converted).to match(has_original_language) + Words words words. + ASCIIDOC + end + include_examples 'has the expected language' + it "the paragraph is intact" do + expect(converted).to match(%r{Words words words.}) + end + end + context 'when it is inside a definition list' do + let(:converted) do + convert <<~ASCIIDOC + == Example + Term:: + Definition + + + -- + #{snippet} + -- + ASCIIDOC + end + include_examples 'has the expected language' + end + context 'when it is followed by a callout list' do + let(:converted) do + convert <<~ASCIIDOC + == Example + #{snippet} + <1> foo + ASCIIDOC + end + include_examples 'has the expected language' + it "has a working callout list" do + expect(converted).to match(/\nfoo/) + end + end end end + include_examples 'snippet language', 'CONSOLE', 'console' + include_examples 'snippet language', 'AUTOSENSE', 'sense' + include_examples 'snippet language', 'KIBANA', 'kibana' + include_examples 'snippet language', 'SENSE:path/to/snippet.sense', 'sense' + include_examples 'snippet language', nil, 'js' end From 96bc6625d6cad23633aca816c004e064fb8a388f Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 15 Mar 2019 14:46:29 -0400 Subject: [PATCH 2/5] Longer name --- .../lib/elastic_compat_tree_processor/extension.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/resources/asciidoctor/lib/elastic_compat_tree_processor/extension.rb b/resources/asciidoctor/lib/elastic_compat_tree_processor/extension.rb index 41eb6ba49ebd7..e03ef0c513052 100644 --- a/resources/asciidoctor/lib/elastic_compat_tree_processor/extension.rb +++ b/resources/asciidoctor/lib/elastic_compat_tree_processor/extension.rb @@ -43,6 +43,8 @@ class ElasticCompatTreeProcessor < TreeProcessorScaffold include Asciidoctor::Logging + LANG_OVERRIDE_RX = %r{^//\s*([^:\]]+)(?::\s*([^\]]+))?$} + def process_block(block) return unless block.context == :listing && block.style == 'source' @@ -76,11 +78,11 @@ def process_lang_override(block) next_block = block.parent.blocks[my_index + 1] return unless next_block && next_block.context == :pass - m = %r{^//\s*([^:\]]+)(?::\s*([^\]]+))?$}.match(next_block.source) - return unless m + match = LANG_OVERRIDE_RX.match(next_block.source) + return unless match - lang = LANG_MAPPING[m[1]] - snippet = m[2] + lang = LANG_MAPPING[match[1]] + snippet = match[2] return unless lang # Not a language we handle block.set_attr 'language', lang From 97f882c5f2640d15ce7add99921a3f3f3f60b9dc Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 15 Mar 2019 15:39:14 -0400 Subject: [PATCH 3/5] More tests --- .../elastic_compat_preprocessor/extension.rb | 2 +- .../spec/elastic_compat_preprocessor_spec.rb | 114 +++++++++++------- 2 files changed, 73 insertions(+), 43 deletions(-) diff --git a/resources/asciidoctor/lib/elastic_compat_preprocessor/extension.rb b/resources/asciidoctor/lib/elastic_compat_preprocessor/extension.rb index fa1c3d89c6cad..9d80a539d3709 100644 --- a/resources/asciidoctor/lib/elastic_compat_preprocessor/extension.rb +++ b/resources/asciidoctor/lib/elastic_compat_preprocessor/extension.rb @@ -113,7 +113,7 @@ class ElasticCompatPreprocessor < Asciidoctor::Extensions::Preprocessor INCLUDE_TAGGED_DIRECTIVE_RX = /^include-tagged::([^\[][^\[]*)\[(#{Asciidoctor::CC_ANY}+)?\]$/ SOURCE_WITH_SUBS_RX = /^\["source", ?"[^"]+", ?subs="(#{Asciidoctor::CC_ANY}+)"\]$/ CODE_BLOCK_RX = /^-----*$/ - SNIPPET_RX = %r{^//\s*(AUTOSENSE|KIBANA|CONSOLE|SENSE:[^\n<]+)$} # NOCOMMIT handle trailing spaces + SNIPPET_RX = %r{^//\s*(AUTOSENSE|KIBANA|CONSOLE|SENSE:[^\n<]+)$} LEGACY_MACROS = 'added|beta|coming|deprecated|experimental' LEGACY_BLOCK_MACRO_RX = /^(#{LEGACY_MACROS})\[([^\]]*)\]/ LEGACY_INLINE_MACRO_RX = /(#{LEGACY_MACROS})\[([^\]]*)\]/ diff --git a/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb b/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb index 3fddb9ccc4406..46c83ce41eaec 100644 --- a/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb +++ b/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb @@ -380,53 +380,83 @@ def stub_file_opts } end - [ - %w[CONSOLE console], - %w[AUTOSENSE sense], - %w[KIBANA kibana], - ].each do |name, lang| - it "transforms #{name} comments into a listing with the #{lang} language" do - input = <<~ASCIIDOC - == Example + shared_context 'snippet conversion' do |lang, override| + let(:snippet) do + snippet = <<~ASCIIDOC [source,js] ---- - foo + GET / <1> ---- - // #{name} ASCIIDOC - expected = <<~DOCBOOK - - Example - foo - - DOCBOOK - actual = convert input, stub_file_opts, eq( - "INFO: : line 3: writing snippet snippets/1.#{lang}" - ) - expect(actual).to eq(expected.strip) + snippet += override if override + snippet + end + let(:has_lang) do + // + end + let(:input) do + <<~ASCIIDOC + == Example + #{snippet} + ASCIIDOC end end + shared_examples 'snippet language' do |override, lang, path, + expected_warnings = "INFO: : line 3: writing snippet snippets/1.#{lang}"| + name = override ? " the #{override} lang override" : 'out a lang override' + context "for a snippet with#{name}" do + let(:has_link_to_path) { %r{} } + let(:converted) do + convert input, stub_file_opts, eq(expected_warnings.strip) + end + shared_examples 'converted with override' do + it "has the #{lang} language" do + expect(converted).to match(has_lang) + end + it "have a link to the snippet" do + expect(converted).to match(has_link_to_path) + end + end + + context 'when there is a space after //' do + include_context 'snippet conversion', lang, "// #{override}" + include_examples 'converted with override' + end + context 'when there is not a space after //' do + include_context 'snippet conversion', lang, "//#{override}" + include_examples 'converted with override' + end + context 'when there is a space after the override command' do + include_context 'snippet conversion', lang, "// #{override} " + include_examples 'converted with override' + end + end + end + include_examples 'snippet language', 'CONSOLE', 'console', 'snippets/1.console' + include_examples 'snippet language', 'AUTOSENSE', 'sense', 'snippets/1.sense' + include_examples 'snippet language', 'KIBANA', 'kibana', 'snippets/1.kibana' + include_examples( + 'snippet language', + 'SENSE: snippet.sense', + 'sense', + 'snippets/snippet.sense', + <<~WARNINGS + INFO: : line 3: copying snippet /docs_build/resources/asciidoctor/spec/snippets/snippet.sense + WARN: : line 3: reading snippets from a path makes the book harder to read + WARNINGS + ) + context "for a snippet without an override" do + include_context 'snippet conversion', 'js', nil + let(:has_any_link) { / - Example - foo - - DOCBOOK - warnings = <<~WARNINGS - INFO: : line 3: copying snippet #{spec_dir}/snippets/snippet.sense - WARN: : line 3: reading snippets from a path makes the book harder to read - WARNINGS - actual = convert input, stub_file_opts, eq(warnings.strip) - expect(actual).to eq(expected.strip) -end + it "has the js language" do + expect(converted).to match(has_lang) + end + it "not have a link to any snippet" do + expect(converted).not_to match(has_any_link) + end + end end From b55d9326912b54ac467cdb147d84c117bc5fd6cb Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 15 Mar 2019 16:04:05 -0400 Subject: [PATCH 4/5] test --- .../spec/elastic_compat_preprocessor_spec.rb | 84 ++++++++++--------- 1 file changed, 46 insertions(+), 38 deletions(-) diff --git a/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb b/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb index 46c83ce41eaec..0558315a094d9 100644 --- a/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb +++ b/resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb @@ -380,7 +380,7 @@ def stub_file_opts } end - shared_context 'snippet conversion' do |lang, override| + shared_context 'general snippet' do |lang, override| let(:snippet) do snippet = <<~ASCIIDOC [source,js] @@ -401,52 +401,60 @@ def stub_file_opts ASCIIDOC end end - shared_examples 'snippet language' do |override, lang, path, - expected_warnings = "INFO: : line 3: writing snippet snippets/1.#{lang}"| - name = override ? " the #{override} lang override" : 'out a lang override' - context "for a snippet with#{name}" do - let(:has_link_to_path) { %r{} } - let(:converted) do - convert input, stub_file_opts, eq(expected_warnings.strip) + shared_examples 'linked snippet' do |override, lang, path| + let(:has_link_to_path) { %r{} } + let(:converted) do + convert input, stub_file_opts, eq(expected_warnings.strip) + end + shared_examples 'converted with override' do + it "has the #{lang} language" do + expect(converted).to match(has_lang) end - shared_examples 'converted with override' do - it "has the #{lang} language" do - expect(converted).to match(has_lang) - end - it "have a link to the snippet" do - expect(converted).to match(has_link_to_path) - end + it "have a link to the snippet" do + expect(converted).to match(has_link_to_path) end + end - context 'when there is a space after //' do - include_context 'snippet conversion', lang, "// #{override}" - include_examples 'converted with override' - end - context 'when there is not a space after //' do - include_context 'snippet conversion', lang, "//#{override}" - include_examples 'converted with override' - end - context 'when there is a space after the override command' do - include_context 'snippet conversion', lang, "// #{override} " - include_examples 'converted with override' + context 'when there is a space after //' do + include_context 'general snippet', lang, "// #{override}" + include_examples 'converted with override' + end + context 'when there is not a space after //' do + include_context 'general snippet', lang, "//#{override}" + include_examples 'converted with override' + end + context 'when there is a space after the override command' do + include_context 'general snippet', lang, "// #{override} " + include_examples 'converted with override' + end + end + shared_examples 'extracted linked snippet' do |override, lang| + context "for a snippet with the #{override} lang override" do + let(:expected_warnings) do + "INFO: : line 3: writing snippet snippets/1.#{lang}" end + include_examples 'linked snippet', override, lang, "snippets/1.#{lang}" end end - include_examples 'snippet language', 'CONSOLE', 'console', 'snippets/1.console' - include_examples 'snippet language', 'AUTOSENSE', 'sense', 'snippets/1.sense' - include_examples 'snippet language', 'KIBANA', 'kibana', 'snippets/1.kibana' - include_examples( - 'snippet language', - 'SENSE: snippet.sense', - 'sense', - 'snippets/snippet.sense', + include_examples 'extracted linked snippet', 'CONSOLE', 'console' + include_examples 'extracted linked snippet', 'AUTOSENSE', 'sense' + include_examples 'extracted linked snippet', 'KIBANA', 'kibana' + context 'for a snippet with the SENSE override pointing to a specific path' do + let(:expected_warnings) do <<~WARNINGS - INFO: : line 3: copying snippet /docs_build/resources/asciidoctor/spec/snippets/snippet.sense + INFO: : line 3: copying snippet #{spec_dir}/snippets/snippet.sense WARN: : line 3: reading snippets from a path makes the book harder to read WARNINGS - ) - context "for a snippet without an override" do - include_context 'snippet conversion', 'js', nil + end + include_examples( + 'linked snippet', + 'SENSE: snippet.sense', + 'sense', + 'snippets/snippet.sense' + ) + end + context 'for a snippet without an override' do + include_context 'general snippet', 'js', nil let(:has_any_link) { / Date: Fri, 15 Mar 2019 16:05:20 -0400 Subject: [PATCH 5/5] Words --- resources/asciidoctor/lib/lang_override/extension.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/asciidoctor/lib/lang_override/extension.rb b/resources/asciidoctor/lib/lang_override/extension.rb index ebf6066b8b6b8..59ceba50dd9ff 100644 --- a/resources/asciidoctor/lib/lang_override/extension.rb +++ b/resources/asciidoctor/lib/lang_override/extension.rb @@ -2,7 +2,7 @@ ## # Block macro that exists entirely to mark language overrides in a clean way -# without additing additional lines to the input file which would throw off +# without adding additional lines to the input file which would throw off # line numbers. # class LangOverride < Asciidoctor::Extensions::BlockMacroProcessor