fast-xml-parser icon indicating copy to clipboard operation
fast-xml-parser copied to clipboard

Published 20 hours ago • verified publisher icon NaturalIntelligence

XMLBuilder: introduce initial indentation level option

Open moki opened this issue 1 year ago • 12 comments

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

moki avatar Apr 23 '23 18:04 moki

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

amitguptagwl avatar Apr 25 '23 05:04 amitguptagwl

Coverage Status

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.

coveralls avatar Apr 25 '23 05:04 coveralls

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

amitguptagwl avatar Apr 26 '23 00:04 amitguptagwl

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?

moki avatar Apr 26 '23 09:04 moki

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.

amitguptagwl avatar Apr 27 '23 00:04 amitguptagwl

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

moki avatar May 02 '23 09:05 moki

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

amitguptagwl avatar May 17 '23 03:05 amitguptagwl

@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 avatar May 17 '23 08:05 moki

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

moki avatar Jun 14 '23 11:06 moki

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.

amitguptagwl avatar Jun 15 '23 02:06 amitguptagwl

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.

moki avatar Jun 15 '23 08:06 moki

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

amitguptagwl avatar Jul 22 '23 03:07 amitguptagwl