Skip to content
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

Asciidoctor: Copy callout icons #570

Merged
merged 6 commits into from
Feb 4, 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
14 changes: 10 additions & 4 deletions lib/ES/Util.pm
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ sub build_chunked {
# Emulate asciidoc_dir because we use it to find shared asciidoc files
# but asciidoctor doesn't support it.
my $asciidoc_dir = dir('resources/asciidoc-8.6.8/')->absolute;
# We use the callouts from asciidoc so add it as a resource so we
# can find them
push @$resources, $asciidoc_dir;
eval {
$output = run(
'asciidoctor', '-v', '--trace',
Expand All @@ -92,7 +95,8 @@ sub build_chunked {
# missing attributes!
# '-a' => 'attribute-missing=warn',
'-a' => 'asciidoc-dir=' . $asciidoc_dir,
$resources ? ( '-a' => 'resources=' . join(',', @$resources)) : (),
'-a' => 'resources=' . join(',', @$resources),
'-a' => 'copy-callout-images=png',

Choose a reason for hiding this comment

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

Does this mean they'll always and forever have to be png? That may be totally fine, I just am asking to be sure that it is known and definitely totally fine :)

Copy link
Member Author

Choose a reason for hiding this comment

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

They are current pngs. We could change it here to whatever we have images for though.

'--destination-dir=' . $dest,
docinfo($index),
$index
Expand All @@ -107,7 +111,7 @@ sub build_chunked {
file('resources/website_chunked.xsl')->absolute,
"$dest/index.xml"
);
unlink "$dest/index.xml";
# unlink "$dest/index.xml";

Choose a reason for hiding this comment

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

Commented-out code left behind

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!

1;
} or do { $output = $@; $died = 1; };
}
Expand Down Expand Up @@ -198,7 +202,9 @@ sub build_single {
# Emulate asciidoc_dir because we use it to find shared asciidoc files
# but asciidoctor doesn't support it.
my $asciidoc_dir = dir('resources/asciidoc-8.6.8/')->absolute;

# We use the callouts from asciidoc so add it as a resource so we
# can find them
push @$resources, $asciidoc_dir;
eval {
$output = run(
'asciidoctor', '-v', '--trace',
Expand All @@ -210,7 +216,7 @@ sub build_single {
'-a' => 'repo_root=' . $root_dir,
$private ? () : ( '-a' => "edit_url=$edit_url" ),
'-a' => 'asciidoc-dir=' . $asciidoc_dir,
$resources ? ( '-a' => 'resources=' . join(',', @$resources)) : (),
'-a' => 'resources=' . join(',', @$resources),
# Disable warning on missing attributes because we have
# missing attributes!
# '-a' => 'attribute-missing=warn',
Expand Down
23 changes: 20 additions & 3 deletions resources/asciidoctor/lib/copy_images/extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@
##
# Copies images that are referenced into the same directory as the output files.
#
# It finds the images by looking in a comma separated list of directories
# defined by the `resources` attribute.
#
# It can also be configured to copy the images that number callout lists by
# setting `copy-callout-images` to the file extension of the images to copy.
#
class CopyImages < TreeProcessorScaffold
include Logging

Expand All @@ -17,9 +23,20 @@ def initialize name
end

def process_block block
return unless block.context == :image
uri = block.image_uri(block.attr 'target')
return if Helpers.uriish? uri # Skip external images
if block.context == :image
uri = block.image_uri(block.attr 'target')
return if Helpers.uriish? uri # Skip external images
copy_image block, uri
elsif (extension = block.document.attr 'copy-callout-images') &&
block.parent &&
block.parent.context == :colist
id = block.attr('coids').scan(/CO(?:\d+)-(\d+)/) {
copy_image block, "images/icons/callouts/#{$1}.#{extension}"
}
end
end

def copy_image block, uri
return unless @copied.add? uri # Skip images we've copied before
source = find_source block, uri
return unless source # Skip images we can't find
Expand Down
141 changes: 141 additions & 0 deletions resources/asciidoctor/spec/copy_images_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -201,4 +201,145 @@ def copy_attributes copied
expect(copied).to eq([])
}
end

it "copies images for callouts when requested (png)" do
copied = []
attributes = copy_attributes copied
attributes['copy-callout-images'] = 'png'
input = <<~ASCIIDOC
== Example
----
foo <1> <2>
----
<1> words
<2> words
ASCIIDOC
expected_warnings = <<~WARNINGS
INFO: <stdin>: line 5: copying #{spec_dir}/resources/copy_images/images/icons/callouts/1.png
INFO: <stdin>: line 6: copying #{spec_dir}/resources/copy_images/images/icons/callouts/2.png
WARNINGS
convert input, attributes, eq(expected_warnings.strip)
expect(copied).to eq([
["images/icons/callouts/1.png", "#{spec_dir}/resources/copy_images/images/icons/callouts/1.png"],
["images/icons/callouts/2.png", "#{spec_dir}/resources/copy_images/images/icons/callouts/2.png"],
])
end

it "copies images for callouts when requested (gif)" do
copied = []
attributes = copy_attributes copied
attributes['copy-callout-images'] = 'gif'
input = <<~ASCIIDOC
== Example
----
foo <1>
----
<1> words
ASCIIDOC
expected_warnings = <<~WARNINGS
INFO: <stdin>: line 5: copying #{spec_dir}/resources/copy_images/images/icons/callouts/1.gif
WARNINGS
convert input, attributes, eq(expected_warnings.strip)
expect(copied).to eq([
["images/icons/callouts/1.gif", "#{spec_dir}/resources/copy_images/images/icons/callouts/1.gif"],
])
end

it "has a nice error message when a callout image is missing" do

Choose a reason for hiding this comment

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

❤️ ❤️ ❤️

copied = []
attributes = copy_attributes copied
attributes['copy-callout-images'] = 'gif'
input = <<~ASCIIDOC
== Example
----
foo <1> <2>
----
<1> words
<2> words
ASCIIDOC
convert input, attributes, match(/
WARN:\ <stdin>:\ line\ 6:\ can't\ read\ image\ at\ any\ of\ \[
"#{spec_dir}\/images\/icons\/callouts\/2.gif",\s
"#{spec_dir}\/resources\/images\/icons\/callouts\/2.gif",\s
.+
"#{spec_dir}\/resources\/copy_images\/images\/icons\/callouts\/2.gif"
.+
\]/x).and(match(/INFO: <stdin>: line 5: copying #{spec_dir}\/resources\/copy_images\/images\/icons\/callouts\/1.gif/))
expect(copied).to eq([
["images/icons/callouts/1.gif", "#{spec_dir}/resources/copy_images/images/icons/callouts/1.gif"],
])
end

it "only copies callout images one time" do
copied = []
attributes = copy_attributes copied
attributes['copy-callout-images'] = 'png'
input = <<~ASCIIDOC
== Example
----
foo <1>
----
<1> words

----
foo <1>
----
<1> words
ASCIIDOC
expected_warnings = <<~WARNINGS
INFO: <stdin>: line 5: copying #{spec_dir}/resources/copy_images/images/icons/callouts/1.png
WARNINGS
convert input, attributes, eq(expected_warnings.strip)
expect(copied).to eq([
["images/icons/callouts/1.png", "#{spec_dir}/resources/copy_images/images/icons/callouts/1.png"],
])
end

it "supports callout lists with multiple callouts per item" do
# This is a *super* weird case but we have it in Elasticsearch.
# The only way I can make callout lists be for two things is by making
# blocks with callouts but only having a single callout list below both.
copied = []
attributes = copy_attributes copied
attributes['copy-callout-images'] = 'png'
input = <<~ASCIIDOC
== Example
----
foo <1>
----

----
foo <1>
----
<1> words
ASCIIDOC
expected_warnings = <<~WARNINGS
INFO: <stdin>: line 9: copying #{spec_dir}/resources/copy_images/images/icons/callouts/1.png
INFO: <stdin>: line 9: copying #{spec_dir}/resources/copy_images/images/icons/callouts/2.png
WARNINGS
convert input, attributes, eq(expected_warnings.strip)
expect(copied).to eq([
["images/icons/callouts/1.png", "#{spec_dir}/resources/copy_images/images/icons/callouts/1.png"],
["images/icons/callouts/2.png", "#{spec_dir}/resources/copy_images/images/icons/callouts/2.png"],
])
end

it "doesn't copy callout images if the extension isn't set" do

Choose a reason for hiding this comment

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

empty test body

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 catching this! I never finished it. I'll get it soon!

copied = []
attributes = copy_attributes copied
input = <<~ASCIIDOC
== Example
----
foo <1>
----
<1> words

----
foo <1>
----
<1> words
ASCIIDOC
convert input, attributes
expect(copied).to eq([])
end
end
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.