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

doc: use asciidoctor instead of a2x #1544

Closed
wants to merge 1 commit into from

Conversation

yous
Copy link
Contributor

@yous yous commented Apr 6, 2020

Ref: Homebrew/linuxbrew-core#19885

AsciiDoc development is continued under asciidoctor. See https://github.com/asciidoc/asciidoc.

I tried to run CI several times, but on nightly build, Rust fails to find the asciidoctor binary even /usr/bin/asciidoctor exists.

Attaching rg.1.asciidoc from current build, and rg.1.asciidoctor from new build.

@yous
Copy link
Contributor Author

yous commented Apr 6, 2020

Seems that replacing { and } is no longer needed. Just updated and here is the updated rg.1.asciidoctor.

Copy link
Owner

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

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

Thanks very much! I had no idea that this was the root cause of the Homebrew issue and also had no idea that asciidoc was itself being EOL'd this year.

So one thing that I'm concerned about here is the abrupt switchover to asciidoctor. I think what I'd rather do is to use asciidoctor like you have in this PR, but fallback to a2x if asciidoctor isn't available. That will make it easy to release this change in a point release with a note that a2x support will be removed and man page generation will require asciidoctor in ripgrep 13. That will give package maintainers a bit more slack to deal with this change.

I know it's more of a pain to do that here. Let me know how you'd like to proceed. If you don't do it, then I'd be happy to take over this PR and do it myself, but I don't know when that will be. If you do want to do it, then I'd recommend just having two totally different paths between asciidoc and asciidoctor. No need to try to reuse code between them in build.rs. Then we can just lift out a2x when we're ready.

Thanks again!

@@ -1,3 +1,3 @@
#!/bin/sh

brew install asciidoc docbook-xsl
brew install asciidoctor docbook-xsl
Copy link
Owner

Choose a reason for hiding this comment

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

I wonder if docbook-xsl is still needed? We can leave it here for now and I'll experiment with it later. IIRC, asciidoc complained when this wasn't installed before. No idea if asciidoctor also needs it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's needed as when I don't have installed docbook or docbook-xsl, asciidoctor worked. How about Ubuntu packages? Are libxslt1-dev, docbook-xsl, xsltproc, libxml2-utils related only to asciidoc? I'll try running the CI without them.

@yous yous force-pushed the use-asciidoctor branch 2 times, most recently from c26623e to 79d68f0 Compare April 7, 2020 10:48
@yous
Copy link
Contributor Author

yous commented Apr 9, 2020

I renamed the original generate_man_page to legacy_generate_man_page, and added generate_man_page that uses asciidoctor. Please have a look.

@BurntSushi BurntSushi added the rollup A PR that has been merged with many others in a rollup. label May 9, 2020
Copy link
Owner

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

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

Brilliant, thank you!

BurntSushi pushed a commit that referenced this pull request May 9, 2020
AsciiDoc development is continued under asciidoctor. See
https://github.com/asciidoc/asciidoc.

We do however fallback to a2x if asciidoctor is not present. This is to
ease migration, but at some point, it's likely that support for a2x will
be dropped.

Originally reported downstream:
https://github.com/Homebrew/linuxbrew-core/issues/19885

Closes #1544
@BurntSushi BurntSushi closed this in 16a1221 May 9, 2020
@yous yous deleted the use-asciidoctor branch May 9, 2020 14:14
yous added a commit to yous/linuxbrew-core that referenced this pull request May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR that has been merged with many others in a rollup.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants