From: Waylan L. <wa...@gm...> - 2010-10-30 01:44:20
|
After a little more testing, I have committed this. The good news is I have added tests, so while we don't use comments within markdown itself, we shouldn't break this in the future. Thanks for the report. The latest is available via git. On Fri, Oct 29, 2010 at 11:06 AM, Waylan Limberg <wa...@gm...> wrote: > Ok, I think I have a working patch for this. I've attached a diff. > Dimitry, any possibility you could test this against your code before > I commit it? > > On Mon, Oct 18, 2010 at 4:40 AM, Dmitry Tyschenko <dty...@gm...> wrote: >>> At first I was a little confused by your report as we don't do >>> anything with comments - if they exist in the source they are pulled >>> out as rawhtml and never see ElementTree. Then I realized you must be >>> using a third party extension which create html comments and inserts >>> them into the tree. That is certainly something we want to support and >>> allow you to do. >> >> I'm writing my own. You know this annoying bug in one popular browser, >> when empty block height will be not zero until you put some content >> inside, even if this content is comment. >> >>> However, it appears you are also using the html4 serializer - which we >>> borrowed (with permission) from the ElementTree 1.3 Preview (still in >>> alpha since 2007 AFAICT). Any changes to that serializer would ideally >>> be changes made upstream as well. >> >>> In fact, your proposed fix, to also import parts of ElementTree when >>> using cElementTree feels wrong and defeats the purpose of cElementTree >>> in the first place. In fact, I see in the notes to ElementTree 1.3 >>> there is a note [3] that it only works with cElementTree 1.0.6 or >>> later. What version are you using? >> >> 1.0.6 >> >> It's not mine idea to import ElementTree.Comment, that is what >> cElementTree is doing. >> >> Problem is in checking that node is Comment. There is no such thing as >> nodeType like in DOM. So ElementTree putting into tag field link to >> ElementTree.Comment function and then making conditions like: >> >> if node.tag is ElementTree.Comment: >> >> Even if node was created using cElementTree implementation node.tag >> will point to ElementTree.Comment. That is why i was thinking that >> ElementTree.Comment implementation is called inside. If you look at >> ElementTree.Comment you'll see why node.tag is ElementTree.Comment: >> >> def Comment(text=None): >> element = Element(Comment) >> element.text = text >> return element >> >>> Hmm, now I'm looking at the code and all isBlockLevel does it take a >>> tag, which is a string. Perhaps elsewhere in the code, we are assuming >>> every ElementTree element has a tag and that is where the exception is >>> being raised. Like in markdown.treeprocessors.PrettifyTreeprocessor. >>> Should we be checking that an element has a tag first and behave >>> differently if not? >> >> As you now know node.tag isn't always a string, it can be function! So >> isBlockLevel is raising TypeError exception. >> >> So to fix these bugs the question should be answered: what is the >> propper way to check that node is Comment? >> > > > > -- > ---- > \X/ /-\ `/ |_ /-\ |\| > Waylan Limberg > -- ---- \X/ /-\ `/ |_ /-\ |\| Waylan Limberg |