philschatz commented 7 years ago

It would be nice to output the HTML line and column number when a warning/error occurs as well as the CSS line number (maybe even use https://github.com/mattrobenolt/python-sourcemap to show the line in the original SASS file).

This would help find bad links, empty buckets, and probably a whole host of other problems during baking.

Also, if the messages are formatted as linter errors/warnings, an editor (like atom) could show the problems directly in the editor to make them easier to fix.

Would others find this useful?

/cc @helenemccarron @zroehr @InconceivableVizzini

Notes

how to get line numbers from xml.etree.ElementTree elements: http://stackoverflow.com/a/36430270
to be able to create sourcemaps we need a hook into the etree.tostring() code but there does not seem to be an easy way. What do folks think about copy/pasta'ing the non-C implementation and adding that hook? https://github.com/python/cpython/blob/c30098c8c6014f3340a369a31df9c74bdbacc269/Lib/xml/etree/ElementTree.py#L898-L946

reedstrm commented 7 years ago

And a pony! I agree, this sounds good, but is probably a large amount of work. We'll have to scope it.

helenemccarron commented 7 years ago

And handling the whole HTML in an editor is nearly impossible. Atom will crash each time. Sublime handles those files better but it takes very very long and almost impossible to use efficiently other than for quick edits.

philschatz commented 7 years ago

I got outputting sourcemaps to work for XML files (Note: not HTML files) by:

extending the etree.XMLParser to squirrel away the line/column numbers onto the Element when parsing
porting Mozilla's sourcemap-serializer code to python

The Python code is at https://github.com/philschatz/python-html-sourcemaps and all the output files are checked in so you do not have to run the code to see the results.

It is a testament to copy-pasta/cargo-culting but it works 😸 . What should I do to clean it up to get it into the various pipelines that convert XML->XML (or HTML)?

It seems the easiest way to get this into easybake is to read in the HTML file, write out the HTML file as XHTML, and then read in the XHTML file using the Annotated XHTML parser.

reedstrm commented 7 years ago

Helene's comment reminds me of a thought I had re: cli baking: perhaps cnx-easybake could/should work on epub zips/unzipped dirs? Can that be shoe-horned into this, if we're doing the mapping code anyway?

reedstrm commented 7 years ago

Also, we'd need timings, but I bet the non-C Element parser is muuuch slower, so this will need to be debug only sort of option.

philschatz commented 7 years ago

Just ran into this again. Posting the stack trace to show an example of where the line numbers of both the CSS and HTML would have been useful:

Traceback (most recent call last):
  File "(...)/cnx-recipes/venv/bin/cnx-easybake", line 9, in <module>
    load_entry_point('cnx-easybake==0.7.0', 'console_scripts', 'cnx-easybake')()
  File "(...)/cnx-recipes/venv/lib/python2.7/site-packages/cnxeasybake/scripts/main.py", line 84, in main
    args.coverage_file, args.use_repeatable_ids)
  File "(...)/cnx-recipes/venv/lib/python2.7/site-packages/cnxeasybake/scripts/main.py", line 21, in easybake
    oven.bake(html_doc, last_step)
  File "(...)/cnx-recipes/venv/lib/python2.7/site-packages/cnxeasybake/oven.py", line 253, in bake
    grouped_insert(target, value)
  File "(...)/cnx-recipes/venv/lib/python2.7/site-packages/cnxeasybake/oven.py", line 1268, in grouped_insert
    t.parent.insert(t.parent.index(t.tree), value)
  File "src/lxml/lxml.etree.pyx", line 1233, in lxml.etree._Element.index (src/lxml/lxml.etree.c:55360)
ValueError: Element is not a child of this node.

The content fix for the immediate problem: https://github.com/Connexions/oer.exports/issues/2498

philschatz commented 7 years ago

A similar error occurred again (in the Travis logs for https://github.com/Connexions/cnx-recipes/pull/147 ):

Generating temporary ./tmp//statistics/mixins/styleguide/page.table.html-baked.html
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.9/bin/cnx-easybake", line 9, in <module>
    load_entry_point('cnx-easybake==0.7.0', 'console_scripts', 'cnx-easybake')()
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/cnxeasybake/scripts/main.py", line 84, in main
    args.coverage_file, args.use_repeatable_ids)
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/cnxeasybake/scripts/main.py", line 21, in easybake
    oven.bake(html_doc, last_step)
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/cnxeasybake/oven.py", line 253, in bake
    grouped_insert(target, value)
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/cnxeasybake/oven.py", line 1268, in grouped_insert
    t.parent.insert(t.parent.index(t.tree), value)
  File "src/lxml/lxml.etree.pyx", line 1233, in lxml.etree._Element.index (src/lxml/lxml.etree.c:55360)
ValueError: Element is not a child of this node.

It seems like outputting the error line numbers are useful even when running "in production" since this is a command-line tool

philschatz commented 7 years ago

I tried adding the HTML source line number that caused the exception but ran into problems; after a couple of hours hitting my head against the wall I gave up, but here is how far I got: https://github.com/Connexions/cnx-easybake/compare/show-css-source-line...use-python-etree

I ended up (probably excessively):

parsing the HTML file twice; once to convert HTML to XML and then 2nd time to parse and annotate the Element objects with line numbers
use python's limited XPath implementation to rewrite id attributes when deep-copying
use python's limited XPath implementation to implement css_to_func
- here is where I got stuck because it could not parse descendant-or-self::dl/dt complaining that prefix u'descendant-or-self' not found in prefix map

reedstrm commented 7 years ago

Why are these tracebacks buried in a issue about wanting better source lines in errors? An issue that has the example of the CSS and HTML that causes the given tracebacks would be a lot more likely to catch my eye. :-)

philschatz commented 7 years ago

Yep, I created the Issue to mark cases where source lines would be useful (difficult to determine which line of CSS was causing the exception). I planned on investigating why the exception occurred and just wanted to note it here for discussion later but kept running out of time to explore.

philschatz commented 7 years ago

Also, I figured you'd want "An issue that has the [distilled] example CSS and HTML" and that's what I hadn't created yet, hence no Issue for this specific problem.

(I did note in the css-refactor doc that the process of making an example CSS+HTML test would be very useful & speed up dev time 😄 )

reedstrm commented 7 years ago

I'd rather have an issue /w a traceback and placeholders for what caused it than nothing. :-)

brittweinstein commented 5 years ago

CLOSING. If this presents itself as an issue again, we will reopen.

openstax / cnx-easybake

Show HTML and CSS source lines when warnings/errors occur #56

Notes