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

[wkrick at eio-online.com]

I've attached a zip file containing both changed files and a patch.

This is the simplest fix possible...

setIndent("  ") means newlines and "  " indents
setIndent("") means newlines and "" indents
setIndent(null) means no newlines and no indents

I considered adding a boolean "newlines" parameter to Format, along  
with getter and setter methods but I thought that adding more methods  
the JDOM API would probably be frowned upon.

If you think that adding "newlines" is a better way of fixing it, let  
me know and I'll submit another patch.


On a related note... Any idea on when there might be a JDOM 1.1 release?



Quoting Jason Hunter <jhunter at servlets.com>:

>> 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-


-------------- next part --------------
A non-text attachment was scrubbed...
Name: indent-fix.zip
Type: application/zip
Size: 18838 bytes
Desc: not available
Url : http://www.jdom.org/pipermail/jdom-interest/attachments/20060908/36e2e4d8/indent-fix.zip