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

Parsing | causes location to be off by one for all following elements #129

Closed
mivok opened this issue Jun 1, 2014 · 3 comments
Closed

Parsing | causes location to be off by one for all following elements #129

mivok opened this issue Jun 1, 2014 · 3 comments

Comments

@mivok
Copy link

@mivok mivok commented Jun 1, 2014

If you parse the following:

`|`

(similar variations work also, such as foo | foo, but this is the simplest), then the location option shows one line too high. In addition, all subsequently parsed elements also have their locations off by one, throwing line numbers off for the rest of the document.

Simple test case:

#!/usr/bin/env ruby
require 'kramdown'
puts Kramdown::Document.new('`|`').root.children.first.options[:location]

I believe this is related to a failed attempt to parse this as a table, and then somehow messing up the parser's idea of what the current line number is. It looks like the line number is being incorrectly increased/reset at this point: https://github.com/gettalong/kramdown/blob/master/lib/kramdown/parser/kramdown/table.rb#L69 but I'm still looking to see if I can identify exactly what is happening.

Edit: this is with kramdown 1.3.3

@mivok
Copy link
Author

@mivok mivok commented Jun 1, 2014

I believe what's happening here is that in the reset_env call, @src.current_line_number is called with pos (and so the point at which scanning for newlines stops) far ahead of the point where the parse_table method was called (and presumably on a new line). When this method exits, @src.pos is reset to orig_pos. However, @src.current_line_number incremented an internal counter, which doesn't get reset.

@gettalong
Copy link
Owner

@gettalong gettalong commented Jun 2, 2014

Thanks for the bug report! I have been looking into the issue and you are right, the problem is with the internal state.

@gettalong gettalong closed this in 37b56b0 Jun 2, 2014
@mivok
Copy link
Author

@mivok mivok commented Jun 2, 2014

Awesome! Thanks for fixing so quickly.

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

Successfully merging a pull request may close this issue.

None yet
2 participants