Re: Bug?
Charlie Reitzel <[email protected]> Wed, 25 Jan 2006 09:24:45 -0500
| Newsgroups | gmane.comp.web.html-tidy.devel |
|---|---|
| Message-ID | <[email protected]> |
Good eye, Cory. Wrt to the lexer.c issue, while it appears to be intentional, it also appears to be wrong! That attrs.c line, imo, is just wrong. That said, it is not immediately clear what is right. Time for a test case or 3 to exercise these bits of code. 1) Lexer, see http://tidy.sf.net/issue/427846 I would expand the scope of inquiry just a bit. Bitwise operations are not appropriate on token modes: IgnoreWhitespace, MixedContent, Preformatted, IgnoreMarkup and CdataContent. As non-negative integers (0, 1, 2, 3, ...) they are appropriate only for equality tests. I do not think relational ops (<, <=, >=, >) have any meaning on these values. The only place I see that happening is on line 1975. This is evaluated only if the the previous trailing blank was trimmed in TrimTrailingSpace() in parser.c. I would propose fixing the test and re-testing bug 427846 to see if the markup is what we want. If not, I propose dropping the Lexer.insertspace state variable altogether. A quick scan on the other modes: MixedContent - never explicitly tested, set frequently. Preformatted - several bitwise ops in parser.c! IgnoreMarkup - all tests correct CdataContent - all tests correct 2) attrs.c - Content Model Restate as nodeHasCM(node, CM_TABLE|CM_ROW) This change is relatively recent (v1.110 of attrs.c), so there is probably a test case for this change already if I know Arnaud. Note: perhaps this tiny function should be refactored as a macro. Ditto for the whole content model nodeXXXCM() family of functions. take it easy, Charlie At 03:45 AM 1/25/2006 -0800, Cory Nelson wrote: >On 1/25/06, Bjoern Hoehrmann <[email protected]> wrote: > > * Cory Nelson wrote: > > >IgnoreWhitespace is defined to 0, and in lexer.c line 1989 there is a > > >bitwise being done with it (mode & IgnoreWhitespace). Bug? > > > > http://cvs.sourceforge.net/viewcvs.py/tidy/tidy/src/lexer.c?r1=1.3&r2=1.4 > > Maybe Charlie knows? Major changes to whitespace processing have been > > made since, so this might just do what we want. > >It will always evaluate to 0, so maybe it should be taken out if that >was intentional. > >I also noticed attrs.c line 1370 - there is an expression >(node->tag->model & CM_TABLE|CM_ROW). Bitwise AND takes precedence >over bitwise OR and the spacing suggests the author really wanted >(node->tag->model & (CM_TABLE|CM_ROW)). > > > Björn Höhrmann > >-- >Cory Nelson ------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK! http://sel.as-us.falkag.net/sel?cmd=lnk&kid3432&bid#0486&dat1642