Asciidoctor: Make Edit Me links know about repos #661
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
I'd love to steal some of the infrastructure in #659 for an integration test for this. |
ninaspitfire
approved these changes
Mar 5, 2019
| logger.error message_with_context "invalid edit_urls, no url" | ||
| next | ||
| end | ||
| url = url[0..-2] if url.end_with? '/' |
There was a problem hiding this comment.
Since you are mutating url anyway, you could:
url.chomp!('/')
Not a change request, just a "fun fact".
| 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 |
There was a problem hiding this comment.
Really happy to see all these new log messages.
| @@ -13,17 +14,41 @@ | |||
| Asciidoctor::Extensions.unregister_all | |||
| end | |||
|
|
|||
| spec_dir = File.dirname(__FILE__) | |||
|
|
|||
| it "has a nice error message if you are missing the edit url" do | |||
There was a problem hiding this comment.
I love test names like this. 👍
Totally slays test_error_message.
There was a problem hiding this comment.
I like this about rspec a lot! "Ruby as a DSL" can have all kinds of problems with error reporting and stuff, but here it is super nice!
|
Thanks for reviewing @jarpy! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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_urlwhenever they include a book from a different repo. We had abug 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_rootandit can automatically handle the edit me links, allowing us to remove the
overridden links when we shift books to asciidoctor.