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

Exception thrown when using render_s with asciidoc #1289

Closed
dometto opened this issue Sep 3, 2019 · 6 comments
Closed

Exception thrown when using render_s with asciidoc #1289

dometto opened this issue Sep 3, 2019 · 6 comments

Comments

@dometto
Copy link
Contributor

dometto commented Sep 3, 2019

Example:

GitHub::Markup.render_s(:asciidoc, "Test")

Error thrown:

no implicit conversion of nil into String

The problem is that the asciidoc rendering block assumes a filename is present (which is not the case when using render_s). See here.

@kivikakk
Copy link
Contributor

kivikakk commented Sep 3, 2019

Nice catch! Would you be up to submitting a PR to fix this? It might be as simple as prepending filename && before the File.basename(…) call.

@mojavelinux What's a reasonable default if no filename is given? Is passing nil here OK?

@mojavelinux
Copy link
Contributor

I would recommend setting outfilesuffix to .adoc and not setting docname or docfilesuffix.

So it might be something like:

if filename
  attributes['docname'] = File.basename(filename, (extname = File.extname(filename)))
  attributes['docfilesuffix'] = attributes['outfilesuffix'] = extname
else
  attributes['outfilesuffix'] = '.adoc'
end

Though, keep in mind that setting outfilesuffix is really specific to a source tree browser like GitHub and GitLab (where you navigate from source file to source file). If this library is used in another context, it may not make sense to set outfilesuffix.

@kivikakk
Copy link
Contributor

kivikakk commented Sep 3, 2019

If this library is used in another context, it may not make sense to set outfilesuffix.

That makes sense! So far we're assuming that is the context for markup, so I think we're clear to proceed along the suggested line.

@dometto Would you like to make the PR? If not, I'm happy to.

@dometto
Copy link
Contributor Author

dometto commented Sep 4, 2019

Yes, I'll submit a PR later today! Thanks for the swift reply both of you.

@kivikakk
Copy link
Contributor

kivikakk commented Sep 5, 2019

Thank you!

@mojavelinux
Copy link
Contributor

Thank you @dometto and @kivikakk!

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

No branches or pull requests

4 participants
@kivikakk @mojavelinux @dometto and others