GNU bug report logs - #12634
24.2; [FEATURE REQUEST] JSON pretty printer

Previous Next

Full log

Message #44 received at 12634 <at> debbugs.gnu.org (full text, mbox):

From: Ryan Crum <ryan.crum <at> eleostech.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 12634 <at> debbugs.gnu.org
Subject: Re: bug#12634: Patch for pretty-printing in json.el
Date: Sat, 17 Nov 2012 12:37:17 -0500

[Message part 1 (text/plain, inline)]

Hi Stefan,

I actually have not yet received anything regarding copyright assignment in the mail, so I resubmitted the questionnaire to assign <at> gnu.org today.

New patch attached.

[json-pretty-print.diff (application/octet-stream, attachment)]

[Message part 3 (text/plain, inline)]

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.