diff --git a/build_docs.pl b/build_docs.pl index f1536a6774df2..304097e727bba 100755 --- a/build_docs.pl +++ b/build_docs.pl @@ -137,16 +137,43 @@ sub _guess_opts_from_file { #=================================== my $index = shift; + my %edit_urls = (); + my $doc_toplevel = _find_toplevel($index->parent); + if ( $doc_toplevel ) { + $Opts->{root_dir} = $doc_toplevel; + my $edit_url = _guess_edit_url($doc_toplevel); + @edit_urls{ $doc_toplevel } = $edit_url if $edit_url; + } else { + $Opts->{root_dir} = $index->parent; + } + for my $resource ( @{ $Opts->{resource} } ) { + my $resource_toplevel = _find_toplevel($resource); + next unless $resource_toplevel; + + my $resource_edit_url = _guess_edit_url($resource_toplevel); + @edit_urls{ $resource_toplevel } = $resource_edit_url if $resource_edit_url; + } + $Opts->{edit_urls} = { %edit_urls }; +} + +#=================================== +sub _find_toplevel { +#=================================== + my $docpath = shift; + my $original_pwd = Cwd::cwd(); - chdir $index->parent; + chdir $docpath; my $toplevel = eval { run qw(git rev-parse --show-toplevel) }; chdir $original_pwd; - unless ( $toplevel ) { - say "Couldn't find edit url because the document doesn't look like it is in git"; - $Opts->{root_dir} = $index->parent; - return; - } - $Opts->{root_dir} = $toplevel; + say "Couldn't find repo toplevel for $docpath" unless $toplevel; + return $toplevel; +} + +#=================================== +sub _guess_edit_url { +#=================================== + my $toplevel = shift; + local $ENV{GIT_DIR} = dir($toplevel)->subdir('.git'); my $remotes = eval { run qw(git remote -v) } || ''; if ($remotes !~ m|\s+(\S+[/:]elastic/\S+)|) { @@ -156,7 +183,7 @@ sub _guess_opts_from_file { } my $remote = $1; my $branch = eval {run qw(git rev-parse --abbrev-ref HEAD) } || 'master'; - $Opts->{edit_url} = ES::Repo::edit_url_for_url_and_branch($remote, $branch); + return ES::Repo::edit_url_for_url_and_branch($remote, $branch); } #=================================== diff --git a/lib/ES/Book.pm b/lib/ES/Book.pm index 4c7a35bce2b1d..9f7871decfe02 100644 --- a/lib/ES/Book.pm +++ b/lib/ES/Book.pm @@ -227,7 +227,6 @@ sub _build_book { my $index = $self->index; my $section_title = $self->section_title($branch); my $subject = $self->subject; - my $edit_url = $self->source->edit_url($branch); my $lang = $self->lang; return @@ -236,7 +235,7 @@ sub _build_book { && !$template->md5_changed($branch_dir) && !$source->has_changed( $self->title, $branch, $self->asciidoctor ); - my ( $checkout, $first_path ) = $source->prepare($branch); + my ( $checkout, $edit_urls, $first_path ) = $source->prepare($branch); $pm->start($branch) and return; say " - Branch: $branch - Building..."; @@ -249,7 +248,7 @@ sub _build_book { $branch_dir, version => $branch, lang => $lang, - edit_url => $edit_url, + edit_urls => $edit_urls, root_dir => $first_path, private => $self->private, noindex => $self->noindex, @@ -269,7 +268,7 @@ sub _build_book { $branch_dir, version => $branch, lang => $lang, - edit_url => $edit_url, + edit_urls => $edit_urls, root_dir => $first_path, private => $self->private, noindex => $self->noindex, diff --git a/lib/ES/Source.pm b/lib/ES/Source.pm index 77e378835d6e1..c8b2b1c318527 100644 --- a/lib/ES/Source.pm +++ b/lib/ES/Source.pm @@ -36,14 +36,6 @@ sub first { return shift->_sources->[0]; } -#=================================== -sub edit_url { -#=================================== - my $self = shift; - my $branch = shift; - return $self->first->{repo}->edit_url($branch); -} - #=================================== sub has_changed { #=================================== @@ -92,19 +84,22 @@ sub prepare { my $self = shift; my $branch = shift; - my %entries; - my $dest = Path::Class::tempdir( DIR => $self->temp_dir ); + my $checkout = Path::Class::tempdir( DIR => $self->temp_dir ); + my %edit_urls = (); + my $first_path = 0; # need to handle repo name here, not in Repo for my $source ( $self->_sources_for_branch($branch) ) { my $repo = $source->{repo}; my $prefix = $source->{prefix}; my $path = $source->{path}; + my $source_checkout = $checkout->subdir($prefix); - $repo->extract( $branch, $path, $dest->subdir($prefix) ); - + $repo->extract( $branch, $path, $source_checkout ); + $edit_urls{ $source_checkout->absolute } = $repo->edit_url($branch); + $first_path = $source_checkout unless $first_path; } - return ( $dest, $dest->subdir( $self->first->{prefix} ) ); + return ( $checkout, \%edit_urls, $first_path ); } #=================================== diff --git a/lib/ES/Toc.pm b/lib/ES/Toc.pm index 76d940a121e88..1867ab0483593 100644 --- a/lib/ES/Toc.pm +++ b/lib/ES/Toc.pm @@ -36,7 +36,12 @@ sub write { my $adoc_file = $dir->file('index.asciidoc'); $adoc_file->spew( iomode => '>:utf8', $adoc ); - build_single( $adoc_file, $dir, type => 'article', lang => $self->lang ); + build_single( $adoc_file, $dir, + type => 'article', + lang => $self->lang, + root_dir => '', + edit_urls => {'' => ''}, + ); $adoc_file->remove; } diff --git a/lib/ES/Util.pm b/lib/ES/Util.pm index 426bd4ef5a3ed..b60c9b750230c 100644 --- a/lib/ES/Util.pm +++ b/lib/ES/Util.pm @@ -37,8 +37,8 @@ sub build_chunked { my $multi = $opts{multi} || 0; my $lenient = $opts{lenient} || ''; my $lang = $opts{lang} || 'en'; - my $edit_url = $opts{edit_url} || ''; - my $root_dir = $opts{root_dir} || ''; + my $edit_urls = $opts{edit_urls}; + my $root_dir = $opts{root_dir}; my $section = $opts{section_title} || ''; my $subject = $opts{subject} || ''; my $private = $opts{private} || ''; @@ -90,11 +90,11 @@ sub build_chunked { '-d' => 'book', '-a' => 'showcomments=1', '-a' => "lang=$lang", - '-a' => 'repo_root=' . $root_dir, # Use ` to delimit monospaced literals because our docs # expect that '-a' => 'compat-mode=legacy', - $private ? () : ( '-a' => "edit_url=$edit_url" ), + $private ? () : ( '-a' => "edit_urls=" . + edit_urls_for_asciidoctor($edit_urls) ), # Disable warning on missing attributes because we have # missing attributes! # '-a' => 'attribute-missing=warn', @@ -121,6 +121,7 @@ sub build_chunked { } or do { $output = $@; $died = 1; }; } else { + my $edit_url = $edit_urls->{$root_dir}; eval { $output = run( 'a2x', '-v', #'--keep', @@ -172,8 +173,8 @@ sub build_single { my $version = $opts{version} || ''; my $multi = $opts{multi} || 0; my $lang = $opts{lang} || 'en'; - my $edit_url = $opts{edit_url} || ''; - my $root_dir = $opts{root_dir} || ''; + my $edit_urls = $opts{edit_urls}; + my $root_dir = $opts{root_dir}; my $section = $opts{section_title} || ''; my $subject = $opts{subject} || ''; my $private = $opts{private} || ''; @@ -222,8 +223,8 @@ sub build_single { '-d' => $type, '-a' => 'showcomments=1', '-a' => "lang=$lang", - '-a' => 'repo_root=' . $root_dir, - $private ? () : ( '-a' => "edit_url=$edit_url" ), + $private ? () : ( '-a' => "edit_urls=" . + edit_urls_for_asciidoctor($edit_urls) ), '-a' => 'asciidoc-dir=' . $asciidoc_dir, '-a' => 'resources=' . join(',', @$resources), '-a' => 'copy-callout-images=png', @@ -250,6 +251,7 @@ sub build_single { } or do { $output = $@; $died = 1; }; } else { + my $edit_url = $edit_urls->{$root_dir}; eval { $output = run( 'a2x', '-v', @@ -444,6 +446,17 @@ sub rawxsltopts { return @opts; } +#=================================== +sub edit_urls_for_asciidoctor { +#=================================== + my $edit_urls = shift; + + # We'd be better off using a csv library for this but we don't want to add + # more dependencies to the pl until we go docker-only. + return join("\n", map { "$_,$edit_urls->{$_}" } keys %{$edit_urls}); +} + + #=================================== sub write_html_redirect { #=================================== diff --git a/resources/asciidoctor/lib/edit_me/extension.rb b/resources/asciidoctor/lib/edit_me/extension.rb index 5f8763730c110..6a27917c425a3 100644 --- a/resources/asciidoctor/lib/edit_me/extension.rb +++ b/resources/asciidoctor/lib/edit_me/extension.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'pathname' +require 'csv' require_relative '../scaffold.rb' ## @@ -12,32 +12,56 @@ class EditMe < TreeProcessorScaffold def process(document) logger.error("sourcemap is required") unless document.sourcemap - super if document.attributes['edit_url'] + edit_urls_string = document.attributes['edit_urls'] + return unless edit_urls_string + + edit_urls = [] + CSV.parse edit_urls_string do |toplevel, url| + unless toplevel + logger.error message_with_context "invalid edit_urls, no toplevel" + next + end + unless url + logger.error message_with_context "invalid edit_urls, no url" + next + end + url = url[0..-2] if url.end_with? '/' + edit_urls << { toplevel: toplevel, url: url } + end + document.attributes['edit_urls'] = edit_urls + super end def process_block(block) return unless %i[preamble section floating_title].include? block.context def block.title - path = source_path - url = @document.attributes['edit_url'] - url += '/' unless url.end_with?('/') - repo_root = @document.attributes['repo_root'] - if repo_root - repo_root = Pathname.new repo_root - base_dir = Pathname.new @document.base_dir - url += "#{base_dir.relative_path_from(repo_root)}/" unless repo_root == base_dir + # || '' allows us to not blow up when translating strings that + # aren't associated with any particular file. '' is asciidoctor's + # standard name for such strings. + path = source_path || '' + + edit_urls = @document.attributes['edit_urls'] + edit_url = edit_urls.find { |e| path.start_with? e[:toplevel] } + unless edit_url + logger.warn message_with_context "couldn't find edit url for #{path}", :source_location => source_location + return super end - url += path + url = edit_url[:url] + url += path[edit_url[:toplevel].length..-1] "#{super}Edit me" end if block.context == :preamble def block.source_path - document.source_location.path + # source_location.path doesn't work for relative includes outside of + # the base_dir which we use when we build books from many repos. + document.source_location.file end else def block.source_path - source_location.path + # source_location.path doesn't work for relative includes outside of + # the base_dir which we use when we build books from many repos. + source_location.file end end end diff --git a/resources/asciidoctor/spec/edit_me_spec.rb b/resources/asciidoctor/spec/edit_me_spec.rb index 422047a502c00..2f617614c3170 100644 --- a/resources/asciidoctor/spec/edit_me_spec.rb +++ b/resources/asciidoctor/spec/edit_me_spec.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require 'pathname' require 'edit_me/extension' RSpec.describe EditMe do @@ -13,9 +14,33 @@ Asciidoctor::Extensions.unregister_all end + spec_dir = File.dirname(__FILE__) + + it "has a nice error message if you are missing the edit url" do + attributes = { + 'edit_urls' => '', + } + warnings = <<~WARNINGS + ERROR: invalid edit_urls, no url + WARN: couldn't find edit url for + WARNINGS + convert 'Words.', attributes, eq(warnings.strip) + end + + it "has a nice error message if you are missing the toplevel" do + attributes = { + 'edit_urls' => ',http://example.com', + } + warnings = <<~WARNINGS + ERROR: invalid edit_urls, no toplevel + WARN: couldn't find edit url for + WARNINGS + convert 'Words.', attributes, eq(warnings.strip) + end + it "adds a link to the preface" do attributes = { - 'edit_url' => 'www.example.com/docs/', + 'edit_urls' => ',www.example.com/docs', } input = <<~ASCIIDOC :preface-title: Preface @@ -23,7 +48,7 @@ ASCIIDOC expected = <<~DOCBOOK - Preface<ulink role="edit_me" url="www.example.com/docs/<stdin>">Edit me</ulink> + Preface<ulink role="edit_me" url="www.example.com/docs">Edit me</ulink> Words. DOCBOOK @@ -46,7 +71,7 @@ it "adds a link to each chapter title" do attributes = { - 'edit_url' => 'www.example.com/docs/', + 'edit_urls' => "#{spec_dir},www.example.com/docs", } input = <<~ASCIIDOC include::resources/edit_me/chapter1.adoc[] @@ -66,44 +91,6 @@ expect(convert input, attributes).to eq(expected.strip) end - it "respects the repo_root attribute" do - attributes = { - 'edit_url' => 'www.example.com/docs/', - 'repo_root' => File.dirname(File.dirname(__FILE__)), - } - input = <<~ASCIIDOC - include::resources/edit_me/chapter1.adoc[] - - include::resources/edit_me/chapter2.adoc[] - ASCIIDOC - expect(convert input, attributes).to match(%r{ - ^.+ - url="www\.example\.com/docs/spec/resources/edit_me/chapter1\.adoc" - .+ - url="www\.example\.com/docs/spec/resources/edit_me/chapter2\.adoc" - .+$ - }xm) - end - - it "doesn't add extra path segments if repo_root is the base_dir" do - attributes = { - 'edit_url' => 'www.example.com/docs/', - 'repo_root' => File.dirname(__FILE__), - } - input = <<~ASCIIDOC - include::resources/edit_me/chapter1.adoc[] - - include::resources/edit_me/chapter2.adoc[] - ASCIIDOC - expect(convert input, attributes).to match(%r{ - ^.+ - url="www\.example\.com/docs/resources/edit_me/chapter1\.adoc" - .+ - url="www\.example\.com/docs/resources/edit_me/chapter2\.adoc" - .+$ - }xm) - end - it "does not add a link to each chapter title if edit_link is not set" do input = <<~ASCIIDOC include::resources/edit_me/chapter1.adoc[] @@ -125,7 +112,7 @@ it "adds a link to each section title" do attributes = { - 'edit_url' => 'www.example.com/docs/', + 'edit_urls' => "#{spec_dir},www.example.com/docs/", } input = <<~ASCIIDOC include::resources/edit_me/section1.adoc[] @@ -166,7 +153,7 @@ it "adds a link to each appendix title" do attributes = { - 'edit_url' => 'www.example.com/docs/', + 'edit_urls' => "#{spec_dir},www.example.com/docs", } input = <<~ASCIIDOC include::resources/edit_me/appendix1.adoc[] @@ -207,7 +194,7 @@ it "adds a link to each glossary title" do attributes = { - 'edit_url' => 'www.example.com/docs/', + 'edit_urls' => "#{spec_dir},www.example.com/docs", } input = <<~ASCIIDOC include::resources/edit_me/glossary1.adoc[] @@ -248,7 +235,7 @@ it "adds a link to each bibliography title" do attributes = { - 'edit_url' => 'www.example.com/docs/', + 'edit_urls' => "#{spec_dir},www.example.com/docs", } input = <<~ASCIIDOC include::resources/edit_me/bibliography1.adoc[] @@ -289,7 +276,7 @@ it "adds a link to each dedication title" do attributes = { - 'edit_url' => 'www.example.com/docs/', + 'edit_urls' => "#{spec_dir},www.example.com/docs", } input = <<~ASCIIDOC include::resources/edit_me/dedication1.adoc[] @@ -330,7 +317,7 @@ it "adds a link to each colophon title" do attributes = { - 'edit_url' => 'www.example.com/docs/', + 'edit_urls' => "#{spec_dir},www.example.com/docs", } input = <<~ASCIIDOC include::resources/edit_me/colophon1.adoc[] @@ -370,8 +357,12 @@ end it "adds a link to each floating title" do + edit_urls = <<~CSV + ,www.example.com/stdin + #{spec_dir},www.example.com/docs + CSV attributes = { - 'edit_url' => 'www.example.com/docs/', + 'edit_urls' => edit_urls, } input = <<~ASCIIDOC == Chapter @@ -382,7 +373,7 @@ ASCIIDOC expected = <<~DOCBOOK - Chapter<ulink role="edit_me" url="www.example.com/docs/<stdin>">Edit me</ulink> + Chapter<ulink role="edit_me" url="www.example.com/stdin">Edit me</ulink> Float 1Edit me Words. Float 2Edit me @@ -411,4 +402,30 @@ DOCBOOK expect(convert input).to eq(expected.strip) end + + it "can handle multiple edit urls" do + edit_urls = <<~CSV + #{spec_dir}/resources/edit_me/chapter1.adoc,www.example.com/1 + #{spec_dir}/resources/edit_me/chapter2.adoc,www.example.com/2 + CSV + attributes = { + 'edit_urls' => edit_urls, + } + input = <<~ASCIIDOC + include::resources/edit_me/chapter1.adoc[] + + include::resources/edit_me/chapter2.adoc[] + ASCIIDOC + expected = <<~DOCBOOK + + Chapter 1<ulink role="edit_me" url="www.example.com/1">Edit me</ulink> + Words. + + + Chapter 2<ulink role="edit_me" url="www.example.com/2">Edit me</ulink> + Words. + + DOCBOOK + expect(convert input, attributes).to eq(expected.strip) + end end