Skip to content

Commit 6732289

Browse files
nik9000nik9000
authored
Asciidoctor: Make Edit Me links know about repos (#661)
The "Edit Me" link generation in Asciidoctor was basically a copy of the asciidoc implementation which was simple because it *had* to be simple. The asciidoctor implementation had trouble dealing with books built from multiple repositories because the asciidoc implementation does. The asciidoc implementation works around it by allowing files to set their `edit_url` whenever they include a book from a different repo. We had a bug the asciidoctor implementation that led to cross repo links not working that was unrelated, but it led me to notice how silly this whole thing was. This change replaces way that asciidoctor finds the edit url. We now pass in an attribute with the root of each repo and the edit me url for that root. When we want to generate the a link we pick the first repo that contains the path of the asciidoctor file and use it's associated url. This is nice because it dosen't need anything like `repo_root` and it can automatically handle the edit me links, allowing us to remove the overridden links when we shift books to asciidoctor.
1 parent 011cf93 commit 6732289

File tree

7 files changed

+176
-96
lines changed

7 files changed

+176
-96
lines changed

‎build_docs.pl

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,16 +137,43 @@ sub _guess_opts_from_file {
137137
#===================================
138138
my $index = shift;
139139

140+
my %edit_urls = ();
141+
my $doc_toplevel = _find_toplevel($index->parent);
142+
if ( $doc_toplevel ) {
143+
$Opts->{root_dir} = $doc_toplevel;
144+
my $edit_url = _guess_edit_url($doc_toplevel);
145+
@edit_urls{ $doc_toplevel } = $edit_url if $edit_url;
146+
} else {
147+
$Opts->{root_dir} = $index->parent;
148+
}
149+
for my $resource ( @{ $Opts->{resource} } ) {
150+
my $resource_toplevel = _find_toplevel($resource);
151+
next unless $resource_toplevel;
152+
153+
my $resource_edit_url = _guess_edit_url($resource_toplevel);
154+
@edit_urls{ $resource_toplevel } = $resource_edit_url if $resource_edit_url;
155+
}
156+
$Opts->{edit_urls} = { %edit_urls };
157+
}
158+
159+
#===================================
160+
sub _find_toplevel {
161+
#===================================
162+
my $docpath = shift;
163+
140164
my $original_pwd = Cwd::cwd();
141-
chdir $index->parent;
165+
chdir $docpath;
142166
my $toplevel = eval { run qw(git rev-parse --show-toplevel) };
143167
chdir $original_pwd;
144-
unless ( $toplevel ) {
145-
say "Couldn't find edit url because the document doesn't look like it is in git";
146-
$Opts->{root_dir} = $index->parent;
147-
return;
148-
}
149-
$Opts->{root_dir} = $toplevel;
168+
say "Couldn't find repo toplevel for $docpath" unless $toplevel;
169+
return $toplevel;
170+
}
171+
172+
#===================================
173+
sub _guess_edit_url {
174+
#===================================
175+
my $toplevel = shift;
176+
150177
local $ENV{GIT_DIR} = dir($toplevel)->subdir('.git');
151178
my $remotes = eval { run qw(git remote -v) } || '';
152179
if ($remotes !~ m|\s+(\S+[/:]elastic/\S+)|) {
@@ -156,7 +183,7 @@ sub _guess_opts_from_file {
156183
}
157184
my $remote = $1;
158185
my $branch = eval {run qw(git rev-parse --abbrev-ref HEAD) } || 'master';
159-
$Opts->{edit_url} = ES::Repo::edit_url_for_url_and_branch($remote, $branch);
186+
return ES::Repo::edit_url_for_url_and_branch($remote, $branch);
160187
}
161188

162189
#===================================

‎lib/ES/Book.pm

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,6 @@ sub _build_book {
227227
my $index = $self->index;
228228
my $section_title = $self->section_title($branch);
229229
my $subject = $self->subject;
230-
my $edit_url = $self->source->edit_url($branch);
231230
my $lang = $self->lang;
232231

233232
return
@@ -236,7 +235,7 @@ sub _build_book {
236235
&& !$template->md5_changed($branch_dir)
237236
&& !$source->has_changed( $self->title, $branch, $self->asciidoctor );
238237

239-
my ( $checkout, $first_path ) = $source->prepare($branch);
238+
my ( $checkout, $edit_urls, $first_path ) = $source->prepare($branch);
240239

241240
$pm->start($branch) and return;
242241
say " - Branch: $branch - Building...";
@@ -249,7 +248,7 @@ sub _build_book {
249248
$branch_dir,
250249
version => $branch,
251250
lang => $lang,
252-
edit_url => $edit_url,
251+
edit_urls => $edit_urls,
253252
root_dir => $first_path,
254253
private => $self->private,
255254
noindex => $self->noindex,
@@ -269,7 +268,7 @@ sub _build_book {
269268
$branch_dir,
270269
version => $branch,
271270
lang => $lang,
272-
edit_url => $edit_url,
271+
edit_urls => $edit_urls,
273272
root_dir => $first_path,
274273
private => $self->private,
275274
noindex => $self->noindex,

‎lib/ES/Source.pm

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,6 @@ sub first {
3636
return shift->_sources->[0];
3737
}
3838

39-
#===================================
40-
sub edit_url {
41-
#===================================
42-
my $self = shift;
43-
my $branch = shift;
44-
return $self->first->{repo}->edit_url($branch);
45-
}
46-
4739
#===================================
4840
sub has_changed {
4941
#===================================
@@ -92,19 +84,22 @@ sub prepare {
9284
my $self = shift;
9385
my $branch = shift;
9486

95-
my %entries;
96-
my $dest = Path::Class::tempdir( DIR => $self->temp_dir );
87+
my $checkout = Path::Class::tempdir( DIR => $self->temp_dir );
88+
my %edit_urls = ();
89+
my $first_path = 0;
9790

9891
# need to handle repo name here, not in Repo
9992
for my $source ( $self->_sources_for_branch($branch) ) {
10093
my $repo = $source->{repo};
10194
my $prefix = $source->{prefix};
10295
my $path = $source->{path};
96+
my $source_checkout = $checkout->subdir($prefix);
10397

104-
$repo->extract( $branch, $path, $dest->subdir($prefix) );
105-
98+
$repo->extract( $branch, $path, $source_checkout );
99+
$edit_urls{ $source_checkout->absolute } = $repo->edit_url($branch);
100+
$first_path = $source_checkout unless $first_path;
106101
}
107-
return ( $dest, $dest->subdir( $self->first->{prefix} ) );
102+
return ( $checkout, \%edit_urls, $first_path );
108103
}
109104

110105
#===================================

‎lib/ES/Toc.pm

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,12 @@ sub write {
3636
my $adoc_file = $dir->file('index.asciidoc');
3737
$adoc_file->spew( iomode => '>:utf8', $adoc );
3838

39-
build_single( $adoc_file, $dir, type => 'article', lang => $self->lang );
39+
build_single( $adoc_file, $dir,
40+
type => 'article',
41+
lang => $self->lang,
42+
root_dir => '',
43+
edit_urls => {'' => ''},
44+
);
4045
$adoc_file->remove;
4146
}
4247

‎lib/ES/Util.pm

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ sub build_chunked {
3737
my $multi = $opts{multi} || 0;
3838
my $lenient = $opts{lenient} || '';
3939
my $lang = $opts{lang} || 'en';
40-
my $edit_url = $opts{edit_url} || '';
41-
my $root_dir = $opts{root_dir} || '';
40+
my $edit_urls = $opts{edit_urls};
41+
my $root_dir = $opts{root_dir};
4242
my $section = $opts{section_title} || '';
4343
my $subject = $opts{subject} || '';
4444
my $private = $opts{private} || '';
@@ -90,11 +90,11 @@ sub build_chunked {
9090
'-d' => 'book',
9191
'-a' => 'showcomments=1',
9292
'-a' => "lang=$lang",
93-
'-a' => 'repo_root=' . $root_dir,
9493
# Use ` to delimit monospaced literals because our docs
9594
# expect that
9695
'-a' => 'compat-mode=legacy',
97-
$private ? () : ( '-a' => "edit_url=$edit_url" ),
96+
$private ? () : ( '-a' => "edit_urls=" .
97+
edit_urls_for_asciidoctor($edit_urls) ),
9898
# Disable warning on missing attributes because we have
9999
# missing attributes!
100100
# '-a' => 'attribute-missing=warn',
@@ -121,6 +121,7 @@ sub build_chunked {
121121
} or do { $output = $@; $died = 1; };
122122
}
123123
else {
124+
my $edit_url = $edit_urls->{$root_dir};
124125
eval {
125126
$output = run(
126127
'a2x', '-v', #'--keep',
@@ -172,8 +173,8 @@ sub build_single {
172173
my $version = $opts{version} || '';
173174
my $multi = $opts{multi} || 0;
174175
my $lang = $opts{lang} || 'en';
175-
my $edit_url = $opts{edit_url} || '';
176-
my $root_dir = $opts{root_dir} || '';
176+
my $edit_urls = $opts{edit_urls};
177+
my $root_dir = $opts{root_dir};
177178
my $section = $opts{section_title} || '';
178179
my $subject = $opts{subject} || '';
179180
my $private = $opts{private} || '';
@@ -222,8 +223,8 @@ sub build_single {
222223
'-d' => $type,
223224
'-a' => 'showcomments=1',
224225
'-a' => "lang=$lang",
225-
'-a' => 'repo_root=' . $root_dir,
226-
$private ? () : ( '-a' => "edit_url=$edit_url" ),
226+
$private ? () : ( '-a' => "edit_urls=" .
227+
edit_urls_for_asciidoctor($edit_urls) ),
227228
'-a' => 'asciidoc-dir=' . $asciidoc_dir,
228229
'-a' => 'resources=' . join(',', @$resources),
229230
'-a' => 'copy-callout-images=png',
@@ -250,6 +251,7 @@ sub build_single {
250251
} or do { $output = $@; $died = 1; };
251252
}
252253
else {
254+
my $edit_url = $edit_urls->{$root_dir};
253255
eval {
254256
$output = run(
255257
'a2x', '-v',
@@ -444,6 +446,17 @@ sub rawxsltopts {
444446
return @opts;
445447
}
446448

449+
#===================================
450+
sub edit_urls_for_asciidoctor {
451+
#===================================
452+
my $edit_urls = shift;
453+
454+
# We'd be better off using a csv library for this but we don't want to add
455+
# more dependencies to the pl until we go docker-only.
456+
return join("\n", map { "$_,$edit_urls->{$_}" } keys %{$edit_urls});
457+
}
458+
459+
447460
#===================================
448461
sub write_html_redirect {
449462
#===================================

‎resources/asciidoctor/lib/edit_me/extension.rb

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

3-
require 'pathname'
3+
require 'csv'
44
require_relative '../scaffold.rb'
55

66
##
@@ -12,32 +12,56 @@ class EditMe < TreeProcessorScaffold
1212

1313
def process(document)
1414
logger.error("sourcemap is required") unless document.sourcemap
15-
super if document.attributes['edit_url']
15+
edit_urls_string = document.attributes['edit_urls']
16+
return unless edit_urls_string
17+
18+
edit_urls = []
19+
CSV.parse edit_urls_string do |toplevel, url|
20+
unless toplevel
21+
logger.error message_with_context "invalid edit_urls, no toplevel"
22+
next
23+
end
24+
unless url
25+
logger.error message_with_context "invalid edit_urls, no url"
26+
next
27+
end
28+
url = url[0..-2] if url.end_with? '/'
29+
edit_urls << { toplevel: toplevel, url: url }
30+
end
31+
document.attributes['edit_urls'] = edit_urls
32+
super
1633
end
1734

1835
def process_block(block)
1936
return unless %i[preamble section floating_title].include? block.context
2037

2138
def block.title
22-
path = source_path
23-
url = @document.attributes['edit_url']
24-
url += '/' unless url.end_with?('/')
25-
repo_root = @document.attributes['repo_root']
26-
if repo_root
27-
repo_root = Pathname.new repo_root
28-
base_dir = Pathname.new @document.base_dir
29-
url += "#{base_dir.relative_path_from(repo_root)}/" unless repo_root == base_dir
39+
# || '<stdin>' allows us to not blow up when translating strings that
40+
# aren't associated with any particular file. '<stdin>' is asciidoctor's
41+
# standard name for such strings.
42+
path = source_path || '<stdin>'
43+
44+
edit_urls = @document.attributes['edit_urls']
45+
edit_url = edit_urls.find { |e| path.start_with? e[:toplevel] }
46+
unless edit_url
47+
logger.warn message_with_context "couldn't find edit url for #{path}", :source_location => source_location
48+
return super
3049
end
31-
url += path
50+
url = edit_url[:url]
51+
url += path[edit_url[:toplevel].length..-1]
3252
"#{super}<ulink role=\"edit_me\" url=\"#{url}\">Edit me</ulink>"
3353
end
3454
if block.context == :preamble
3555
def block.source_path
36-
document.source_location.path
56+
# source_location.path doesn't work for relative includes outside of
57+
# the base_dir which we use when we build books from many repos.
58+
document.source_location.file
3759
end
3860
else
3961
def block.source_path
40-
source_location.path
62+
# source_location.path doesn't work for relative includes outside of
63+
# the base_dir which we use when we build books from many repos.
64+
source_location.file
4165
end
4266
end
4367
end

0 commit comments

Comments
 (0)