Attempt to fix issue #145 - Rendering error line number offsets #149
Conversation
* libsass is faster as it is written in C * Drop Ruby dependency * Add Sass to the test site * Use libsass-python API to compile Sass files * Remove .sass/.scss files from output_dir after compilation
…of metadata. This is to fix issue mythmon#145: reStructuredText rendering errors have incorrect line numbers
|
It is probable that when you created your new branch, you didn't first check out the master branch. Most ways of creating branches create them from the current place in history, so if you are on a branch they start there. To fix this, you could make a new branch off the current master branch and cherry pick your commit on to it. You could also use |
|
Adding the erroring file to the test site is a good first step, but it doesn't really test this change. The Travis job could pass just fine without the code changes, since that output line doesn't get checked in an automated way. I'm not sure how to do it, but I'd prefer to see an automated way to see this tested. It may be possible to monitor the logging output during test runs? |
|
How do you feel about "analytically" testing this change? Given that it's only five lines of changes in wok/page.py, perhaps another set or two of eyes looking at it can agree that it will work as expected. As far as monitoring the logging output: are you suggesting incorporating some kind of unit test framework? |
|
FYI, I fixed my branch by merging in changes from upstream/master. Now this PR only shows my actual changes. Sorry for the noise, I'm slowly getting the hang of git/GitHub. :) |
|
I read the intro for Travis CI. Armed with a little knowledge, I'm now officialy dangerous. :) What do you think about enhancing bin/site-tests to optionally also do a "compare output" test? For example, add a new environment variable ("COMPARE_OUTPUT") and if set true: run "cmp" on the output of the test run to compare against some pre-defined expected output. Thoughts? |
|
I took a stab at enhancing the Travis-CI job a bit to compare wok output versus some canned expected output, see PR #151. I used "diff" instead of "cmp" so if you look at the actual Travis job log, you can get more useful info on why it failed. Note my local Wok output differs from the output of Wok running under Travis... Perhaps requirements.txt needs to be updated, or some environmental assumptions documented? Anyway, the change is super-simple. If you like this direction, another thought is to replace "diff" with a custom script that allows for some things to be different. |
Hi, this is a simple change to wok/page.py. What it does is prepend X newline characters to the page.original document, where X is the number of lines of metadata (including the YAML delimiter). This is to fix issue #145, where rendering errors report incorrect line numbers due to the YAML header being stripped away.
It also adds a new file to the test_site: test_site/content/tests/rest_error.rst. This is a ReST document with a deliberate error to demonstrate the new functionality.
NOTE: GitHub is showing additional changes, I believe these are from a previous commit (not mine). My changes should be against master, so I'm not sure why these are showing up. Quite likely an issue with my git/GitHub usage, as I'm still learning. Open to tips/pointers.