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 3 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
20 changes: 17 additions & 3 deletions resources/asciidoctor/lib/copy_images/extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,23 @@ 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
else
extension = block.attr 'copy-callout-images'
if block.parent && block.parent.context == :colist

Choose a reason for hiding this comment

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

This could be an elsif on line 24 right? There's no other in-scope usage of extension anyway here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah! This is what that empty test block was about! It looks like I never finished it. sorry!

id = block.attr('coids').scan(/CO(?:\d+)-(\d+)/) {
copy_image block, "images/icons/callouts/#{$1}.#{extension}"
}
else
return
end
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
126 changes: 126 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,130 @@ def copy_attributes copied
expect(copied).to eq([])
}
end

it "copies a images for callouts when requested (png)" do

Choose a reason for hiding this comment

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

typo: "copies a images"

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!

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 a images for callouts when requested (gif)" do

Choose a reason for hiding this comment

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

typo: "copies a images"

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!


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.