Add custom indent for pretty printing #1279

[WasylF]

Implementation for issue #1279

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

[googlebot]

CLAs look good, thanks!

[lyubomyr-shaydariv]

Is this field necessary? The indent field seems to cover this case.

[WasylF]

I've removed it.

[lyubomyr-shaydariv]

I think this is a pretty useful feature to have in GsonBuilder. By the way, did you consider something like .setPrettyPrintingByTabs(int count) and .setPrettyPrintingBySpaces(int count) rather than allowing a user to specify an arbitrary indent string? I know that JsonWriter allows arbitrary indent strings, but it looks more of a design issue allowing to emit illegal JSON. Your thoughts?

[WasylF]

@lyubomyr-shaydariv, Thanks for your review! Your proposal looks good. I could implement it. But what about just add check for indent string? smth like:

    if (indent != null && !indent.matches("\\s*")) {
        throw new IllegalArgumentException("Indent could contains ONLY whitespaces");
    }

[lyubomyr-shaydariv]

@wslf The \s pattern also accepts CR, LF, FF and VT. I hardly imagine any of these as an indent character, so the matches can be improved:

• the ^(?: *|\t*)$ is more strict and it does not allow a mixture of spaces and tabs (e.g. \t \t);
• extracting the regex to a static field of java.util.regex.Pattern might slightly increase the performance having the regex already-compiled and ready to use).

But if I'd design a new API, I'd probably would go with methods that do not allow to add characters other than and \t, but accept non-negative count. Additionally, checking for a non-negative value, count < 0, is just cheaper than matching against the pattern: I saw a lot of cases at StackOverflow where people create a new GsonBuilder every time they need it even if the GsonBuilder requires no dynamic arguments.

However, my idea has at lease on disadvantage and would complicate things for indent strings that are stored elsewhere, let's say, as configuration. This would require a user to store both indent character and indent character count or have some defaults to these + and dynamically dispatch to either to setPrettyPrintingByTabs or to setPrettyPrintingBySpaces. From other perspective, instead of having the latter two, there could be another overload: setPrettyPrinting(char indent, int count). This would require no a regex at all and would never go into a tab/spaces mixture + it might be aligned with indentation settings stored elsewhere. So, in general, three methods perhaps something like:

• setPrettyPrinting(char indent, int count) with indent != ' ' && indent != '\t' and count < 0 invariants;
• setPrettyPrintingByTabs(count) with just a call to setPrettyPrinting('\t', count) (will never throw an 'Invalid indent character' exception);
• and setPrettyPrintingBySpaces with a call to setPrettyPrinting(' ', count) (will never throw an Invalid indent character as well).

Anyway, the final decisions always go to the Gson development team.

[ryber]

I'd love to see this PR merged (or some variation), this is pretty old now. Any possibility it will get merged?

[inder123]

We would like to see a more general solution. For example, something that supplies a formatter that walks through a DOM tree and prints it.

[lyubomyr-shaydariv]

@inder123 but does it really need to be such complicated, and doesn't this PR merely implement a minor tweak for an existing currently poorly customizable indentation feature (that's pretty old itself) rather than implementing a neat well-designed formatter from scratch?

[inder123]

@lyubomyr-shaydariv It's not a minor tweak if you are introducing a new API.