[webkit-reviews] review granted: [Bug 64398] Character reference parser for new XML parser : [Attachment 100743] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jul 13 18:45:49 PDT 2011
Adam Barth <abarth at webkit.org> has granted Jeffrey Pfau <jeffrey at endrift.com>'s
request for review:
Bug 64398: Character reference parser for new XML parser
https://bugs.webkit.org/show_bug.cgi?id=64398
Attachment 100743: Patch
https://bugs.webkit.org/attachment.cgi?id=100743&action=review
------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=100743&action=review
This looks great. We could od some work to fixup some naming mistakes I made
when writing this code originally, but that's nto a big deal. (The most
pervasive one is renaming "cc" to either currentCharacter or "c" as
appropriate.)
> Source/WebCore/xml/parser/CharacterReferenceParserInlineMethods.h:28
> +#ifndef CharacterReferenceParserInlineMethods_h
> +#define CharacterReferenceParserInlineMethods_h
I'd drop the word "Methods" from the name of this file/
> Source/WebCore/xml/parser/CharacterReferenceParserInlineMethods.h:51
> +template <typename ParserFunctions>
> +inline bool consumeCharacterReference(SegmentedString& source, Vector<UChar,
16>& decodedCharacter, bool& notEnoughCharacters, UChar
additionalAllowedCharacter)
jamesr says to get rid of this inline keyword. That's probably the right. The
compiler will probably just generate an unconditional jump anyway.
> Source/WebCore/xml/parser/XMLCharacterReferenceParser.cpp:45
> + return 0xFFFD;
bad indents in this file.
> Source/WebCore/xml/parser/XMLCharacterReferenceParser.cpp:66
> + ASSERT_NOT_REACHED();
Do you mean notImplemented() ?
More information about the webkit-reviews
mailing list