Skip to content

Dottydoc crashes when upgrading liqp to 0.7.2 #3859

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

Closed
smarter opened this issue Jan 17, 2018 · 10 comments · Fixed by #14238
Closed

Dottydoc crashes when upgrading liqp to 0.7.2 #3859

smarter opened this issue Jan 17, 2018 · 10 comments · Fixed by #14238

Comments

@smarter
Copy link
Member

smarter commented Jan 17, 2018

Dottydoc currently uses liqp 0.6.7, when upgrading to 0.7.2 (see branch https://github.com/dotty-staging/dotty/commits/upgrade/liqp-0.7.2) and running genDocs we get:

java.lang.ArrayIndexOutOfBoundsException: 1
        at scala.collection.mutable.WrappedArray$ofRef.apply(WrappedArray.scala:130)
        at dotty.tools.dottydoc.staticsite.tags$RenderTitle.render(tags.scala:189)
        at dotty.tools.dottydoc.staticsite.tags$RenderTitle.render(tags.scala:166)
        at liqp.nodes.TagNode.render(TagNode.java:32)
        at liqp.tags.For.renderArray(For.java:112)
        at liqp.tags.For.render(For.java:56)
        at liqp.nodes.TagNode.render(TagNode.java:32)
        at liqp.nodes.BlockNode.render(BlockNode.java:38)
        at liqp.Template$1.call(Template.java:266)
        ... 5 more

The crash happens here: https://github.com/lampepfl/dotty/blob/517ba8cbd7c4c0cd5058a4dd1bdfce2c0fe441cf/doc-tool/src/dotty/tools/dottydoc/staticsite/tags.scala#L189 maybe @felixmulder can give some context on what this does? :)

@felixmulder
Copy link
Contributor

Looks like functionality to implement a custom tag like:

{ renderTitle | "someArg" }

@felixmulder
Copy link
Contributor

Yes, the syntax in my previous response was wrong - but the sentiment the same. It's using liqp to add some custom tags to the markup language.

It tries to get the arguments from "nodes" but I guess there has been some changes to liqp

@felixmulder
Copy link
Contributor

I'd say most likely someone is calling the tag with the wrong number of args - or liqp changed in some unexpected way

@smarter
Copy link
Member Author

smarter commented Jan 17, 2018

Apparently all params are parsed as one string now: bkiers/Liqp#46, I'll try to make a PR.

@felixmulder
Copy link
Contributor

Interesting, I wonder what prompted that changerge.

@smarter
Copy link
Member Author

smarter commented Jan 17, 2018

Apparently it's to conform with the spec of Liquid.

@smarter
Copy link
Member Author

smarter commented Jan 17, 2018

Alright, I give up for now, in 0.7.2 when we want to handle {% renderRef member.alias %}, we get just the string "member.alias" and there's no way to automatically parse that, so you have to split on ".", lookup "member" in the context, then "alias" in the resulting map, that's super annoying to do and probably not even correct if the grammar of the language allows more complicated things. We should open an issue against liqp I think. (EDIT: bkiers/Liqp#70)

@bkiers
Copy link

bkiers commented Jan 17, 2018

Interesting, I wonder what prompted that changerge.
...
Apparently it's to conform with the spec of Liquid.

True: it's the specs of Liquid that say all parameters to tags are to be treated as a single string. Me implementing them as expressions was wrong.

... and there's no way to automatically parse that

No need to parse it; evaluating is all that's needed. I posted a possible workaround how to evaluate the string/expression member.alias: bkiers/Liqp#70

@nicolasstucki
Copy link
Contributor

@abgruszecki is this issue still relevant?

@smarter
Copy link
Member Author

smarter commented Feb 12, 2021

The new scaladoc still uses liqp and it's still an old version so I'd say yes: https://github.com/lampepfl/dotty/blob/3d85473ee9706b00882f2a1419575b8f0d88487e/project/Build.scala#L1486 /cc @romanowski

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

Successfully merging a pull request may close this issue.

4 participants