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

Upgrade Asciidoctor #1258

Closed
ghost opened this issue Feb 20, 2019 · 24 comments
Closed

Upgrade Asciidoctor #1258

ghost opened this issue Feb 20, 2019 · 24 comments

Comments

@ghost
Copy link

ghost commented Feb 20, 2019

looks like current version is 1.5.6.1 (Jul 2017):

markup/Gemfile

Line 14 in 8f436ff

gem "asciidoctor", "= 1.5.6.1"

since then, a few versions have dropped:

  • 1.5.6.2 (Mar 2018)
  • 1.5.7 (May 2018)
  • 1.5.7.1 (May 2018)
  • 1.5.8 (Oct 2018)

can we please get an update?

@mojavelinux
Copy link
Contributor

I would recommend waiting for 2.0.0, which is about to be released.

@ghost ghost changed the title Upgrade to Asciidoctor 1.5.8 Upgrade Asciidoctor Feb 20, 2019
@mojavelinux
Copy link
Contributor

@cup This isn't related to the Rouge integration. On GitHub, syntax highlighting is performed after the AsciiDoc is converted to HTML. That's what the html-pipeline syntax highlighter is for. It outputs HTML that can be processed by GitHub's internal HTML processing pipeline. In effect, it outputs a <pre> tags marked with the language. That pipeline happens to use Rouge downstream, but that has nothing to do with the Rouge integration in Asciidoctor.

@ghost
Copy link
Author

ghost commented Feb 21, 2019

@mojavelinux if rouge is already being used, and the asciidoctor currently used by github doesnt support rouge... then what?

where is the missing piece? how would that even work? perhaps Pygments or
highlight.js is being used?

im not saying you are wrong, but if you are right what is the workflow they are using?

@mojavelinux
Copy link
Contributor

Syntax highlighting of code blocks already works on GitHub. That syntax highlighting is performed by GitHub's (proprietary) HTML pipeline using Rouge (AFAIK) after Asciidoctor converts the AsciiDoc to HTML. Here's how it works.

Let's assume you have the following source block in your AsciiDoc file:

[source,ruby]
----
puts 'Hello, GitHub!'
----

Asciidoctor will convert that to the following HTML:

<div class="listingblock">
<div class="content">
<pre lang="ruby"><code>puts "Hello, GitHub!"</code></pre>
</div>
</div>

GitHub's pipeline will then use Rouge (AFAIK) to apply syntax highlighting. Here's what the browser sees:

<div class="highlight highlight-source-ruby"><pre><span class="pl-c1">puts</span> <span class="pl-s"><span class="pl-pds">'</span>Hello, GitHub!<span class="pl-pds">'</span></span></pre></div>

The last step is proprietary, so the exact details are unknown to me.

the asciidoctor currently used by github doesnt support rouge... then what?

What exactly are you asking about? What is it that you think doesn't work?

@mojavelinux
Copy link
Contributor

@cup The source code between <code></code> is unescaped (since it was extracted from HTML), then passed to Rouge to be highlighted (using the specified language). The result is then inserted back into the HTML document.

Rouge is not highlighting HTML. Rouge is highlighting the source code you provided. It's the (post-Asciidoctor) pipeline that is giving Rouge the source code to highlight.

@mojavelinux
Copy link
Contributor

my understanding is that rouge takes raw source code...and turns it into the final output that you presented.

That understanding is correct. And that is what is happening.

@ghost
Copy link
Author

ghost commented Feb 21, 2019

@mojavelinux that seems... convoluted.

but even if thats true (which is likely because AsciiDoc highlight does work
currently) that would mean that we dont need to wait for AsciiDoctor 2. We
could go ahead and just upgrade to 1.5.8, which fixes the callout issue i
linked above.

i didnt see you state a reason for waiting for version 2, perhaps you could name
a "bulletpoint" that would make sense waiting for?

@mojavelinux
Copy link
Contributor

that seems... convoluted.

It is what it is. I'm just reporting the facts.

I see why you may have been confused. You thought my suggestion about waiting for 2.0 had to do with the Rouge integration. By now, it should be clear that it does not.

The reason for waiting for 2.0.0 is that a) it's about to be released and b) GitHub has a very thorough process for upgrades. I won't want them to have to go through that whole process for the 1.5.8 upgrade only to have to turn around and do the same thing for 2.0.0 (which will certainly lead to delays). (Besides, I will definitely have 2.0.0 released before GitHub would have a chance to do the 1.5.8 upgrade anyway, so this is not any longer of a wait).

Although 2.0 is mostly about dropping support for outdated versions of Ruby and switching the semantic versioning, there are several key enhancements, improvements, and bug fixes included as well. As always, you can find these in the CHANGELOG. See https://github.com/asciidoctor/asciidoctor/blob/master/CHANGELOG.adoc#unreleased

@mojavelinux
Copy link
Contributor

I'll be sure to post here as soon as Asciidoctor 2.0 is out. I know many people are eager to see Asciidoctor be upgraded, and I want to make sure we land on the version that writers are expecting.

