Re: [cp-patches] DOM conformance fixes

[Mark Wielaard]

Hi Chris,

On Sat, 2005-12-31 at 10:32 +0000, Chris Burdess wrote:
>
> 2005-12-31  Chris Burdess  <address@hidden>
> 
>         * gnu/xml/dom/DomNamedNodeMap.java,
>           gnu/xml/dom/DomNode.java,
>           gnu/xml/dom/ls/SAXEventSink.java,
>           gnu/xml/stream/SAXParser.java,
>           gnu/xml/stream/XMLParser.java: Fix entity reference DOM construction
>           and correct DOM tree normalisation.

Please do try to write a bit more specific ChangeLog entries. Like
described in the hacker guide:
http://www.gnu.org/software/classpath/docs/hacking.html#SEC10
That makes it a little easier to review patches since it is more
explicit what change was deliberate and what might not be.
(Of course a complete rewrite can just be commented as "rewrite", but in
this case I would have liked to see some more explicit change log
entries.)

> @@ -2946,23 +2948,20 @@
>                  else
>                    {
>                      reset();
> -                    if (replaceERefs || (flags & LIT_NORMALIZE) > 0)
> -                      {
> -                        String entityName = readNmtoken(true);
> -                        require(';');
> -                        String text =
> -                          (String) PREDEFINED_ENTITIES.get(entityName);
> -                        if (text != null)
> -                          literalBuf.append(text);
> -                        else
> -                          expandEntity(entityName,
> -                                       (flags & LIT_ATTRIBUTE) != 0);
> -                        entities = true;
> -                        continue;
> -                      }
> +                    //if (replaceERefs || (flags & LIT_NORMALIZE) > 0)
> +                    //  {
> +                    String entityName = readNmtoken(true);
> +                    require(';');
> +                    String text =
> +                      (String) PREDEFINED_ENTITIES.get(entityName);
> +                    if (text != null)
> +                      literalBuf.append(text);
>                      else
> -                      error("parser is configured not to replace entity " +
> -                            "references");
> +                      expandEntity(entityName,
> +                                   (flags & LIT_ATTRIBUTE) != 0);
> +                    entities = true;
> +                    continue;
> +                    //  }
>                    }
>                }
>              break;

This change can be confusing, you commented the if statement and { }
block out. If it isn't necessary here please just remove such dead code.

Thanks,

Mark

-- 
Escape the Java Trap with GNU Classpath!
http://www.gnu.org/philosophy/java-trap.html

Join the community at http://planet.classpath.org/

signature.asc
Description: This is a digitally signed message part