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

Duplicated section IDs #192

Closed
mshibanami opened this issue May 6, 2018 · 3 comments
Closed

Duplicated section IDs #192

mshibanami opened this issue May 6, 2018 · 3 comments

Comments

@mshibanami
Copy link

Input:

== hello こんにちは

Slide 1

== hello 你好

Slide 2

Output:

<div class="reveal">
  <div class="slides">
    <section id="hello_"><h2>hello こんにちは</h2><div class="paragraph"><p>Slide 1</p></div></section>
    <section id="hello_"><h2>hello 你好</h2><div class="paragraph"><p>Slide 2</p></div></section>
  </div>
</div>

Probably hello_ and hello_2 would be better than the double hello_.

(As a workaround, I add [id=...] to the previous line of titles.)

@mshibanami mshibanami changed the title Deplicated section IDs Duplicated section IDs May 7, 2018
@mojavelinux
Copy link
Member

The converter is simplifying the id that Asciidoctor generators to work with Reveal.js. However, in doing so, it's not checking if the ID it produces has already been used. The relevant code is here: https://github.com/asciidoctor/asciidoctor-reveal.js/blob/master/templates/section.html.slim#L5

This is case where support for the ascii-ids attribute could be useful in Asciidoctor core (something AsciiDoc Python supports). I didn't really have a use case for it before, but this would be one such example.

@obilodeau
Copy link
Member

Upstream fixed hakimel/reveal.js#1230, so we can remove our filter which causes the name collision. This would avoid many of the collisions I believe.

Targeting next release for that workaround removal / fix.

@obilodeau obilodeau added this to the 2.0.0 milestone Nov 7, 2018
obilodeau added a commit to obilodeau/asciidoctor-reveal.js that referenced this issue Dec 12, 2018
Fixes asciidoctor#192 but also removes our workaround in asciidoctor#150.

Test cases from asciidoctor#192 were added.

Will impact links because the prepending of `_` is reintroduced on automatically generated links (Asciidoctor's behavior) so CHANGELOG will be explicit about that.
obilodeau added a commit to obilodeau/asciidoctor-reveal.js that referenced this issue Dec 12, 2018
@obilodeau
Copy link
Member

PR submitted: #232

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

No branches or pull requests

3 participants