CommonMark.jl icon indicating copy to clipboard operation
CommonMark.jl copied to clipboard

Some detailed code issues

Open Rratic opened this issue 3 years ago • 3 comments

The code advance_offset(parser, length(parser.buf) - parser.pos + 1, false) appeared several times. I guess it means to advance to end, but parser.pos does not seem to be the current char number. And this calculation is not needed, so I guess a advance_offset_to_end(parser::Parser, columns::Bool) = advance_offset(parser, typemax(Int), columns) is needed.

Also note that you defined many parser.pos related functions, but they are not always used.

Rratic avatar Jan 17 '23 04:01 Rratic

Add: in the definition of incorporate_line(parser::Parser, ln::AbstractString), in the code elseif parser.pos ≤ length(ln) && !parser.blank, parser.pos and length does not match either.

Rratic avatar Jan 17 '23 05:01 Rratic

It isn't satisfying that none of the above solves https://github.com/MichaelHatherly/CommonMark.jl/pull/57

Add: write_markdown(::TableHeader, w, node, enter) the if block is empty. Did you forget to write something? This can't make the code look nice.

Rratic avatar Jan 17 '23 07:01 Rratic

Thanks for finding those @Rratic, feel free to open PRs if you feel comfortable addressing them, otherwise I'll have a look at them we I next have some free time.

MichaelHatherly avatar Jan 17 '23 12:01 MichaelHatherly