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

resolves #519 enable source-to-source navigation for inter-document xrefs #1039

Merged
merged 1 commit into from
Apr 3, 2017

Conversation

mojavelinux
Copy link
Contributor

@mojavelinux mojavelinux commented Apr 1, 2017

  • set outfilesuffix=.adoc so inter-document xref links are created between AsciiDoc files

…ment xrefs

- set outfilesuffix=.adoc so inter-document xref links are created between AsciiDoc files
@mojavelinux
Copy link
Contributor Author

mojavelinux commented Apr 1, 2017

@kivikakk As the test shows, an inter-document xref will be created from one AsciiDoc file to another, which enables navigation between files to work on GitHub.

The downside of this patch is that we assume the AsciiDoc file extension is .adoc—even though GitHub also supports .asc and .asciidoc—and yet this setting cannot be overridden by the document. We have to do this since we don't know the source filename. It would be nice if that information could be passed to the markup block.

The reason the attribute cannot be overridden stems from a limitation in Asciidoctor. Normally, you can add a @ to the end of an attribute value and the document is allow to override it. However, in the case of outfilesuffix attribute, the value always gets overridden internally (hence nullifying the assignment). Therefore, we have to set it as a "hard override" in order for it to take effect. I could address this issue Asciidoctor 1.5.6.

However, the real solution is to pass the filename to the markup block. Then, we can guarantee the right value.

@mojavelinux
Copy link
Contributor Author

To summarize, this patch will work for anyone who uses the .adoc extension (which is most users). However, inter-document xrefs will not work for anyone who does not use the .adoc extension. (However, consider the fact that most inter-document xrefs are broken right now because they are linking to .html files).

@kivikakk
Copy link
Contributor

kivikakk commented Apr 3, 2017

Looks great. 👍 Thank you!

@kivikakk kivikakk merged commit 4998caa into github:master Apr 3, 2017
@kivikakk
Copy link
Contributor

kivikakk commented Apr 3, 2017

I'm going to take some time reading the 1.5.2...1.5.5 compare, then I'll push a new version of the gem and start the process internally to get this live.

@mojavelinux
Copy link
Contributor Author

Keep in mind that GitHub is already running 1.5.5. The Gemfile (which is only used when running the test suite for this gem) was just out of date.

@mojavelinux
Copy link
Contributor Author

I have decided that I'm going to introduce the attribute relfilesuffix in 1.5.6 so we can more easily manage the interdocument xrefs. After 1.5.6 is released, I'll send a new PR to switch to that new attribute.

@mojavelinux
Copy link
Contributor Author

Any thoughts about passing the filename to the markup block? I think it's really important for the converter to know the name of the file of the content its converting. It will be useful for more than just the AsciiDoc converter.

@mojavelinux mojavelinux deleted the issue-519 branch April 3, 2017 02:07
@kivikakk
Copy link
Contributor

kivikakk commented Apr 3, 2017

Good point re: 1.5.5. As for passing the filename, I'd be happy to do that! I'll do that now in master, and add it as a second argument for GemImplementation.

@mojavelinux
Copy link
Contributor Author

Great! Thank you!

@mojavelinux
Copy link
Contributor Author

With that information, I can make the update to this new feature so that we have source-to-source navigation that respects the file extension the author chooses.

@mojavelinux
Copy link
Contributor Author

I'll send a follow-up PR.

@kivikakk
Copy link
Contributor

kivikakk commented Apr 3, 2017

That sounds great. The change is now in master at 9fe0536, so I'll wait for the PR and then release markup 1.6.0.

@mojavelinux
Copy link
Contributor Author

Great! I'm on it!

@mojavelinux
Copy link
Contributor Author

Here you go! #1041

Now that we have the filename, I can also set the docname and docfilesuffix attributes. This are mostly just informational, but could prove useful for the author.

@kivikakk
Copy link
Contributor

kivikakk commented Apr 3, 2017

Awesome! I'm in favour of eagerly providing more information to document authors just in case they can make use of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants