Skip to content

Asciidoctor: BWC for // CONSOLE then callouts #721

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions README.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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]
----------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<]+)$}
LEGACY_MACROS = 'added|beta|coming|deprecated|experimental'
LEGACY_BLOCK_MACRO_RX = /^(#{LEGACY_MACROS})\[([^\]]*)\]/
LEGACY_INLINE_MACRO_RX = /(#{LEGACY_MACROS})\[([^\]]*)\]/
Expand Down Expand Up @@ -182,7 +182,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -74,11 +76,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

match = LANG_OVERRIDE_RX.match(next_block.source)
return unless match

lang = LANG_MAPPING[$1]
snippet = $2
lang = LANG_MAPPING[match[1]]
snippet = match[2]
return unless lang # Not a language we handle

block.set_attr 'language', lang
Expand Down
2 changes: 2 additions & 0 deletions resources/asciidoctor/lib/extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
15 changes: 15 additions & 0 deletions resources/asciidoctor/lib/lang_override/extension.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# frozen_string_literal: true

##
# Block macro that exists entirely to mark language overrides in a clean way
# without adding 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
122 changes: 81 additions & 41 deletions resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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
Expand Down Expand Up @@ -378,53 +380,91 @@ 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 'general snippet' do |lang, override|
let(:snippet) do
snippet = <<~ASCIIDOC
[source,js]
----
foo
GET / <1>
----
// #{name}
ASCIIDOC
expected = <<~DOCBOOK
<chapter id="_example">
<title>Example</title>
<programlisting language="#{lang}" linenumbering="unnumbered"><ulink type="snippet" url="snippets/1.#{lang}"/>foo</programlisting>
</chapter>
DOCBOOK
actual = convert input, stub_file_opts, eq(
snippet += override if override
snippet
end
let(:has_lang) do
/<programlisting language="#{lang}" linenumbering="unnumbered">/
end
let(:input) do
<<~ASCIIDOC
== Example
#{snippet}
ASCIIDOC
end
end
shared_examples 'linked snippet' do |override, lang, path|
let(:has_link_to_path) { %r{<ulink type="snippet" url="#{path}"/>} }
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 '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: <stdin>: line 3: writing snippet snippets/1.#{lang}"
)
expect(actual).to eq(expected.strip)
end
include_examples 'linked snippet', override, lang, "snippets/1.#{lang}"
end
end
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: <stdin>: line 3: copying snippet #{spec_dir}/snippets/snippet.sense
WARN: <stdin>: line 3: reading snippets from a path makes the book harder to read
WARNINGS
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) { /<ulink type="snippet"/ }
let(:converted) do
convert input, stub_file_opts
end

it "transforms SENSE comments into a listing with the SENSE language and a path" do
input = <<~ASCIIDOC
== Example
[source,js]
----
foo
----
// SENSE: snippet.sense
ASCIIDOC
expected = <<~DOCBOOK
<chapter id="_example">
<title>Example</title>
<programlisting language="sense" linenumbering="unnumbered"><ulink type="snippet" url="snippets/snippet.sense"/>foo</programlisting>
</chapter>
DOCBOOK
warnings = <<~WARNINGS
INFO: <stdin>: line 3: copying snippet #{spec_dir}/snippets/snippet.sense
WARN: <stdin>: 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
121 changes: 77 additions & 44 deletions resources/asciidoctor/spec/elastic_compat_tree_processor_spec.rb
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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
<chapter id="_example">
<title>Example</title>
<programlisting language="#{lang}" linenumbering="unnumbered">GET /</programlisting>
</chapter>
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you seen the #tap method in Ruby? It's a style thing, but you could use it here. I like using it so I never accidentally forget to return the object (in this case, snippet) from the let block.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#tap seems pretty complex to use here because the string is frozen. Though I bet there are other places i can use it....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, then it wouldn't work if the String is frozen..

end
let(:has_lang) do
/<programlisting language="#{lang}" linenumbering="unnumbered">/
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'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very nuanced, but you could also use it_behaves_like here. include_examples is best suited for what you're doing from the topic level - creating specs using different values but when you have a nested context and want to emphasize a behavior, people tend to use it_behaves_like. They're effectively interchangeable and I think the difference really comes down to how you want the specs to print out. Sometimes it's also awkward to find a phrase that describes the behavior. For example, in this case, it might be strange to say it_behaves_like 'a string converter' - or whatever. So anyway, maybe include_examples is best here and you can ignore this whole comment = )

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! I looked at it_behaves_like for a bunch of things. I'll play with it some more and see what I can find. I found that it doesn't work well when you want to apply the same set of shared examples multiple times with different parameters. Or, rather, it doesn't seem to properly list the parameters when you run rspec. I'll try it some more though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you're saying. Ultimately, they can accomplish very similar things in the specs, so it's up to you and how you want the the specs to run and print out.

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{<programlisting language="js" linenumbering="unnumbered">GET /</programlisting>}
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{<simpara>Words words words.</simpara>})
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(/<callout arearefs="CO1-1">\n<para>foo/)
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