NaturalIntelligence / fast-xml-parser

Validate XML, Parse XML and Build XML rapidly without C/C++ based libraries and no callback.

https://naturalintelligence.github.io/fast-xml-parser/

MIT License

2.45k stars 296 forks source link

XMLBuilder: introduce initial indentation level option #566

Open moki opened 1 year ago

moki commented 1 year ago

Purpose / Goal

Introduce new option initialIndentationLevel and initialize builder with it instead of the literal value 0.

The option initialIndentationLevel is going to default to 0 so that default behaviour will be preserved.

Yet will allow to start generating xml document starting from the specified indentation level.

I explain the reasoning behind introducing this feature extensively in the #565

Closes: #565

Type

[ ] Bug Fix
[ ] Refactoring / Technology upgrade
[x] New Feature

amitguptagwl commented 1 year ago

These changes seems fine to me. I've to think more on option name, if something else be more concise.

coveralls commented 1 year ago

Coverage: 98.204% (+0.01%) from 98.194% when pulling 0045494c9a7600082a6cdd8665bff1c1ddae9403 on moki:moki-codes/builder-initial-indentation-level-option into b6cad831ebb673949e285b7244225b0736e44c51 on NaturalIntelligence:master.

amitguptagwl commented 1 year ago

@moki I'm just wondering if we should name it as "margin". It might not be a good suggestion so I'm thinking more like in this way.

moki commented 1 year ago

margin

i guess margin describes well what one will actually see in the end result.

but doesn't fit well regarding terminology...

What do you think about startIndent or maybe indentPrefix?

amitguptagwl commented 1 year ago

indentPrefix looks good but may confuse users that it might be used everytime. startIndent in itself is not explanatory. Let me think more like marginLevel or something else.

moki commented 1 year ago

@amitguptagwl sorry for pinging you, but do you have any updates on the option naming?

amitguptagwl commented 1 year ago

@moki Sorry I was on a long vacation and now sick. I'll check it back as soon as I feel little good.

moki commented 1 year ago

@moki Sorry I was on a long vacation and now sick. I'll check it back as soon as I feel little good.

its all good, get well soon!

moki commented 1 year ago

hey @amitguptagwl, have you had time to revisit this?

amitguptagwl commented 1 year ago

Hey @moki , I've completed the major development of v5 of this library few weeks back. Hence, I'm avoiding any change in the current version of library until or unless some bug is reported. However, the new version has some performance issue. So I'll merge this PR if v5 is discarded or I'll inform you once it is released. For the new version, you can check my post under discussion. Thanks for your effort and sorry for the delay.

moki commented 1 year ago

Hey @moki , I've completed the major development of v5 of this library few weeks back. Hence, I'm avoiding any change in the current version of library until or unless some bug is reported. However, the new version has some performance issue. So I'll merge this PR if v5 is discarded or I'll inform you once it is released. For the new version, you can check my post under discussion. Thanks for your effort and sorry for the delay.

alright keep me posted, also do you plan to release the draft of the v5 as a separate branch.

i mean i would like to "rebase" this PR onto it.

in quotes - because prolly i will have to re-write my pr for it.

amitguptagwl commented 1 year ago

Hi @moki, thought v5 is supposed to support many awaited features, it's speed is downgraded by 4-5 times. I have few ideas of improving it by shifting to old js style. However, not getting time to work due to job shift. As soon as I fix the performance issue in new version, I'll cut the branch and will let you know. Thanks