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

>=: undefined method `>=' for nil #4046

Closed
jboxman opened this issue May 3, 2021 · 9 comments
Closed

>=: undefined method `>=' for nil #4046

jboxman opened this issue May 3, 2021 · 9 comments
Assignees
Labels
bug compliance v2.0.16 Issues resolved in the 2.0.16 release
Milestone

Comments

@jboxman
Copy link

jboxman commented May 3, 2021

This originates from:

ifeval::[{release} >= 4.5]

where {release} is not defined anywhere. It builds without any errors when using CLI (asciidoctor -B . -a toc -d book <file>), but not with JavaScript:

const asciidocOptions = {
  doctype: 'book',
  // asciidoctor: WARNING: include file is outside of jail; recovering automatically
  //safe: 'server',
  safe: 'unsafe',
  sourcemap: true,
  base_dir: repoDir,
  attributes: {
    icons: 'font',
    toc: 'macro'
  }
}
const doc = adoctor.loadFile(assembly, asciidocOptions);
doc.convert();

Is there any option that avoids the exception?

Thanks!

@mojavelinux mojavelinux transferred this issue from asciidoctor/asciidoctor.js May 3, 2021
@mojavelinux
Copy link
Member

I have transferred this issue to Asciidoctor core since the same problem exists there and is inherited by Asciidoctor.js

@mojavelinux
Copy link
Member

What this expression resolves to is:

ifeval::[nil >= 4.5]

If you try to run that in Ruby, you will get a NoMethodError.

For any operation that cannot be run, I think Asciidoctor should resolve the value to false instead of raising an exception.

@mojavelinux
Copy link
Member

There's actually a test for this scenario, but the test uses the == operator, which is defined for nil. If the test tried any other operator, it would fail. So it looks like we intended it to work this way, but the test was not robust enough.

@mojavelinux mojavelinux self-assigned this May 3, 2021
@mojavelinux mojavelinux added this to the v2.0.x milestone May 3, 2021
mojavelinux added a commit to mojavelinux/asciidoctor that referenced this issue May 3, 2021
…if comparison operation cannot be performed
@mojavelinux
Copy link
Member

The workaround is to enclose the value in quotes.

ifeval::["{release}" >= "4.5"]

However, that does slightly change the semantics of the comparison. It may work for your use case, though.

@jboxman
Copy link
Author

jboxman commented May 3, 2021

The workaround is to enclose the value in quotes.

ifeval::["{release}" >= "4.5"]

However, that does slightly change the semantics of the comparison. It may work for your use case, though.

Is it the string comparison rules for JavaScript or Ruby in effect here for the Asciidoctor.js version?

@mojavelinux
Copy link
Member

For these kinds of comparisons, the behavior seems to be the same.

"4.0" > "4.5"
false

"4.7" > "4.5"
true

@mojavelinux
Copy link
Member

Is it the string comparison rules for JavaScript or Ruby in effect here for the Asciidoctor.js version?

...and even if the behavior of JavaScript differs from Ruby, the Opal abstraction layer should be normalizing any differences.

@mojavelinux
Copy link
Member

It's pretty easy to reproduce this error even for a defined attribute:

ifeval::[{asciidoctor-version} > true]
yep
endif::[]

Generally, the policy of Asciidoctor is to not crash as a result of document contents. Therefore, it's consistent with that policy to skip the block when the comparison fails. I don't think a warning is warranted since a failed comparison is a valid outcome.

mojavelinux added a commit to mojavelinux/asciidoctor that referenced this issue May 5, 2021
…if comparison operation cannot be performed
mojavelinux added a commit to mojavelinux/asciidoctor that referenced this issue May 7, 2021
…if comparison operation cannot be performed
@jboxman
Copy link
Author

jboxman commented May 10, 2021

@mojavelinux thanks!

@mojavelinux mojavelinux added the v2.0.16 Issues resolved in the 2.0.16 release label Jun 18, 2021
mojavelinux added a commit that referenced this issue Aug 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug compliance v2.0.16 Issues resolved in the 2.0.16 release
Projects
None yet
Development

No branches or pull requests

2 participants