Skip to content

Commit

Permalink
Don't throw errors on RST tables in Markdown and RstMarkdown modes
Browse files Browse the repository at this point in the history
Additions to RST simple tables (nim-lang#19859) made their parsing more
restrictive, which can introduce problems with of some old
nimforum posts, which have tables with sloppily aligned columns
(like this one:
nim-lang/nimforum#330 (comment)).
Also this strictness contradicts to Markdown style of not getting
in the way (ignoring errors).

So this PR proposes a new strategy of dealing with errors:
* In Markdown and legacy (old default) RstMarkdown we try to
  continue parsing, emitting only warnings
* And only in pure RST mode we throw a error

I expect that this strategy will be applied to more parts of markup code
in the future.
  • Loading branch information
a-mr committed Jun 27, 2023
1 parent 4ce3a68 commit df2b1de
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 18 deletions.
52 changes: 39 additions & 13 deletions lib/packages/docutils/rst.nim
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,29 @@ proc rstMessage(p: RstParser, msgKind: MsgKind) =
p.col + currentTok(p).col, msgKind,
currentTok(p).symbol)

# Functions `isPureRst` & `stopOrWarn` address differences between
# Markdown and RST:
# * Markdown always tries to continue working. If it is really impossible
# to parse a markup element, its proc just returns `nil` and parsing
# continues for it as for normal text paragraph.
# The downside is that real mistakes/typos are often silently ignored.
# The same applies to legacy `RstMarkdown` mode for nimforum.
# * RST really signals errors. The downside is that it's more intrusive -
# the user must escape special syntax with \ explicitly.
#
# TODO: we need to apply this strategy to all markup elements eventually.

func isPureRst(p: RstParser): bool =
roSupportMarkdown notin p.s.options

proc stopOrWarn(p: RstParser, errorType: MsgKind, arg: string) =
let realMsgKind = if isPureRst(p): errorType else: mwRstStyle
rstMessage(p, realMsgKind, arg)

proc stopOrWarn(p: RstParser, errorType: MsgKind, arg: string, line, col: int) =
let realMsgKind = if isPureRst(p): errorType else: mwRstStyle
rstMessage(p, realMsgKind, arg, line, col)

proc currInd(p: RstParser): int =
result = p.indentStack[high(p.indentStack)]

Expand Down Expand Up @@ -2593,14 +2616,14 @@ proc getColumns(p: RstParser, cols: var RstCols, startIdx: int): int =
if p.tok[result].kind != tkAdornment: break
if p.tok[result].kind == tkIndent: inc result

proc checkColumns(p: RstParser, cols: RstCols) =
proc checkColumns(p: RstParser, cols: RstCols): bool =
var i = p.idx
if p.tok[i].symbol[0] != '=':
rstMessage(p, mwRstStyle,
stopOrWarn(p, meIllformedTable,
"only tables with `=` columns specification are allowed")
for col in 0 ..< cols.len:
if tokEnd(p, i) != cols[col].stop:
rstMessage(p, meIllformedTable,
stopOrWarn(p, meIllformedTable,
"end of table column #$1 should end at position $2" % [
$(col+1), $(cols[col].stop+ColRstOffset)],
p.tok[i].line, tokEnd(p, i))
Expand All @@ -2609,12 +2632,14 @@ proc checkColumns(p: RstParser, cols: RstCols) =
if p.tok[i].kind == tkWhite:
inc i
if p.tok[i].kind notin {tkIndent, tkEof}:
rstMessage(p, meIllformedTable, "extraneous column specification")
stopOrWarn(p, meIllformedTable, "extraneous column specification")
elif p.tok[i].kind == tkWhite:
inc i
else:
rstMessage(p, meIllformedTable, "no enough table columns",
p.tok[i].line, p.tok[i].col)
stopOrWarn(p, meIllformedTable,
"no enough table columns", p.tok[i].line, p.tok[i].col)
# return false
result = true

proc getSpans(p: RstParser, nextLine: int,
cols: RstCols, unitedCols: RstCols): seq[int] =
Expand Down Expand Up @@ -2669,17 +2694,18 @@ proc parseSimpleTableRow(p: var RstParser, cols: RstCols, colChar: char): PRstNo
if tokEnd(p) <= colEnd(nCell):
if tokStart(p) < colStart(nCell):
if currentTok(p).kind != tkWhite:
rstMessage(p, meIllformedTable,
stopOrWarn(p, meIllformedTable,
"this word crosses table column from the left")
else:
inc p.idx
row[nCell].add(currentTok(p).symbol)
else:
row[nCell].add(currentTok(p).symbol)
inc p.idx
inc p.idx
else:
if tokStart(p) < colEnd(nCell) and currentTok(p).kind != tkWhite:
rstMessage(p, meIllformedTable,
stopOrWarn(p, meIllformedTable,
"this word crosses table column from the right")
row[nCell].add(currentTok(p).symbol)
inc p.idx
inc nCell
if currentTok(p).kind == tkIndent: inc p.idx
if tokEnd(p) <= colEnd(0): break
Expand All @@ -2706,12 +2732,12 @@ proc parseSimpleTable(p: var RstParser): PRstNode =
result = newRstNodeA(p, rnTable)
let startIdx = getColumns(p, cols, p.idx)
let colChar = currentTok(p).symbol[0]
checkColumns(p, cols)
if not checkColumns(p, cols): return nil
p.idx = startIdx
result.colCount = cols.len
while true:
if currentTok(p).kind == tkAdornment:
checkColumns(p, cols)
if not checkColumns(p, cols): return nil
p.idx = tokenAfterNewline(p)
if currentTok(p).kind in {tkEof, tkIndent}:
# skip last adornment line:
Expand Down
33 changes: 28 additions & 5 deletions tests/stdlib/trst.nim
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ import os
import std/[assertions, syncio]

const preferMarkdown = {roPreferMarkdown, roSupportMarkdown, roNimFile, roSandboxDisabled}
# legacy nimforum / old default mode:
const preferRst = {roSupportMarkdown, roNimFile, roSandboxDisabled}
const pureRst = {roNimFile, roSandboxDisabled}

proc toAst(input: string,
rstOptions: RstParseOptions = preferMarkdown,
Expand Down Expand Up @@ -917,10 +919,31 @@ suite "RST tables":
====== ======
Inputs Output
====== ======
""".toAst(error=error) == "")
""".toAst(rstOptions = pureRst, error = error) == "")
check(error[] == "input(2, 2) Error: Illformed table: " &
"this word crosses table column from the right")

# In nimforum compatibility mode & Markdown we raise a warning instead:
let expected = dedent"""
rnTable colCount=2
rnTableRow
rnTableDataCell
rnLeaf 'Inputs'
rnTableDataCell
rnLeaf 'Output'
"""
for opt in [preferRst, preferMarkdown]:
var warnings = new seq[string]

check(
dedent"""
====== ======
Inputs Output
====== ======
""".toAst(rstOptions = opt, warnings = warnings) == expected)
check(warnings[] == @[
"input(2, 2) Warning: RST style: this word crosses table column from the right"])

test "tables with slightly overflowed cells cause an error (2)":
var error = new string
check("" == dedent"""
Expand All @@ -929,7 +952,7 @@ suite "RST tables":
===== ===== ======
False False False
===== ===== ======
""".toAst(error=error))
""".toAst(rstOptions = pureRst, error = error))
check(error[] == "input(2, 8) Error: Illformed table: " &
"this word crosses table column from the right")

Expand All @@ -941,7 +964,7 @@ suite "RST tables":
===== ===== ======
False False False
===== ===== ======
""".toAst(error=error))
""".toAst(rstOptions = pureRst, error = error))
check(error[] == "input(2, 7) Error: Illformed table: " &
"this word crosses table column from the left")

Expand All @@ -954,7 +977,7 @@ suite "RST tables":
===== ======
False False
===== =======
""".toAst(error=error))
""".toAst(rstOptions = pureRst, error = error))
check(error[] == "input(5, 14) Error: Illformed table: " &
"end of table column #2 should end at position 13")

Expand All @@ -966,7 +989,7 @@ suite "RST tables":
===== =======
False False
===== ======
""".toAst(error=error))
""".toAst(rstOptions = pureRst, error = error))
check(error[] == "input(3, 14) Error: Illformed table: " &
"end of table column #2 should end at position 13")

Expand Down

0 comments on commit df2b1de

Please sign in to comment.