@mojavelinux
Copy link
Contributor

Asciidoctor 2.0.0 is now available. See https://github.com/asciidoctor/asciidoctor/releases/tag/v2.0.0

If you'd like me to send a PR to upgrade, just let me know and I'll submit one.

@ghost
Copy link
Author

ghost commented Mar 23, 2019

CC @kivikakk as he not subscribed

@mojavelinux
Copy link
Contributor

he not subscribed

they/them

@kivikakk
Copy link
Contributor

kivikakk commented Apr 2, 2019

@cup I don't appreciate your thumbs down on the last comment!

@mojavelinux An upgrade PR would be great! Would you expect much in the way of breakage, or do you anticipate it to be smooth? If so, full steam ahead!

@ghost
Copy link
Author

ghost commented Apr 2, 2019

@kivikakk I leave my personal opinions "at the door". I expect you and all to do
the same. This isn’t Twitter.

On the Internet, nobody knows you’re a dog.

@mojavelinux
Copy link
Contributor

@cup If you really meant that, then you wouldn't have included a pronoun at all, which really was unnecessary. The whole point of an @ mention is that you have the freedom to not use pronouns at all.

@ghost
Copy link
Author

ghost commented Apr 2, 2019

@mojavelinux youd be best to drop it as this line of discussion is completely off topic.

@mojavelinux
Copy link
Contributor

@kivikakk I'd be glad to submit one. I went to great lengths to ensure that few if any existing documents would render differently. There's always a chance of something we didn't anticipate, but we expect this to be smooth upgrade.

I have one question about logs. Asciidoctor now has a proper logger. Would it be helpful if I silenced all log messages (which are going to be content warnings like a malformed block) or do you find these useful to have in the GitHub logs? I can configure it either way. If you aren't sure, we can just leave it how it is and decide later.

@mojavelinux
Copy link
Contributor

@cup Do not threaten me (which is how I read your last comment). I will not hesitate to report you.

@ghost
Copy link
Author

ghost commented Apr 2, 2019

@mojavelinux report me if you must, but I chose my words carefully. I think all would agree that "Upgrade Asciidoctor" issue on GitHub is not the place to discuss such things as you are doing.

Perhaps you should look upon yourself as bringing about off topic discussion and take time to reflect upon that. If you took my words as a threat, then that was an error in your judgment. Cheers.

@kivikakk
Copy link
Contributor

kivikakk commented Apr 2, 2019

@mojavelinux:

@kivikakk I'd be glad to submit one. I went to great lengths to ensure that few if any existing documents would render differently. There's always a chance of something we didn't anticipate, but we expect this to be smooth upgrade.

Excellent! Please go ahead, and I'll be happy to merge.

I have one question about logs. Asciidoctor now has a proper logger. Would it be helpful if I silenced all log messages (which are going to be content warnings like a malformed block) or do you find these useful to have in the GitHub logs? I can configure it either way. If you aren't sure, we can just leave it how it is and decide later.

I'd say we leave it how it is -- I have found these kinds of log messages helpful in diagnosing markup-related issues in the past, though I should note they don't make it into any kind of stored log, only stderr (and noticed when reproducing manually at the console later).

@kivikakk
Copy link
Contributor

kivikakk commented Apr 2, 2019

@cup Please note the code of conduct for the Markup repository: https://github.com/github/markup/blob/24d25cbf924b59884caee0b3a1539202867d8cb4/CODE_OF_CONDUCT.md

a harassment-free experience for everyone, regardless of […] gender identity and expression

I have the right to have my pronouns respected, as does everyone. You're right -- this is not the place to discuss these things, and discussion will not be entered into as to the validity of a person to be addressed as requested.

Please consider this a warning from a project maintainer that you are violating the CoC, and a reminder that maintainers are responsible for upholding the CoC in order to create an environment where everyone feels welcome.

@mojavelinux
Copy link
Contributor

@kivikakk Thank you for that feedback. A PR is on the way.

@ghost
Copy link
Author

ghost commented Apr 2, 2019

@kivikakk I made my wish very clear to stay on topic. In fact I reported
@mojavelinux for his off topic discussion, here is a copy:

This user is posting off topic and inappropriate comments about gender
identity in a GitHub issue.

While these issues certainly have their place in society and are valuable
topics of discussion, having this discussion on a technical thread at GitHub
is not the place.

Since you refuse to respect my wish in this regard, here is my comment. You do
not have your pronouns listed here:

https://github.com/kivikakk

so it would be difficult for people to respect them if they dont know what they
are. Regarding harassment, see my previous sentence. Cheers

@kivikakk
Copy link
Contributor

kivikakk commented Apr 2, 2019

@cup You're being deliberately obtuse here -- you were corrected on pronouns and you decided to turn this thread into a debate about the validity of pronouns by referring to them as "personal opinions", instead of just accepting the correction and moving on.

I'm locking this thread. If this matter comes up again, you will be banned from the project.

@github github locked as too heated and limited conversation to collaborators Apr 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants