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

[Stefan Monnier]

> OK, let's try this again.
> New patch attached.

Getting there.  But still waiting for the copyright paperwork.

Since we still have time, here are some further nitpicks.

> @@ -99,6 +100,25 @@
>  tell the difference between `false' and `null'.  Consider let-binding
>  this around your call to `json-read' instead of `setq'ing it.")
 
> +(defvar json-encoding-separator ","
> +  "Value to use as an element seperator when encoding.")
> +
> +(defvar json-encoding-default-indentation "  "
> +  "The default indentation level for encoding. Used only when
> +`json-encoding-pretty-print' is non-nil.")
> +
> +(defvar json-encoding-current-indentation "\n"
> +  "Internally used to keep track of the current indentation level of
> +encoding. Used only when `json-encoding-pretty-print' is non-nil.")

Use a "json--" prefix, since it's a convention we use to express that
something is internal.

> +(defvar json-encoding-pretty-print nil
> +  "Setting this to non-nil will result in the output of `json-encode'
> +to be pretty-printed.")
> +
> +(defvar json-encoding-lisp-style-closings nil
> +  "Setting this to `t' will cause ] and } closings to happen lisp-style,
> +without indentation.")

The first line of a docstring should "stand on it own", i.e. it
shouldn't end in the middle of a sentence.
Try C-u M-x checkdoc-current-buffer.

Rather than "Setting this to <foo> will cause <bar>", we usually just
say "If <foo>, <bar>".  E.g.
  "If non-nil, the output of `json-encode' will be pretty-printed."

Also, contrary to all other symbols, t (like nil) is written without
`...' quoting.
  
> +(defmacro json--with-indentation (body)
> +  `(let ((json-encoding-current-indentation
> +          (if json-encoding-pretty-print
> +              (concat json-encoding-current-indentation
> +                      json-encoding-default-indentation)
> +            "")))
> +     ,body))

Good.

>  (defun json-encode-hash-table (hash-table)
>    "Return a JSON representation of HASH-TABLE."
> -  (format "{%s}"
> +  (format (if json-encoding-pretty-print "{%s%s}" "{%s}")

Hmm... if json-encoding-pretty-print is nil, we still pass 2 args, and
the second is always "", so we can always use "{%s%s}", right?

>            (json-join
>             (let (r)
> -             (maphash
> -              (lambda (k v)
> -                (push (format "%s:%s"
> -                              (json-encode-key k)
> -                              (json-encode v))
> -                      r))
> -              hash-table)
> +             (json--with-indentation
> +               (maphash
> +                (lambda (k v)
> +                  (push (format
> +                         (if json-encoding-pretty-print
> +                             "%s%s: %s"
> +                           "%s%s:%s")
> +                         json-encoding-current-indentation
> +                         (json-encode-key k)
> +                         (json-encode v))
> +                        r))
> +                hash-table))
>               r)
> -           ", ")))
> +           json-encoding-separator)
> +          (if json-encoding-lisp-style-closings
> +              ""
> +              json-encoding-current-indentation)))

[ It just occurs to me, that the json printer should print to a buffer,
  rather than to a string.  But let's keep this issue for later.  ]

> -  (concat "[" (mapconcat 'json-encode array ", ") "]"))
> +  (if (and json-encoding-pretty-print
> +           (> (length array) 0))
> +      (concat
> +       (let ((json-encoding-current-indentation
> +              (concat json-encoding-current-indentation
> +                      json-encoding-default-indentation)))

Use json--with-indentation here (even if it performs an extra redundant
test of json-encoding-pretty-print).


        Stefan