[jdom-interest] setIndent("") doesn't work as expected

Jason Hunter jhunter at servlets.com
Fri Sep 8 00:32:51 PDT 2006


> In Format.java setLineSeparator(), the docs say...
> 
>      * This will set the newline separator (<code>lineSeparator</code>).
>      * The default is <code>\r\n</code>. Note that if the "newlines"
>      * property is false, this value is irrelevant.
> 
> As far as I can see, there is no "newlines" property.

Happily, that was fixed a while ago in the code checked into CVS.

> Then in XMLOutputter.java...
> 
>     /**
>      * This will print a new line only if the newlines flag was set to
>      * true.
>      *
>      * @param out <code>Writer</code> to use
>      */
>     private void newline(Writer out) throws IOException {
>         if (currentFormat.indent != null) {
>             out.write(currentFormat.lineSeparator);
>         }
>     }
> 
>     /**
>      * This will print indents (only if the newlines flag was
>      * set to <code>true</code>, and indent is non-null).
>      *
>      * @param out <code>Writer</code> to use
>      * @param level current indent level (number of tabs)
>      */
>     private void indent(Writer out, int level) throws IOException {
>         if (currentFormat.indent == null ||
>             currentFormat.indent.equals("")) {
>             return;
>         }
> 
>         for (int i = 0; i < level; i++) {
>             out.write(currentFormat.indent);
>         }
>     }
> 
> 
> ...notice that neither method actually tests a "newlines" flag for true, 
> they only test "indent" for null.

That one's still there.

> In order to really be able to control the output, you need to be able to 
> have these options...
> 
> 1) newlines and indents (1 or more spaces)
> 2) newlines and no indents (0 spaces)
> 3) neither
> 
> It doesn't make sense to have indents with no newlines.
> 
> Judging from the documentation, I believe this was what was originally 
> intended but someone tried to simplify the logic somewhere and 
> inadvertently removed the ability to have option #2.  I think they were 
> trying to overload the meaning of indents=null to mean newlines=false.  
> Probably not a good idea.
> 
> 
> I'd like to make a patch to put that functionality back in but I don't 
> want to break anything if I can avoid it.  Do you have a set of test 
> scripts/programs that you run against it to verify that things aren't 
> broken after changes are made?

I'd welcome a patch.  Please base it on the CVS code, if you can.  You 
can submit a patch either as a full file or using the "diff" tool with 
ideally the -u flag.

You'll see we have a jdom-test module under CVS with test cases. 
They're not exhaustive, but they'll help confirm your fix is good.

-jh-


More information about the jdom-interest mailing list