Fix rt 128221 #353
Fix rt 128221 #353
Conversation
+ RT #124403 - Table parsing broken in Rakudo + RT #128221 - Weird internal error when parsing some very simple pod... + RT #129862 - Pod::To::Text failing on uneven row lengths + RT #132341 - table handling should ensure table rows have the same number of cells... + RT #132348 - tables should accept an inline comment (Z<some comment>) Note that this commit should only be used with Rakudo after incorporating its PR #1240.
| # test fix for RT #132341 | ||
| # also tests fix for RT #129862 | ||
| =table | ||
| X O |
There was a problem hiding this comment.
These lines originally had trailing whitespace. Was trailing whitespace here part of the test, or is it irrelevant (in your version it is removed)?
There was a problem hiding this comment.
Whitespace, or the lack of it, no longer matter due to more robust code. But having a test with and without WS would be cool if there is a way to reliably assure keeping any trailing whitespace from being stripped.
There was a problem hiding this comment.
I think it could be done by the code creating a temp file that includes the trailing spaces, and running it as a sub-process.
S26-documentation/07a-tables.t
Outdated
| @@ -88,6 +88,7 @@ is $r.contents[1].elems, 3; | |||
| is $r.contents[2].elems, 3; | |||
|
|
|||
| # test fix for RT #132341 | |||
| # also tests fix for RT #129862 | |||
There was a problem hiding this comment.
what do you mean by that? The test was passing before, so what changed? Was this line intended to be here? If yes, then ok.
There was a problem hiding this comment.
RT #129862 was fixed by a downstream change to Pod::To::Text. The new NQP Pod changes fix that original problem at the source in the Pod code.
S26-documentation/07a-tables.t
Outdated
| is $r.contents.elems, 1; | ||
| is $r.contents[0][0], "-r0c0"; # <= note leading hyphen which caused the original issue | ||
| is $r.contents[0][1], "r0c1"; | ||
| is $r.contents.elems, 1, "test {++$t}"; |
There was a problem hiding this comment.
I think the number is included in the test output anyway, so what are these descriptions for?
There was a problem hiding this comment.
The second two lines removed were redundant. The "test {++$t}" was left from some debugging code--it can be removed.
| is $r.headers.elems, 1; | ||
| is $r.contents.elems, 7; | ||
| is $hdrs, "-Col 1 -Col 2 _Col 3 =Col 4"; # <= note leading hyphen which caused the original issue | ||
| is @rows[0], "r0Col 1 -r0Col 2 _r0Col 3 =r0Col 4"; |
There was a problem hiding this comment.
I guess this wasn't simply removed, but moved to another file in a different commit? I see that it has changed, can you clarify?
There was a problem hiding this comment.
The original skipped test was written by Zoffix early on. The tests were not actually valid due probably to our lack of knowledge of interrogating pod blocks. The tests were corrected.
|
OK let's merge for now, but I hope there'll be a followup for trailing whitespace tests and whatever else you notice. |
|
On Tue, Nov 28, 2017 at 10:31 AM, Aleks-Daniel Jakimenko-Aleksejev ***@***.***> wrote:
OK let's merge for now, but I hope there'll be a followup for trailing whitespace tests and whatever else you notice.
Will do--thanks, @AlexDaniel!
|
Provides new and updated test for Rakudo PR #1240.