Subject: Re: bug#12634: Patch for pretty-printing in json.el

From: Ryan Crum <[email-masked]>
Date: Tue, 30 Oct 2012 17:04:52 -0400

OK, let's try this again.

New patch attached.

On Tue, Oct 30, 2012 at 4:03 PM, Stefan Monnier <[email-masked]>wrote:

> - Your patch does not apply to the trunk version of json.el where
>   alist/plist keys are encoded with a new json-encode-key.
>

Oops. Fixed.

>
> - I don't understand this helper function.  Why not store the leading "\n"
>   directly in json-encoding-current-indentation so that
>   we can use json-encoding-current-indentation directly instead of
>   calling json--current-whitespace?
>

Yeah, I see what you mean. Fixed.

>
> - BTW your patch calls json-encoding-current-indentation as a function in
>   json-encode-plist.
>

Foggy-headed. Fixed.

>
> - OTOH, I wouldn't mind a helper function/macro to consolidate all the
>
>           (let ((json-encoding-current-indentation
>                  (if json-encoding-pretty-print
>                      (concat json-encoding-current-indentation
>                              json-encoding-default-indentation)
>                    "")))
>
>   in a single spot.
>

I created a little macro for this (json--with-indentation).

>
> - Why use ", " rather than "," for json-encoding-default-separator?
>

Vestigial code from a much older version of this patch. Fixed.

>
> - json-encoding-default-separator is a bad name since it holds the
>   *current* separator, rather than the default.
>

Renamed to json-encoding-separator. This was also vestigial (at one point
it only used line-breaks if lists were beyond a certain length; overly
complex).

> - Why (format "%s" (json--current-whitespace)) rather than
>   (json--current-whitespace)?
>

Yeah, good question. Fixed.