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

Asciidoctor fails with asciidoctor style cells in tables with wrongly defined columns #1460

Closed
robertpanzer opened this issue Aug 6, 2015 · 6 comments
Assignees
Labels
Milestone

Comments

@robertpanzer
Copy link
Member

When a table only defines 2 columns, but makes Asciidoctor detect 3 columns and a cell in the third column uses an asciidoctor style rendering fails with:

Caused by: org.jruby.exceptions.RaiseException: (NoMethodError) asciidoctor: FAILED: <stdin>: Failed to parse AsciiDoc source, undefined method `table' for nil:NilClass
    at RUBY.initialize(/Users/robertpanzer/dev/asciidoctorj/asciidoctorj-core/build/resources/main/gems/asciidoctor-1.5.3.dev/lib/asciidoctor/table.rb:224)
    at RUBY.close_cell(/Users/robertpanzer/dev/asciidoctorj/asciidoctorj-core/build/resources/main/gems/asciidoctor-1.5.3.dev/lib/asciidoctor/table.rb:480)
    at org.jruby.RubyInteger.upto(org/jruby/RubyInteger.java:133)

This is because in https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/table.rb#L223 the column variable is nil.

I cannot tell whether Asciidoctor should better

  • fail with a clearer error message ("Fix your column specification!") or
  • should ignore the asciidoctor style and continue rendering (In case of accepting the asciidoctor style rendering will still fail in https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/table.rb#L226 because the Cell has the column(=nil) as parent and therefore @document is also nil.
  • or should automatically add a column with default settings.
@mojavelinux
Copy link
Member

mojavelinux commented Aug 6, 2015 via email

@robertpanzer
Copy link
Member Author

Will do once I'm back at an ordinateur.
But it should be sth like this:

[cols=","]
|===
|foo |bar a| another content
|===

@mojavelinux
Copy link
Member

Thanks! That's what I was looking for. Just needed confirmation to make sure I was testing the right scenario.

@robertpanzer
Copy link
Member Author

The example I mentioned before works.
I just retried.
The smallest example that fails taken from Sean's document is:

[cols=","]
|===
3+|D
|E
a|F

Still F
|===

Removing the cols attribute or the initial 3+|D makes rendering pass again.

@mojavelinux mojavelinux added this to the v1.5.3 milestone Oct 4, 2015
@mojavelinux mojavelinux self-assigned this Oct 4, 2015
@mojavelinux mojavelinux added the bug label Oct 4, 2015
mojavelinux added a commit to mojavelinux/asciidoctor that referenced this issue Oct 10, 2015
mojavelinux added a commit to mojavelinux/asciidoctor that referenced this issue Oct 10, 2015
@mojavelinux
Copy link
Member

I think I've got it sorted out with PR #1506. If a column can't be resolved when attempting to close a cell, skip the cell with a warning. The output of the table is still very likely to be empty or missing rows, but it's better than a crash.

Asciidoctor doesn't attempt to recover a broken table if the actual number of columns (as a result of cells with colspan and rowspan) differs from the colspec.

@mojavelinux
Copy link
Member

To be more clear, I opted for "skip cell and warn". Adding a default column is not really possible with the current parsing code.

mojavelinux added a commit that referenced this issue Oct 11, 2015
resolves #1460 don't crash if colspan exceeds colspec
@mojavelinux mojavelinux removed the wip label Oct 11, 2015
ggrossetie pushed a commit to ggrossetie/asciidoctor that referenced this issue Jun 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants