[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#12634: Patch for pretty-printing in json.el
From: |
Ryan Crum |
Subject: |
bug#12634: Patch for pretty-printing in json.el |
Date: |
Sat, 17 Nov 2012 12:37:17 -0500 |
Hi Stefan,
I actually have not yet received anything regarding copyright assignment in the
mail, so I resubmitted the questionnaire to assign@gnu.org today.
New patch attached.
json-pretty-print.diff
Description: Binary data
On Nov 14, 2012, at 8:56 PM, Stefan Monnier wrote:
> Use a "json--" prefix, since it's a convention we use to express that
> something is internal.
Fixed.
> 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.
Ah, cool feature. Fixed.
>
>> (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?
Not exactly, since json--encoding-current-indentation defaults to newline. I've
tweaked this part to shift the complexity down into the second clause, however,
instead of specifying 2 formats with a potentially unused second argument.
>> - (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).
Agreed, fixed.
I've also added an error handling clause to fix a bug I noticed where if you
attempt to `json-pretty-print' some invalid JSON your text gets killed but
formatting fails and it doesn't get replaced. Is there a more idiomatic way of
doing this? I considered using copy-region-as-kill and then only killing the
text if the call to json-encode succeeds, but that seemed more awkward.
Thanks for your help!
-Ryan