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: Make Edit Me links know about repos #661

Merged
merged 1 commit into from
Mar 5, 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
43 changes: 35 additions & 8 deletions build_docs.pl
Original file line number Diff line number Diff line change
Expand Up @@ -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+)|) {
Expand All @@ -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);
}

#===================================
Expand Down
7 changes: 3 additions & 4 deletions lib/ES/Book.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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...";
Expand All @@ -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,
Expand All @@ -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,
Expand Down
21 changes: 8 additions & 13 deletions lib/ES/Source.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
#===================================
Expand Down Expand Up @@ -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 );
}

#===================================
Expand Down
7 changes: 6 additions & 1 deletion lib/ES/Toc.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
29 changes: 21 additions & 8 deletions lib/ES/Util.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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} || '';
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand Down Expand Up @@ -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} || '';
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand Down Expand Up @@ -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 {
#===================================
Expand Down
50 changes: 37 additions & 13 deletions resources/asciidoctor/lib/edit_me/extension.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# frozen_string_literal: true

require 'pathname'
require 'csv'
require_relative '../scaffold.rb'

##
Expand All @@ -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? '/'
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you are mutating url anyway, you could:

url.chomp!('/')

Not a change request, just a "fun fact".

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
# || '<stdin>' allows us to not blow up when translating strings that
# aren't associated with any particular file. '<stdin>' is asciidoctor's
# standard name for such strings.
path = source_path || '<stdin>'

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Really happy to see all these new log messages.

end
url += path
url = edit_url[:url]
url += path[edit_url[:toplevel].length..-1]
"#{super}<ulink role=\"edit_me\" url=\"#{url}\">Edit me</ulink>"
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
Expand Down
Loading