Saxon

12.3 has a fix for https://saxonica.plan.io/issues/6063

This was a bug fix https://saxonica.plan.io/issues/6063

Did we get it wrong? if so, please send us a repro.

Thanks Martin. Then we probably have a bad patch somewhere, you can probably close this one and we'll try to investigate further on our side.

It might be worth pointing out that the user originally reporting bug #6063 said the behaviour varied according to the operating system, and we never got to the bottom of that suggestion.

I also cannot reproduce the problem using the original Saxon 12.3 HE JAR libraries and a bit of Java code so I'm inclining to blame one of our patches for this.

There are things I don't understand about the change made to net.sf.saxon.serialize.XMLIndenter Something like this:

 int indent = (level - 1) * getIndentation() + 3;

was changed to:

 int indent = (level - 1) * getIndentation() + 2;

I do not understand why not use directly the current attribute's level indentation

``
 int indent = level * getIndentation();

also I do not understand why if the default indentation is "3" for all elements I get an indentation of "4" on my side but maybe this is caused by one of our patches..

I think I understand, in XMLEmitter the tagname length is added to the indentation

            if (indentForNextAttribute >= 0) {
                indentForNextAttribute += displayName.length();
            }

so that "+2" means the start tag char and the space after the tag name so that the wrapped attribute is aligned with the attribute from the previous line.

int indent = (level - 1) * getIndentation() + 2;

And I confirm the problem is in one of our patches :( Sorry for wasting your time.

It was changed from +3 to +2 because it was wrong: the value no longer includes the newline character, only the number of spaces (see XMLEmitter.writeAttributeIndentString()).

The original design made XMLIndenter responsible for all aspects of indentation, by inserting text nodes into the pipeline, so XMLEmitter didn't have to know anything about indentation. This broke down when we started indenting attributes, so there's now a rather complicated interaction between the two classes.

If we assume line==0 (that is, the start tag isn't preceded by multi-line text), the number of spaces before a startElement start tag will be level*getIndentation(). We increment level before emitting the attributes, so for attributes we use (level-1) -- it would make more sense to defer incrementing level, but this is delicate code... The +2 is +1 for the "<" before the element name, and +1 for the space after the element name; XMLEmitter itself adds the length of the element name at line 471.

Sorry it's so convoluted, I'm sure it could be done much more elegantly.

Hi Michael, thanks, this is all clear to me now. Indeed the classes are a bit tightly coupled. Maybe from the point of view of someone reading the code that +2 could have been added in XMLEmitter along with the tag name. Or the other way around, in XMLIndenter take the element name into account:

                int indent = (level - 1) * getIndentation() + 2 + nameCode.getDisplayName().length();
                emitter.setIndentForNextAttribute(indent);