send2vinnie / pugixml

Automatically exported from code.google.com/p/pugixml
0 stars 0 forks source link

Add format option to place each attribute to new line #241

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
For now attributes is placed in same line as parent xml element.
For example:
<some int="5" bool="false" string="value" />

I suggest to introduce new format option which place each attribute to new line.
For example:
<some
    int="5"
    bool="false"
    string="value" />

Main motivation is that it will help to compare configuration xml files more 
easily.

For example, I often need to compare and merge configuration from build to 
production. There is some attributes that I don't need to merge to production, 
and some attributes changed and must be merged.
If all attributes is placed on same line, I need to merge changes manually.
If all attributes is placed on new line, I just can press hot-key in compare 
tool to merge only attributes that need changes.

I found that it is easy to add this formatting option to pugixml. I just need 
to:

1) add format option in pugixml.hpp:
// Set each attribute to new line
const unsigned int format_each_attribute_on_new_line = 0x40;

2) modify node_output_attributes() function:

    PUGI__FN void node_output_attributes(xml_buffered_writer& writer, const xml_node& node, const char_t* indent, unsigned int flags, unsigned int depth)
    {
        const char_t* default_name = PUGIXML_TEXT(":anonymous");

        for (xml_attribute a = node.first_attribute(); a; a = a.next_attribute())
        {
            if ((flags & format_indent) && (flags & format_each_attribute_on_new_line) && (flags & format_raw) == 0)
            {
                writer.write('\n');
                for (unsigned int i = 0; i < depth; ++i) writer.write(indent);
            }
            else
            {
                writer.write(' ');
            }
            writer.write(a.name()[0] ? a.name() : default_name);
            writer.write('=', '"');

            text_output(writer, a.value(), ctx_special_attr, flags);

            writer.write('"');
        }
    }

Note in function above that attribute lines will be indented only if 
format_indent is ON. I think it is right decision.

3) make sure that node_output() invokes node_output_attributes() like this:
node_output_attributes(writer, node, indent, flags, depth+1);

Would you merge this feature to source base?
Best regards, Aleksei.

PS. git-diff patch is attached.

Original issue reported on code.google.com by AKhar...@gmail.com on 5 Oct 2014 at 7:01

Attachments:

GoogleCodeExporter commented 9 years ago
This makes sense. It seems that putting the closing angular bracket on a 
separate line is even better for merging though? I.e.

<foo
  attr1='value1'
  attr2='value2'
>

This makes adding/removing attributes merge friendly.

Original comment by arseny.k...@gmail.com on 6 Oct 2014 at 1:29

GoogleCodeExporter commented 9 years ago
Yes, agree.
For me it is more usual to keep indentation for closing bracket.
<foo
    attr1='value1'
    attr2='value2'
    >
    <subobject />
</foo>

or
<foo
    attr1='value1'
    attr2='value2'
    />

(notepads like Sublime Text supports indentation in that style out of the box)

I think it is not very important, the decision is yours.

Original comment by AKhar...@gmail.com on 6 Oct 2014 at 6:01

GoogleCodeExporter commented 9 years ago
Hi!
Last day I think twice about xml formatting and it seems that formatting like

<some
    int="5"
    bool="false"
    string="value" />
<some
    int="5"
    bool="false"
    string="value">
    <subobject />
</some>

is more than appropriate.

Pros:
- less lines of xml and more readable
- Visual Studio IDE has formatting options for xml with this style for closing 
bracket
- implementation is simple (like in original post's patch)

So, I think it make sense to support two additional formats:
- format_each_attribute_on_new_line
- format_closing_bracket_on_new_line

format_closing_bracket_on_new_line is applied only when format_indent and 
format_each_attribute_on_new_line is enabled.

For convinience:
format_indent_attribute = format_indent | format_each_attribute_on_new_line
format_indent_full = format_indent | format_each_attribute_on_new_line | 
format_closing_bracket_on_new_line

can be added to pugixml.hpp

I implemented this behavior, see implementation patch in attachments.
I tested formatting with format_indent_full flag a little bit.
My opinion that format_indent_attribute is looking better :)

Best regards, Aleksei.

Original comment by AKhar...@gmail.com on 7 Oct 2014 at 10:17

Attachments:

GoogleCodeExporter commented 9 years ago
Moving the issue to GitHub: https://github.com/zeux/pugixml/issues/14

Original comment by arseny.k...@gmail.com on 26 Oct 2014 at 8:56