Skip to content

Commit 29e8fcd

Browse files
authored
Asciidoctor: BWC for // CONSOLE then callouts (#721)
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.
1 parent 91c0aa7 commit 29e8fcd

File tree

7 files changed

+186
-102
lines changed

7 files changed

+186
-102
lines changed

README.asciidoc

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,19 +1022,9 @@ the Elaticsearch repository because it can recognize it to make tests. The
10221022
"Asciidoctor". Try it first and if it works then use it. Otherwise, use the
10231023
"AsciiDoc" way.
10241024

1025-
////
1026-
1027-
The tricks that we pull to make ascidoctor support // CONSOLE are windy
1028-
and force us to add subs=+macros when we render an asciidoc snippet.
1029-
We *don't* require that for normal snippets, just those that contain
1030-
asciidoc.
1031-
1032-
////
1033-
10341025
.Code block with CONSOLE link (AsciiDoc way)
10351026
==================================
1036-
ifdef::asciidoctor[[source,asciidoc,subs=+macros]]
1037-
ifndef::asciidoctor[[source,asciidoc]]
1027+
[source,asciidoc]
10381028
--
10391029
[source,js]
10401030
----------------------------------

resources/asciidoctor/lib/elastic_compat_preprocessor/extension.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ class ElasticCompatPreprocessor < Asciidoctor::Extensions::Preprocessor
113113
INCLUDE_TAGGED_DIRECTIVE_RX = /^include-tagged::([^\[][^\[]*)\[(#{Asciidoctor::CC_ANY}+)?\]$/
114114
SOURCE_WITH_SUBS_RX = /^\["source", ?"[^"]+", ?subs="(#{Asciidoctor::CC_ANY}+)"\]$/
115115
CODE_BLOCK_RX = /^-----*$/
116-
SNIPPET_RX = %r{//\s*(?:AUTOSENSE|KIBANA|CONSOLE|SENSE:[^\n<]+)}
116+
SNIPPET_RX = %r{^//\s*(AUTOSENSE|KIBANA|CONSOLE|SENSE:[^\n<]+)$}
117117
LEGACY_MACROS = 'added|beta|coming|deprecated|experimental'
118118
LEGACY_BLOCK_MACRO_RX = /^(#{LEGACY_MACROS})\[([^\]]*)\]/
119119
LEGACY_INLINE_MACRO_RX = /(#{LEGACY_MACROS})\[([^\]]*)\]/
@@ -182,7 +182,7 @@ def reader.process_line(line)
182182
# CONSOLE snippet. Asciidoctor really doesn't recommend this sort of
183183
# thing but we have thousands of them and it'll take us some time to
184184
# stop doing it.
185-
line&.gsub!(SNIPPET_RX, 'pass:[\0]')
185+
line&.gsub!(SNIPPET_RX, 'lang_override::[\1]')
186186
end
187187
end
188188
reader

resources/asciidoctor/lib/elastic_compat_tree_processor/extension.rb

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@
4343
class ElasticCompatTreeProcessor < TreeProcessorScaffold
4444
include Asciidoctor::Logging
4545

46+
LANG_OVERRIDE_RX = %r{^//\s*([^:\]]+)(?::\s*([^\]]+))?$}
47+
4648
def process_block(block)
4749
return unless block.context == :listing && block.style == 'source'
4850

@@ -74,11 +76,13 @@ def process_lang_override(block)
7476
return unless my_index
7577

7678
next_block = block.parent.blocks[my_index + 1]
77-
return unless next_block && next_block.context == :paragraph
78-
return unless next_block.source =~ %r{pass:\[//\s*([^:\]]+)(?::\s*([^\]]+))?\]}
79+
return unless next_block && next_block.context == :pass
80+
81+
match = LANG_OVERRIDE_RX.match(next_block.source)
82+
return unless match
7983

80-
lang = LANG_MAPPING[$1]
81-
snippet = $2
84+
lang = LANG_MAPPING[match[1]]
85+
snippet = match[2]
8286
return unless lang # Not a language we handle
8387

8488
block.set_attr 'language', lang

resources/asciidoctor/lib/extensions.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
require_relative 'elastic_compat_tree_processor/extension'
99
require_relative 'elastic_compat_preprocessor/extension'
1010
require_relative 'elastic_include_tagged/extension'
11+
require_relative 'lang_override/extension'
1112
require_relative 'open_in_widget/extension'
1213

1314
Asciidoctor::Extensions.register CareAdmonition
@@ -16,6 +17,7 @@
1617
# Enable storing the source locations so we can look at them. This is required
1718
# for EditMe to get a nice location.
1819
document.sourcemap = true
20+
block_macro LangOverride
1921
preprocessor CrampedInclude
2022
preprocessor ElasticCompatPreprocessor
2123
treeprocessor CopyImages::CopyImages
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# frozen_string_literal: true
2+
3+
##
4+
# Block macro that exists entirely to mark language overrides in a clean way
5+
# without adding additional lines to the input file which would throw off
6+
# line numbers.
7+
#
8+
class LangOverride < Asciidoctor::Extensions::BlockMacroProcessor
9+
use_dsl
10+
named :lang_override
11+
name_positional_attributes :override
12+
def process(parent, _target, attrs)
13+
Asciidoctor::Block.new(parent, :pass, :source => "// #{attrs[:override]}")
14+
end
15+
end

resources/asciidoctor/spec/elastic_compat_preprocessor_spec.rb

Lines changed: 81 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
require 'elastic_compat_preprocessor/extension'
66
require 'elastic_compat_tree_processor/extension'
77
require 'elastic_include_tagged/extension'
8+
require 'lang_override/extension'
89
require 'open_in_widget/extension'
910
require 'shared_examples/does_not_break_line_numbers'
1011

@@ -13,6 +14,7 @@
1314
Asciidoctor::Extensions.register CareAdmonition
1415
Asciidoctor::Extensions.register ChangeAdmonition
1516
Asciidoctor::Extensions.register do
17+
block_macro LangOverride
1618
preprocessor ElasticCompatPreprocessor
1719
include_processor ElasticIncludeTagged
1820
treeprocessor ElasticCompatTreeProcessor
@@ -378,53 +380,91 @@ def stub_file_opts
378380
}
379381
end
380382

381-
[
382-
%w[CONSOLE console],
383-
%w[AUTOSENSE sense],
384-
%w[KIBANA kibana],
385-
].each do |name, lang|
386-
it "transforms #{name} comments into a listing with the #{lang} language" do
387-
input = <<~ASCIIDOC
388-
== Example
383+
shared_context 'general snippet' do |lang, override|
384+
let(:snippet) do
385+
snippet = <<~ASCIIDOC
389386
[source,js]
390387
----
391-
foo
388+
GET / <1>
392389
----
393-
// #{name}
394390
ASCIIDOC
395-
expected = <<~DOCBOOK
396-
<chapter id="_example">
397-
<title>Example</title>
398-
<programlisting language="#{lang}" linenumbering="unnumbered"><ulink type="snippet" url="snippets/1.#{lang}"/>foo</programlisting>
399-
</chapter>
400-
DOCBOOK
401-
actual = convert input, stub_file_opts, eq(
391+
snippet += override if override
392+
snippet
393+
end
394+
let(:has_lang) do
395+
/<programlisting language="#{lang}" linenumbering="unnumbered">/
396+
end
397+
let(:input) do
398+
<<~ASCIIDOC
399+
== Example
400+
#{snippet}
401+
ASCIIDOC
402+
end
403+
end
404+
shared_examples 'linked snippet' do |override, lang, path|
405+
let(:has_link_to_path) { %r{<ulink type="snippet" url="#{path}"/>} }
406+
let(:converted) do
407+
convert input, stub_file_opts, eq(expected_warnings.strip)
408+
end
409+
shared_examples 'converted with override' do
410+
it "has the #{lang} language" do
411+
expect(converted).to match(has_lang)
412+
end
413+
it "have a link to the snippet" do
414+
expect(converted).to match(has_link_to_path)
415+
end
416+
end
417+
418+
context 'when there is a space after //' do
419+
include_context 'general snippet', lang, "// #{override}"
420+
include_examples 'converted with override'
421+
end
422+
context 'when there is not a space after //' do
423+
include_context 'general snippet', lang, "//#{override}"
424+
include_examples 'converted with override'
425+
end
426+
context 'when there is a space after the override command' do
427+
include_context 'general snippet', lang, "// #{override} "
428+
include_examples 'converted with override'
429+
end
430+
end
431+
shared_examples 'extracted linked snippet' do |override, lang|
432+
context "for a snippet with the #{override} lang override" do
433+
let(:expected_warnings) do
402434
"INFO: <stdin>: line 3: writing snippet snippets/1.#{lang}"
403-
)
404-
expect(actual).to eq(expected.strip)
435+
end
436+
include_examples 'linked snippet', override, lang, "snippets/1.#{lang}"
405437
end
406438
end
439+
include_examples 'extracted linked snippet', 'CONSOLE', 'console'
440+
include_examples 'extracted linked snippet', 'AUTOSENSE', 'sense'
441+
include_examples 'extracted linked snippet', 'KIBANA', 'kibana'
442+
context 'for a snippet with the SENSE override pointing to a specific path' do
443+
let(:expected_warnings) do
444+
<<~WARNINGS
445+
INFO: <stdin>: line 3: copying snippet #{spec_dir}/snippets/snippet.sense
446+
WARN: <stdin>: line 3: reading snippets from a path makes the book harder to read
447+
WARNINGS
448+
end
449+
include_examples(
450+
'linked snippet',
451+
'SENSE: snippet.sense',
452+
'sense',
453+
'snippets/snippet.sense'
454+
)
455+
end
456+
context 'for a snippet without an override' do
457+
include_context 'general snippet', 'js', nil
458+
let(:has_any_link) { /<ulink type="snippet"/ }
459+
let(:converted) do
460+
convert input, stub_file_opts
461+
end
407462

408-
it "transforms SENSE comments into a listing with the SENSE language and a path" do
409-
input = <<~ASCIIDOC
410-
== Example
411-
[source,js]
412-
----
413-
foo
414-
----
415-
// SENSE: snippet.sense
416-
ASCIIDOC
417-
expected = <<~DOCBOOK
418-
<chapter id="_example">
419-
<title>Example</title>
420-
<programlisting language="sense" linenumbering="unnumbered"><ulink type="snippet" url="snippets/snippet.sense"/>foo</programlisting>
421-
</chapter>
422-
DOCBOOK
423-
warnings = <<~WARNINGS
424-
INFO: <stdin>: line 3: copying snippet #{spec_dir}/snippets/snippet.sense
425-
WARN: <stdin>: line 3: reading snippets from a path makes the book harder to read
426-
WARNINGS
427-
actual = convert input, stub_file_opts, eq(warnings.strip)
428-
expect(actual).to eq(expected.strip)
429-
end
463+
it "has the js language" do
464+
expect(converted).to match(has_lang)
465+
end
466+
it "not have a link to any snippet" do
467+
expect(converted).not_to match(has_any_link)
468+
end
469+
end
430470
end

resources/asciidoctor/spec/elastic_compat_tree_processor_spec.rb

Lines changed: 77 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
# frozen_string_literal: true
22

33
require 'elastic_compat_tree_processor/extension'
4+
require 'lang_override/extension'
45

56
RSpec.describe ElasticCompatTreeProcessor do
67
before(:each) do
78
Asciidoctor::Extensions.register do
89
treeprocessor ElasticCompatTreeProcessor
10+
block_macro LangOverride
911
end
1012
end
1113

@@ -72,51 +74,82 @@
7274
expect(actual).to eq(expected.strip)
7375
end
7476

75-
[
76-
%w[CONSOLE console],
77-
%w[AUTOSENSE sense],
78-
%w[KIBANA kibana],
79-
%w[SENSE:path/to/snippet.sense sense],
80-
].each do |command, lang|
81-
it "transforms legacy // #{command} commands into the #{lang} language" do
82-
actual = convert <<~ASCIIDOC
83-
== Example
84-
[source,js]
85-
----
86-
GET /
87-
----
88-
pass:[// #{command}]
89-
ASCIIDOC
90-
expected = <<~DOCBOOK
91-
<chapter id="_example">
92-
<title>Example</title>
93-
<programlisting language="#{lang}" linenumbering="unnumbered">GET /</programlisting>
94-
</chapter>
95-
DOCBOOK
96-
expect(actual).to eq(expected.strip)
97-
end
98-
end
77+
shared_examples 'snippet language' do |override, lang|
78+
name = override ? " the #{override} lang override" : 'out a lang override'
79+
context "for a snippet with#{name}" do
80+
let(:snippet) do
81+
snippet = <<~ASCIIDOC
82+
[source,js]
83+
----
84+
GET / <1>
85+
----
86+
ASCIIDOC
87+
snippet += "lang_override::[#{override}]" if override
88+
snippet
89+
end
90+
let(:has_lang) do
91+
/<programlisting language="#{lang}" linenumbering="unnumbered">/
92+
end
93+
shared_examples 'has the expected language' do
94+
it "has the #{lang} language" do
95+
expect(converted).to match(has_lang)
96+
end
97+
end
98+
context 'when it is alone' do
99+
let(:converted) do
100+
convert <<~ASCIIDOC
101+
== Example
102+
#{snippet}
103+
ASCIIDOC
104+
end
105+
include_examples 'has the expected language'
106+
end
107+
context 'when it is followed by a paragraph' do
108+
let(:converted) do
109+
convert <<~ASCIIDOC
110+
== Example
111+
#{snippet}
99112
100-
context 'a snippet is inside of a definition list' do
101-
let(:converted) do
102-
convert <<~ASCIIDOC
103-
== Example
104-
Term::
105-
Definition
106-
+
107-
--
108-
[source,js]
109-
----
110-
GET /
111-
----
112-
--
113-
ASCIIDOC
114-
end
115-
let(:has_original_language) do
116-
%r{<programlisting language="js" linenumbering="unnumbered">GET /</programlisting>}
117-
end
118-
it "doesn't break" do
119-
expect(converted).to match(has_original_language)
113+
Words words words.
114+
ASCIIDOC
115+
end
116+
include_examples 'has the expected language'
117+
it "the paragraph is intact" do
118+
expect(converted).to match(%r{<simpara>Words words words.</simpara>})
119+
end
120+
end
121+
context 'when it is inside a definition list' do
122+
let(:converted) do
123+
convert <<~ASCIIDOC
124+
== Example
125+
Term::
126+
Definition
127+
+
128+
--
129+
#{snippet}
130+
--
131+
ASCIIDOC
132+
end
133+
include_examples 'has the expected language'
134+
end
135+
context 'when it is followed by a callout list' do
136+
let(:converted) do
137+
convert <<~ASCIIDOC
138+
== Example
139+
#{snippet}
140+
<1> foo
141+
ASCIIDOC
142+
end
143+
include_examples 'has the expected language'
144+
it "has a working callout list" do
145+
expect(converted).to match(/<callout arearefs="CO1-1">\n<para>foo/)
146+
end
147+
end
120148
end
121149
end
150+
include_examples 'snippet language', 'CONSOLE', 'console'
151+
include_examples 'snippet language', 'AUTOSENSE', 'sense'
152+
include_examples 'snippet language', 'KIBANA', 'kibana'
153+
include_examples 'snippet language', 'SENSE:path/to/snippet.sense', 'sense'
154+
include_examples 'snippet language', nil, 'js'
122155
end

0 commit comments

Comments
 (0)