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

Double quotes around URL macro prevent it from being converted #3376

Closed
mojavelinux opened this issue Aug 4, 2019 · 7 comments · Fixed by #3717
Closed

Double quotes around URL macro prevent it from being converted #3376

mojavelinux opened this issue Aug 4, 2019 · 7 comments · Fixed by #3717
Assignees
Labels
bug v2.0.11 Issues resolved in the 2.0.11 release
Milestone

Comments

@mojavelinux
Copy link
Member

mojavelinux commented Aug 4, 2019

Enclosing a URL or URL macro in double quotes prevents it from being recognized and converted. For example:

"https://asciidoctor.org[Asciidoctor]"

However, it's acceptable to put double quotes around an explicit link macro or xref macro.

"link:https://asciidoctor.org[Asciidoctor]"

"xref:index.adoc[Home]"

The behavior should be consistent.

@mojavelinux mojavelinux self-assigned this Aug 4, 2019
@mojavelinux mojavelinux added the bug label Aug 4, 2019
@mojavelinux mojavelinux added this to the v2.0.x milestone Aug 4, 2019
@mojavelinux mojavelinux added the v2.0.11 Issues resolved in the 2.0.11 release label Aug 4, 2019
@ggrossetie
Copy link
Member

ggrossetie commented Jul 4, 2020

The behavior should be consistent.

As in, we should allow double quotes around a URL or URL macro?

@mojavelinux
Copy link
Member Author

Yes.

@ggrossetie
Copy link
Member

What is the exhaustive list of allowed enclosing characters?
In my opinion, when using an implicit link, we should only support a strict list of prefixes and suffixes:

"https://asciidoctor.org" is the project page for AsciiDoc.
<https://asciidoctor.org> is the project page for AsciiDoc.
[https://asciidoctor.org] is the project page for AsciiDoc.
(https://asciidoctor.org) is the project page for AsciiDoc.
'https://asciidoctor.org' is the project page for AsciiDoc.

When we detect a valid prefix (i.e. [, (, < or " or '), we should search for the matching suffix and remove it (if found).

AsciiDoc Link (href) Comments
<https://asciidoctor.org> https://asciidoctor.org < is a valid prefix,
the last character of the target is a valid suffix >
[https://asciidoctor.org" https://asciidoctor.org" [ is a valid prefix,
] was expected as a suffix, since " does not match,
we consider this character as part of the URL
]https://asciidoctor.org[ - ] is not a valid prefix

Not sure about the second, maybe we should consider that this is not a valid link since the suffix is not in compliance with the prefix? It's more strict but might avoid some confusions.

Implementation wise, I think it might the right time to fix the following "FIXME", otherwise, the regular expression and the associated code will be hard to grasp.

# FIXME revisit! the main issue is we need different rules for implicit vs explicit

@mojavelinux
Copy link
Member Author

I'm not interested right now in considering all permutations. I want to stay focused on the inconsistency at hand.

@mojavelinux
Copy link
Member Author

What it probably should be is that the URL protocol as macro name should be treated the same way as the link macro. But there are various reasons why that is not easily supported as the code currently stands. So it may be something we look at in the spec. Until then, just allows the URL protocol to be prefixed with a quote goes a long way.

mojavelinux added a commit to mojavelinux/asciidoctor that referenced this issue Jul 16, 2020
@mojavelinux
Copy link
Member Author

Implementation wise, I think it might the right time to fix the following "FIXME", otherwise, the regular expression and the associated code will be hard to grasp.

That needs to wait until at least a minor release (and maybe even a major one).

mojavelinux added a commit to mojavelinux/asciidoctor that referenced this issue Jul 17, 2020
@mojavelinux
Copy link
Member Author

"https://asciidoctor.org"

The reason this one is not supported is because of the way substitutions are currently done in AsciiDoc. This would end up catching href attributes returned by partially converted text and would wreck havoc on the result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug v2.0.11 Issues resolved in the 2.0.11 release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants