unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#12634: 24.2; [FEATURE REQUEST] JSON pretty printer
@ 2012-10-13  8:39 Leo
  2012-10-13 21:56 ` Stefan Monnier
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Leo @ 2012-10-13  8:39 UTC (permalink / raw)
  To: 12634

It would be lovely if the included json.el could do pretty printing.





^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: 24.2; [FEATURE REQUEST] JSON pretty printer
  2012-10-13  8:39 bug#12634: 24.2; [FEATURE REQUEST] JSON pretty printer Leo
@ 2012-10-13 21:56 ` Stefan Monnier
  2012-10-18 21:18 ` bug#12634: existing implementation Aurélien Aptel
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 18+ messages in thread
From: Stefan Monnier @ 2012-10-13 21:56 UTC (permalink / raw)
  To: Leo; +Cc: 12634

> It would be lovely if the included json.el could do pretty printing.

Patch welcome,


        Stefan





^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: existing implementation
  2012-10-13  8:39 bug#12634: 24.2; [FEATURE REQUEST] JSON pretty printer Leo
  2012-10-13 21:56 ` Stefan Monnier
@ 2012-10-18 21:18 ` Aurélien Aptel
  2012-10-19  1:37   ` Stefan Monnier
  2012-10-25 14:55 ` bug#12634: Patch for pretty-printing in json.el Ryan Crum
  2012-12-14  2:56 ` bug#12634: Patch Ryan Crum
  3 siblings, 1 reply; 18+ messages in thread
From: Aurélien Aptel @ 2012-10-18 21:18 UTC (permalink / raw)
  To: 12634

Someone already made a pretty-print patch [1] and other utility functions [2].

1: https://github.com/thorstadt/json.el
2: https://github.com/thorstadt/json-pretty-print.el





^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: existing implementation
  2012-10-18 21:18 ` bug#12634: existing implementation Aurélien Aptel
@ 2012-10-19  1:37   ` Stefan Monnier
  2012-10-19 16:20     ` Aurélien Aptel
  0 siblings, 1 reply; 18+ messages in thread
From: Stefan Monnier @ 2012-10-19  1:37 UTC (permalink / raw)
  To: Aurélien Aptel; +Cc: 12634

> Someone already made a pretty-print patch [1] and other utility
> functions [2].
> 1: https://github.com/thorstadt/json.el
> 2: https://github.com/thorstadt/json-pretty-print.el

Would you be so kind and try to see if we can get the copyright cleared?


        Stefan





^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: existing implementation
  2012-10-19  1:37   ` Stefan Monnier
@ 2012-10-19 16:20     ` Aurélien Aptel
  2012-10-19 18:04       ` Stefan Monnier
  0 siblings, 1 reply; 18+ messages in thread
From: Aurélien Aptel @ 2012-10-19 16:20 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12634

On Fri, Oct 19, 2012 at 3:37 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
> Would you be so kind and try to see if we can get the copyright cleared?

I've asked him via email (mentioning the url to this bug). I've also
sent the instructions to get the assignment form.





^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: existing implementation
  2012-10-19 16:20     ` Aurélien Aptel
@ 2012-10-19 18:04       ` Stefan Monnier
  2012-10-19 21:28         ` Stefan Monnier
  0 siblings, 1 reply; 18+ messages in thread
From: Stefan Monnier @ 2012-10-19 18:04 UTC (permalink / raw)
  To: Aurélien Aptel; +Cc: 12634

>> Would you be so kind and try to see if we can get the copyright cleared?
> I've asked him via email (mentioning the url to this bug). I've also
> sent the instructions to get the assignment form.

Great, thank you,


        Stefan





^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: existing implementation
  2012-10-19 18:04       ` Stefan Monnier
@ 2012-10-19 21:28         ` Stefan Monnier
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Monnier @ 2012-10-19 21:28 UTC (permalink / raw)
  To: Aurélien Aptel; +Cc: 12634

>>> Would you be so kind and try to see if we can get the copyright cleared?
>> I've asked him via email (mentioning the url to this bug). I've also
>> sent the instructions to get the assignment form.
> Great, thank you,

BTW, please make sure that he specifies "Emacs" (and not json.el for
example) as the package for which he wants to sign the paperwork.


        Stefan





^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: Patch for pretty-printing in json.el
  2012-10-13  8:39 bug#12634: 24.2; [FEATURE REQUEST] JSON pretty printer Leo
  2012-10-13 21:56 ` Stefan Monnier
  2012-10-18 21:18 ` bug#12634: existing implementation Aurélien Aptel
@ 2012-10-25 14:55 ` Ryan Crum
  2012-10-25 18:08   ` Stefan Monnier
  2012-12-14  2:56 ` bug#12634: Patch Ryan Crum
  3 siblings, 1 reply; 18+ messages in thread
From: Ryan Crum @ 2012-10-25 14:55 UTC (permalink / raw)
  To: 12634


[-- Attachment #1.1: Type: text/plain, Size: 269 bytes --]

Hi,

I've consolidated my changes into json.el. I have not yet received the form
for copyright assignment in the mail, but I have requested it. Please
advise me if you'd like me to make any changes here, and I'll be happy to
accomodate. Patch attached.

Thanks,

-Ryan

[-- Attachment #1.2: Type: text/html, Size: 306 bytes --]

[-- Attachment #2: pretty-printing-patch --]
[-- Type: application/octet-stream, Size: 10039 bytes --]

*** a/json.el
--- b/json.el
***************
*** 3,9 ****
  ;; Copyright (C) 2006-2012 Free Software Foundation, Inc.
  
  ;; Author: Edward O'Connor <ted@oconnor.cx>
! ;; Version: 1.3
  ;; Keywords: convenience
  
  ;; This file is part of GNU Emacs.
--- 3,9 ----
  ;; Copyright (C) 2006-2012 Free Software Foundation, Inc.
  
  ;; Author: Edward O'Connor <ted@oconnor.cx>
! ;; Version: 1.4
  ;; Keywords: convenience
  
  ;; This file is part of GNU Emacs.
***************
*** 48,53 ****
--- 48,54 ----
  ;; 2006-12-29 - XEmacs support, from Aidan Kehoe <kehoea@parhasard.net>.
  ;; 2008-02-21 - Installed in GNU Emacs.
  ;; 2011-10-17 - Patch `json-alist-p' and `json-plist-p' to avoid recursion -tzz
+ ;; 2012-10-25 - Added pretty-printed reformatting -Ryan Crum (ryan@ryancrum.org)
  
  ;;; Code:
  
***************
*** 99,104 **** If this has the same value as `json-false', you might not be able to
--- 100,123 ----
  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-default-separator ", "
+   "Value to use as an element seperator when encoding.")
+ 
+ (defvar json-encoding-current-separator json-encoding-default-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 ""
+   "Internally used to keep track of the current indentation level of
+ encoding. Used only when `json-encoding-pretty-print' is non-nil.")
+ 
+ (defvar json-encoding-pretty-print nil
+   "Setting this to non-nil will result in the output of `json-encode'
+ to be pretty-printed.")
+ 
  \f
  
  ;;; Utilities
***************
*** 389,429 **** Please see the documentation of `json-object-type' and `json-key-type'."
  
  (defun json-encode-hash-table (hash-table)
    "Return a JSON representation of HASH-TABLE."
!   (format "{%s}"
            (json-join
             (let (r)
!              (maphash
!               (lambda (k v)
!                 (push (format "%s:%s"
!                               (json-encode k)
!                               (json-encode v))
!                       r))
!               hash-table)
               r)
!            ", ")))
  
  ;; List encoding (including alists and plists)
  
  (defun json-encode-alist (alist)
    "Return a JSON representation of ALIST."
!   (format "{%s}"
!           (json-join (mapcar (lambda (cons)
!                                (format "%s:%s"
!                                        (json-encode (car cons))
!                                        (json-encode (cdr cons))))
!                              alist)
!                      ", ")))
  
  (defun json-encode-plist (plist)
    "Return a JSON representation of PLIST."
    (let (result)
      (while plist
!       (push (concat (json-encode (car plist))
!                     ":"
!                     (json-encode (cadr plist)))
!             result)
        (setq plist (cddr plist)))
!     (concat "{" (json-join (nreverse result) ", ") "}")))
  
  (defun json-encode-list (list)
    "Return a JSON representation of LIST.
--- 408,479 ----
  
  (defun json-encode-hash-table (hash-table)
    "Return a JSON representation of HASH-TABLE."
!   (format (if json-encoding-pretty-print "{\n%s\n%s}" "{%s}")
            (json-join
             (let (r)
!              (let ((json-encoding-current-indentation
!                     (if json-encoding-pretty-print
!                         (concat json-encoding-current-indentation
!                                 json-encoding-default-indentation)
!                       "")))
!                (maphash
!                 (lambda (k v)
!                   (push (format (if json-encoding-pretty-print "%s%s: %s" "%s%s:%s")
!                                 json-encoding-current-indentation
!                                 (json-encode k)
!                                 (json-encode v))
!                         r))
!                 hash-table))
               r)
!            json-encoding-current-separator)
!           json-encoding-current-indentation))
  
  ;; List encoding (including alists and plists)
  
  (defun json-encode-alist (alist)
    "Return a JSON representation of ALIST."
!   (format (if json-encoding-pretty-print "{\n%s\n%s}" "{%s}")
!           (json-join
!            (let ((json-encoding-current-indentation
!                   (if json-encoding-pretty-print
!                       (concat json-encoding-current-indentation
!                               json-encoding-default-indentation)
!                     "")))
!              (mapcar (lambda (cons)
!                        (format (if json-encoding-pretty-print
!                                    "%s%s: %s"
!                                  "%s%s:%s")
!                                json-encoding-current-indentation
!                                (json-encode (car cons))
!                                (json-encode (cdr cons))))
!                      alist))
!                      json-encoding-current-separator)
!           json-encoding-current-indentation))
  
  (defun json-encode-plist (plist)
    "Return a JSON representation of PLIST."
    (let (result)
      (while plist
!       (let ((json-encoding-current-indentation
!              (if json-encoding-pretty-print
!                  (concat json-encoding-current-indentation
!                          json-encoding-default-indentation)
!                "")))
!         (push (concat
!                json-encoding-current-indentation
!                (json-encode (car plist))
!                (if json-encoding-current-indentation
!                    ": "
!                  ":")
!                (json-encode (cadr plist)))
!               result))
        (setq plist (cddr plist)))
!     (concat (if json-encoding-pretty-print "{\n" "{")
!             (json-join (nreverse result) json-encoding-current-separator)
!             (if json-encoding-pretty-print
!                 (format "\n%s" json-encoding-current-indentation)
!               "")
!             "}")))
  
  (defun json-encode-list (list)
    "Return a JSON representation of LIST.
***************
*** 462,468 **** become JSON objects."
  
  (defun json-encode-array (array)
    "Return a JSON representation of ARRAY."
!   (concat "[" (mapconcat 'json-encode array ", ") "]"))
  
  \f
  
--- 512,530 ----
  
  (defun json-encode-array (array)
    "Return a JSON representation of 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)))
!          (concat (format "[\n%s" json-encoding-current-indentation)
!                  (json-join (mapcar 'json-encode array)
!                             (format "%s%s"
!                                     json-encoding-current-separator
!                                     json-encoding-current-indentation))))
!        (format "\n%s]" json-encoding-current-indentation))
!     (concat "[" (mapconcat 'json-encode array json-encoding-current-separator) "]")))
  
  \f
  
***************
*** 516,533 **** Advances point just past JSON object."
  
  (defun json-encode (object)
    "Return a JSON representation of OBJECT as a string."
!   (cond ((memq object (list t json-null json-false))
!          (json-encode-keyword object))
!         ((stringp object)      (json-encode-string object))
!         ((keywordp object)     (json-encode-string
!                                 (substring (symbol-name object) 1)))
!         ((symbolp object)      (json-encode-string
!                                 (symbol-name object)))
!         ((numberp object)      (json-encode-number object))
!         ((arrayp object)       (json-encode-array object))
!         ((hash-table-p object) (json-encode-hash-table object))
!         ((listp object)        (json-encode-list object))
!         (t                     (signal 'json-error (list object)))))
  
  (provide 'json)
  
--- 578,622 ----
  
  (defun json-encode (object)
    "Return a JSON representation of OBJECT as a string."
!   (let ((json-encoding-current-separator
!          (if json-encoding-pretty-print
!              (concat json-encoding-default-separator "\n")
!            json-encoding-default-separator)))
!     (cond ((memq object (list t json-null json-false))
!            (json-encode-keyword object))
!           ((stringp object)      (json-encode-string object))
!           ((keywordp object)     (json-encode-string
!                                   (substring (symbol-name object) 1)))
!           ((symbolp object)      (json-encode-string
!                                   (symbol-name object)))
!           ((numberp object)      (json-encode-number object))
!           ((arrayp object)       (json-encode-array object))
!           ((hash-table-p object) (json-encode-hash-table object))
!           ((listp object)        (json-encode-list object))
!           (t                     (signal 'json-error (list object))))))
! 
! ;; Pretty printing
! 
! (defun json-pretty-print-buffer ()
!   "Pretty-print current buffer."
!   (interactive)
!   (let* ((json-encoding-pretty-print t)
!          (json-string (json-encode (json-read-from-string (buffer-string))))
!          (buf (current-buffer)))
!     (with-current-buffer buf
!       (erase-buffer)
!       (insert json-string))))
! 
! (defun json-pretty-print ()
!   "Pretty-print selected region."
!   (interactive)
!   (unless mark-active
!     (error "No region selected."))
!   (let ((begin (region-beginning))
!         (end (region-end))
!         (json-encoding-pretty-print t))
!     (kill-region begin end)
!     (insert (json-encode (json-read-from-string (current-kill 0))))))
  
  (provide 'json)
  

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: Patch for pretty-printing in json.el
  2012-10-25 14:55 ` bug#12634: Patch for pretty-printing in json.el Ryan Crum
@ 2012-10-25 18:08   ` Stefan Monnier
  2012-10-27 19:31     ` Ryan Crum
  0 siblings, 1 reply; 18+ messages in thread
From: Stefan Monnier @ 2012-10-25 18:08 UTC (permalink / raw)
  To: Ryan Crum; +Cc: 12634

> I've consolidated my changes into json.el.  I have not yet received
> the form for copyright assignment in the mail, but I have
> requested it.

Great, thanks.

> Please advise me if you'd like me to make any changes here, and I'll
> be happy to accomodate.  Patch attached.

It looks OK overall, but I do have some comments:
- it would be better not to re-compute json-encoding-current-separator
  every time we call json-encode, since that function is called all
  the time.
  IOW, build it once in an external caller.  Or better yet: get rid of
  json-encoding-current-separator and add a "\n" at the beginning of
  json-encoding-current-indentation instead.
- you can use the "json--" prefix to indicate it is an internal
  variable/function.
- You could also prefer to place the closing ] at the end of the
  previous line, à la Lisp ;-)


        Stefan





^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: Patch for pretty-printing in json.el
  2012-10-25 18:08   ` Stefan Monnier
@ 2012-10-27 19:31     ` Ryan Crum
  2012-10-30 20:03       ` Stefan Monnier
  0 siblings, 1 reply; 18+ messages in thread
From: Ryan Crum @ 2012-10-27 19:31 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12634


[-- Attachment #1.1: Type: text/plain, Size: 1064 bytes --]

Cool, new patch attached. I've consolidated current-separator into
current-indentation and created a little private helper function
`json--current-whitespace' for the newline/indentation.

I've also created a var called `json-encoding-lisp-style-closings' per your
request. :-)

Just let me know if there's anything else.

Thanks,

-Ryan

On Thu, Oct 25, 2012 at 2:08 PM, Stefan Monnier <monnier@iro.umontreal.ca>wrote:

> It looks OK overall, but I do have some comments:
> - it would be better not to re-compute json-encoding-current-separator
>   every time we call json-encode, since that function is called all
>   the time.
>   IOW, build it once in an external caller.  Or better yet: get rid of
>   json-encoding-current-separator and add a "\n" at the beginning of
>   json-encoding-current-indentation instead.
> - you can use the "json--" prefix to indicate it is an internal
>   variable/function.
> - You could also prefer to place the closing ] at the end of the
>   previous line, à la Lisp ;-)
>
>
>         Stefan
>

[-- Attachment #1.2: Type: text/html, Size: 1465 bytes --]

[-- Attachment #2: json-pretty-printing --]
[-- Type: application/octet-stream, Size: 8853 bytes --]

*** a/json.el
--- b/json.el
***************
*** 3,9 ****
  ;; Copyright (C) 2006-2012 Free Software Foundation, Inc.
  
  ;; Author: Edward O'Connor <ted@oconnor.cx>
! ;; Version: 1.3
  ;; Keywords: convenience
  
  ;; This file is part of GNU Emacs.
--- 3,9 ----
  ;; Copyright (C) 2006-2012 Free Software Foundation, Inc.
  
  ;; Author: Edward O'Connor <ted@oconnor.cx>
! ;; Version: 1.4
  ;; Keywords: convenience
  
  ;; This file is part of GNU Emacs.
***************
*** 48,53 ****
--- 48,54 ----
  ;; 2006-12-29 - XEmacs support, from Aidan Kehoe <kehoea@parhasard.net>.
  ;; 2008-02-21 - Installed in GNU Emacs.
  ;; 2011-10-17 - Patch `json-alist-p' and `json-plist-p' to avoid recursion -tzz
+ ;; 2012-10-25 - Added pretty-printed reformatting -Ryan Crum (ryan@ryancrum.org)
  
  ;;; Code:
  
***************
*** 99,104 **** If this has the same value as `json-false', you might not be able to
--- 100,124 ----
  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-default-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 ""
+   "Internally used to keep track of the current indentation level of
+ encoding. Used only when `json-encoding-pretty-print' is non-nil.")
+ 
+ (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.")
+ 
  \f
  
  ;;; Utilities
***************
*** 387,429 **** Please see the documentation of `json-object-type' and `json-key-type'."
  
  ;; Hash table encoding
  
  (defun json-encode-hash-table (hash-table)
    "Return a JSON representation of HASH-TABLE."
!   (format "{%s}"
            (json-join
             (let (r)
!              (maphash
!               (lambda (k v)
!                 (push (format "%s:%s"
!                               (json-encode k)
!                               (json-encode v))
!                       r))
!               hash-table)
               r)
!            ", ")))
  
  ;; List encoding (including alists and plists)
  
  (defun json-encode-alist (alist)
    "Return a JSON representation of ALIST."
!   (format "{%s}"
!           (json-join (mapcar (lambda (cons)
!                                (format "%s:%s"
!                                        (json-encode (car cons))
!                                        (json-encode (cdr cons))))
!                              alist)
!                      ", ")))
  
  (defun json-encode-plist (plist)
    "Return a JSON representation of PLIST."
    (let (result)
      (while plist
!       (push (concat (json-encode (car plist))
!                     ":"
!                     (json-encode (cadr plist)))
!             result)
        (setq plist (cddr plist)))
!     (concat "{" (json-join (nreverse result) ", ") "}")))
  
  (defun json-encode-list (list)
    "Return a JSON representation of LIST.
--- 407,493 ----
  
  ;; Hash table encoding
  
+ (defun json--current-whitespace ()
+   (if json-encoding-pretty-print
+       (concat "\n" json-encoding-current-indentation)
+     ""))
+ 
  (defun json-encode-hash-table (hash-table)
    "Return a JSON representation of HASH-TABLE."
!   (format (if json-encoding-pretty-print "{%s%s}" "{%s}")
            (json-join
             (let (r)
!              (let ((json-encoding-current-indentation
!                     (if json-encoding-pretty-print
!                         (concat json-encoding-current-indentation
!                                 json-encoding-default-indentation)
!                       "")))
!                (maphash
!                 (lambda (k v)
!                   (push (format
!                          (if json-encoding-pretty-print
!                              "%s%s: %s"
!                            "%s%s:%s")
!                          (json--current-whitespace)
!                          (json-encode k)
!                          (json-encode v))
!                         r))
!                 hash-table))
               r)
!            json-encoding-default-separator)
!           (if json-encoding-lisp-style-closings
!               ""
!               (json--current-whitespace))))
  
  ;; List encoding (including alists and plists)
  
  (defun json-encode-alist (alist)
    "Return a JSON representation of ALIST."
!   (format (if json-encoding-pretty-print "{%s%s}" "{%s}")
!           (json-join
!            (let ((json-encoding-current-indentation
!                   (if json-encoding-pretty-print
!                       (concat json-encoding-current-indentation
!                               json-encoding-default-indentation)
!                     "")))
!              (mapcar (lambda (cons)
!                        (format (if json-encoding-pretty-print
!                                    "%s%s: %s"
!                                  "%s%s:%s")
!                                (json--current-whitespace)
!                                (json-encode (car cons))
!                                (json-encode (cdr cons))))
!                      alist))
!                      json-encoding-default-separator)
!           (if json-encoding-lisp-style-closings
!               ""
!             (json--current-whitespace))))
  
  (defun json-encode-plist (plist)
    "Return a JSON representation of PLIST."
    (let (result)
      (while plist
!       (let ((json-encoding-current-indentation
!              (if json-encoding-pretty-print
!                  (concat json-encoding-current-indentation
!                          json-encoding-default-indentation)
!                "")))
!         (push (concat
!                (json-encoding-current-indentation)
!                (json-encode (car plist))
!                (if json-encoding-pretty-print
!                    ": "
!                  ":")
!                (json-encode (cadr plist)))
!               result))
        (setq plist (cddr plist)))
!     (concat "{"
!             (json-join (nreverse result) json-encoding-default-separator)
!             (if (and json-encoding-pretty-print
!                      (not json-encoding-lisp-style-closings))
!                 (format "%s" (json--current-whitespace))
!               "")
!             "}")))
  
  (defun json-encode-list (list)
    "Return a JSON representation of LIST.
***************
*** 462,468 **** become JSON objects."
  
  (defun json-encode-array (array)
    "Return a JSON representation of ARRAY."
!   (concat "[" (mapconcat 'json-encode array ", ") "]"))
  
  \f
  
--- 526,549 ----
  
  (defun json-encode-array (array)
    "Return a JSON representation of 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)))
!          (concat (format "[%s" (json--current-whitespace))
!                  (json-join (mapcar 'json-encode array)
!                             (format "%s%s"
!                                     json-encoding-default-separator
!                                     (json--current-whitespace)))))
!        (format "%s]"
!                (if json-encoding-lisp-style-closings
!                    ""
!                  (json--current-whitespace))))
!     (concat "["
!             (mapconcat 'json-encode array json-encoding-default-separator)
!             "]")))
  
  \f
  
***************
*** 529,534 **** Advances point just past JSON object."
--- 610,638 ----
          ((listp object)        (json-encode-list object))
          (t                     (signal 'json-error (list object)))))
  
+ ;; Pretty printing
+ 
+ (defun json-pretty-print-buffer ()
+   "Pretty-print current buffer."
+   (interactive)
+   (let* ((json-encoding-pretty-print t)
+          (json-string (json-encode (json-read-from-string (buffer-string))))
+          (buf (current-buffer)))
+     (with-current-buffer buf
+       (erase-buffer)
+       (insert json-string))))
+ 
+ (defun json-pretty-print ()
+   "Pretty-print selected region."
+   (interactive)
+   (unless mark-active
+     (error "No region selected."))
+   (let ((begin (region-beginning))
+         (end (region-end))
+         (json-encoding-pretty-print t))
+     (kill-region begin end)
+     (insert (json-encode (json-read-from-string (current-kill 0))))))
+ 
  (provide 'json)
  
  ;;; json.el ends here

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: Patch for pretty-printing in json.el
  2012-10-27 19:31     ` Ryan Crum
@ 2012-10-30 20:03       ` Stefan Monnier
  2012-10-30 21:04         ` Ryan Crum
  0 siblings, 1 reply; 18+ messages in thread
From: Stefan Monnier @ 2012-10-30 20:03 UTC (permalink / raw)
  To: Ryan Crum; +Cc: 12634

> Cool, new patch attached. I've consolidated current-separator into
> current-indentation and created a little private helper function
> `json--current-whitespace' for the newline/indentation.

Thanks.  More questions/remarks:

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

- 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?

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

- 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.

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

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

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

> I've also created a var called `json-encoding-lisp-style-closings' per your
> request. :-)

Thanks.

> Just let me know if there's anything else.

I think that's enough nitpicking for now.


        Stefan





^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: Patch for pretty-printing in json.el
  2012-10-30 20:03       ` Stefan Monnier
@ 2012-10-30 21:04         ` Ryan Crum
  2012-11-15  1:56           ` Stefan Monnier
  0 siblings, 1 reply; 18+ messages in thread
From: Ryan Crum @ 2012-10-30 21:04 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12634


[-- Attachment #1.1: Type: text/plain, Size: 1659 bytes --]

OK, let's try this again.

New patch attached.

On Tue, Oct 30, 2012 at 4:03 PM, Stefan Monnier <monnier@iro.umontreal.ca>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.

[-- Attachment #1.2: Type: text/html, Size: 2931 bytes --]

[-- Attachment #2: json-pretty-printing.diff --]
[-- Type: application/octet-stream, Size: 8516 bytes --]

*** a/json.el
--- b/json.el
***************
*** 3,9 ****
  ;; Copyright (C) 2006-2012 Free Software Foundation, Inc.
  
  ;; Author: Edward O'Connor <ted@oconnor.cx>
! ;; Version: 1.3
  ;; Keywords: convenience
  
  ;; This file is part of GNU Emacs.
--- 3,9 ----
  ;; Copyright (C) 2006-2012 Free Software Foundation, Inc.
  
  ;; Author: Edward O'Connor <ted@oconnor.cx>
! ;; Version: 1.4
  ;; Keywords: convenience
  
  ;; This file is part of GNU Emacs.
***************
*** 48,53 ****
--- 48,54 ----
  ;; 2006-12-29 - XEmacs support, from Aidan Kehoe <kehoea@parhasard.net>.
  ;; 2008-02-21 - Installed in GNU Emacs.
  ;; 2011-10-17 - Patch `json-alist-p' and `json-plist-p' to avoid recursion -tzz
+ ;; 2012-10-25 - Added pretty-printed reformatting -Ryan Crum (ryan@ryancrum.org)
  
  ;;; Code:
  
***************
*** 99,104 **** If this has the same value as `json-false', you might not be able to
--- 100,124 ----
  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.")
+ 
+ (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.")
+ 
  \f
  
  ;;; Utilities
***************
*** 124,129 **** this around your call to `json-read' instead of `setq'ing it.")
--- 144,157 ----
                   'not-plist)))
    (null list))
  
+ (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))
+ 
  ;; Reader utilities
  
  (defsubst json-advance (&optional n)
***************
*** 402,442 **** Please see the documentation of `json-object-type' and `json-key-type'."
  
  (defun json-encode-hash-table (hash-table)
    "Return a JSON representation of HASH-TABLE."
!   (format "{%s}"
            (json-join
             (let (r)
!              (maphash
!               (lambda (k v)
!                 (push (format "%s:%s"
!                               (json-encode-key k)
!                               (json-encode v))
!                       r))
!               hash-table)
               r)
!            ", ")))
  
  ;; List encoding (including alists and plists)
  
  (defun json-encode-alist (alist)
    "Return a JSON representation of ALIST."
!   (format "{%s}"
!           (json-join (mapcar (lambda (cons)
!                                (format "%s:%s"
!                                        (json-encode-key (car cons))
!                                        (json-encode (cdr cons))))
!                              alist)
!                      ", ")))
  
  (defun json-encode-plist (plist)
    "Return a JSON representation of PLIST."
    (let (result)
!     (while plist
!       (push (concat (json-encode-key (car plist))
!                     ":"
!                     (json-encode (cadr plist)))
!             result)
!       (setq plist (cddr plist)))
!     (concat "{" (json-join (nreverse result) ", ") "}")))
  
  (defun json-encode-list (list)
    "Return a JSON representation of LIST.
--- 430,497 ----
  
  (defun json-encode-hash-table (hash-table)
    "Return a JSON representation of HASH-TABLE."
!   (format (if json-encoding-pretty-print "{%s%s}" "{%s}")
            (json-join
             (let (r)
!              (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)))
  
  ;; List encoding (including alists and plists)
  
  (defun json-encode-alist (alist)
    "Return a JSON representation of ALIST."
!   (format (if json-encoding-pretty-print "{%s%s}" "{%s}")
!           (json-join
!            (json--with-indentation
!              (mapcar (lambda (cons)
!                        (format (if json-encoding-pretty-print
!                                    "%s%s: %s"
!                                  "%s%s:%s")
!                                json-encoding-current-indentation
!                                (json-encode-key (car cons))
!                                (json-encode (cdr cons))))
!                      alist))
!                      json-encoding-separator)
!           (if json-encoding-lisp-style-closings
!               ""
!             json-encoding-current-indentation)))
  
  (defun json-encode-plist (plist)
    "Return a JSON representation of PLIST."
    (let (result)
!     (json--with-indentation
!       (while plist
!         (push (concat
!                json-encoding-current-indentation
!                (json-encode-key (car plist))
!                (if json-encoding-pretty-print
!                    ": "
!                  ":")
!                (json-encode (cadr plist)))
!               result)
!         (setq plist (cddr plist))))
!     (concat "{"
!             (json-join (nreverse result) json-encoding-separator)
!             (if (and json-encoding-pretty-print
!                      (not json-encoding-lisp-style-closings))
!                 json-encoding-current-indentation
!               "")
!             "}")))
  
  (defun json-encode-list (list)
    "Return a JSON representation of LIST.
***************
*** 475,481 **** become JSON objects."
  
  (defun json-encode-array (array)
    "Return a JSON representation of ARRAY."
!   (concat "[" (mapconcat 'json-encode array ", ") "]"))
  
  \f
  
--- 530,553 ----
  
  (defun json-encode-array (array)
    "Return a JSON representation of 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)))
!          (concat (format "[%s" json-encoding-current-indentation)
!                  (json-join (mapcar 'json-encode array)
!                             (format "%s%s"
!                                     json-encoding-separator
!                                     json-encoding-current-indentation))))
!        (format "%s]"
!                (if json-encoding-lisp-style-closings
!                    ""
!                  json-encoding-current-indentation)))
!     (concat "["
!             (mapconcat 'json-encode array json-encoding-separator)
!             "]")))
  
  \f
  
***************
*** 542,547 **** Advances point just past JSON object."
--- 614,642 ----
          ((listp object)        (json-encode-list object))
          (t                     (signal 'json-error (list object)))))
  
+ ;; Pretty printing
+ 
+ (defun json-pretty-print-buffer ()
+   "Pretty-print current buffer."
+   (interactive)
+   (let* ((json-encoding-pretty-print t)
+          (json-string (json-encode (json-read-from-string (buffer-string))))
+          (buf (current-buffer)))
+     (with-current-buffer buf
+       (erase-buffer)
+       (insert json-string))))
+ 
+ (defun json-pretty-print ()
+   "Pretty-print selected region."
+   (interactive)
+   (unless mark-active
+     (error "No region selected."))
+   (let ((begin (region-beginning))
+         (end (region-end))
+         (json-encoding-pretty-print t))
+     (kill-region begin end)
+     (insert (json-encode (json-read-from-string (current-kill 0))))))
+ 
  (provide 'json)
  
  ;;; json.el ends here

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: Patch for pretty-printing in json.el
  2012-10-30 21:04         ` Ryan Crum
@ 2012-11-15  1:56           ` Stefan Monnier
  2012-11-17 17:37             ` Ryan Crum
  0 siblings, 1 reply; 18+ messages in thread
From: Stefan Monnier @ 2012-11-15  1:56 UTC (permalink / raw)
  To: Ryan Crum; +Cc: 12634

> 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





^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: Patch for pretty-printing in json.el
  2012-11-15  1:56           ` Stefan Monnier
@ 2012-11-17 17:37             ` Ryan Crum
  2012-11-20 18:52               ` Stefan Monnier
  0 siblings, 1 reply; 18+ messages in thread
From: Ryan Crum @ 2012-11-17 17:37 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12634

[-- Attachment #1: Type: text/plain, Size: 186 bytes --]

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.


[-- Attachment #2: json-pretty-print.diff --]
[-- Type: application/octet-stream, Size: 8756 bytes --]

*** a/json.el
--- b/json.el
***************
*** 3,9 ****
  ;; Copyright (C) 2006-2012 Free Software Foundation, Inc.
  
  ;; Author: Edward O'Connor <ted@oconnor.cx>
! ;; Version: 1.3
  ;; Keywords: convenience
  
  ;; This file is part of GNU Emacs.
--- 3,9 ----
  ;; Copyright (C) 2006-2012 Free Software Foundation, Inc.
  
  ;; Author: Edward O'Connor <ted@oconnor.cx>
! ;; Version: 1.4
  ;; Keywords: convenience
  
  ;; This file is part of GNU Emacs.
***************
*** 48,53 ****
--- 48,54 ----
  ;; 2006-12-29 - XEmacs support, from Aidan Kehoe <kehoea@parhasard.net>.
  ;; 2008-02-21 - Installed in GNU Emacs.
  ;; 2011-10-17 - Patch `json-alist-p' and `json-plist-p' to avoid recursion -tzz
+ ;; 2012-10-25 - Added pretty-printed reformatting -Ryan Crum (ryan@ryancrum.org)
  
  ;;; Code:
  
***************
*** 99,104 **** If this has the same value as `json-false', you might not be able to
--- 100,122 ----
  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.")
+ 
+ (defvar json-encoding-pretty-print nil
+   "If non-nil, then the output of `json-encode' will be pretty-printed.")
+ 
+ (defvar json-encoding-lisp-style-closings nil
+   "If non-nil, ] and } closings will be formatted lisp-style, without indentation.")
+ 
  \f
  
  ;;; Utilities
***************
*** 124,129 **** this around your call to `json-read' instead of `setq'ing it.")
--- 142,155 ----
                   'not-plist)))
    (null list))
  
+ (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))
+ 
  ;; Reader utilities
  
  (defsubst json-advance (&optional n)
***************
*** 402,442 **** Please see the documentation of `json-object-type' and `json-key-type'."
  
  (defun json-encode-hash-table (hash-table)
    "Return a JSON representation of HASH-TABLE."
!   (format "{%s}"
            (json-join
             (let (r)
!              (maphash
!               (lambda (k v)
!                 (push (format "%s:%s"
!                               (json-encode-key k)
!                               (json-encode v))
!                       r))
!               hash-table)
               r)
!            ", ")))
  
  ;; List encoding (including alists and plists)
  
  (defun json-encode-alist (alist)
    "Return a JSON representation of ALIST."
!   (format "{%s}"
!           (json-join (mapcar (lambda (cons)
!                                (format "%s:%s"
!                                        (json-encode-key (car cons))
!                                        (json-encode (cdr cons))))
!                              alist)
!                      ", ")))
  
  (defun json-encode-plist (plist)
    "Return a JSON representation of PLIST."
    (let (result)
!     (while plist
!       (push (concat (json-encode-key (car plist))
!                     ":"
!                     (json-encode (cadr plist)))
!             result)
!       (setq plist (cddr plist)))
!     (concat "{" (json-join (nreverse result) ", ") "}")))
  
  (defun json-encode-list (list)
    "Return a JSON representation of LIST.
--- 428,497 ----
  
  (defun json-encode-hash-table (hash-table)
    "Return a JSON representation of HASH-TABLE."
!   (format "{%s%s}"
            (json-join
             (let (r)
!              (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 (or (not json-encoding-pretty-print)
!                   json-encoding-lisp-style-closings)
!               ""
!             json--encoding-current-indentation)))
  
  ;; List encoding (including alists and plists)
  
  (defun json-encode-alist (alist)
    "Return a JSON representation of ALIST."
!   (format "{%s%s}"
!           (json-join
!            (json--with-indentation
!             (mapcar (lambda (cons)
!                       (format (if json-encoding-pretty-print
!                                   "%s%s: %s"
!                                 "%s%s:%s")
!                               json--encoding-current-indentation
!                               (json-encode-key (car cons))
!                               (json-encode (cdr cons))))
!                     alist))
!            json-encoding-separator)
!           (if (or (not json-encoding-pretty-print)
!                   json-encoding-lisp-style-closings)
!               ""
!             json--encoding-current-indentation)))
  
  (defun json-encode-plist (plist)
    "Return a JSON representation of PLIST."
    (let (result)
!     (json--with-indentation
!       (while plist
!         (push (concat
!                json--encoding-current-indentation
!                (json-encode-key (car plist))
!                (if json-encoding-pretty-print
!                    ": "
!                  ":")
!                (json-encode (cadr plist)))
!               result)
!         (setq plist (cddr plist))))
!     (concat "{"
!             (json-join (nreverse result) json-encoding-separator)
!             (if (and json-encoding-pretty-print
!                      (not json-encoding-lisp-style-closings))
!                 json--encoding-current-indentation
!               "")
!             "}")))
  
  (defun json-encode-list (list)
    "Return a JSON representation of LIST.
***************
*** 475,481 **** become JSON objects."
  
  (defun json-encode-array (array)
    "Return a JSON representation of ARRAY."
!   (concat "[" (mapconcat 'json-encode array ", ") "]"))
  
  \f
  
--- 530,551 ----
  
  (defun json-encode-array (array)
    "Return a JSON representation of ARRAY."
!   (if (and json-encoding-pretty-print
!            (> (length array) 0))
!       (concat
!        (json--with-indentation
!          (concat (format "[%s" json--encoding-current-indentation)
!                  (json-join (mapcar 'json-encode array)
!                             (format "%s%s"
!                                     json-encoding-separator
!                                     json--encoding-current-indentation))))
!        (format "%s]"
!                (if json-encoding-lisp-style-closings
!                    ""
!                  json--encoding-current-indentation)))
!     (concat "["
!             (mapconcat 'json-encode array json-encoding-separator)
!             "]")))
  
  \f
  
***************
*** 542,547 **** Advances point just past JSON object."
--- 612,644 ----
          ((listp object)        (json-encode-list object))
          (t                     (signal 'json-error (list object)))))
  
+ ;; Pretty printing
+ 
+ (defun json-pretty-print-buffer ()
+   "Pretty-print current buffer."
+   (interactive)
+   (let* ((json-encoding-pretty-print t)
+          (json-string (json-encode (json-read-from-string (buffer-string))))
+          (buf (current-buffer)))
+     (with-current-buffer buf
+       (erase-buffer)
+       (insert json-string))))
+ 
+ (defun json-pretty-print ()
+   "Pretty-print selected region."
+   (interactive)
+   (unless mark-active
+     (error "No region selected."))
+   (let ((begin (region-beginning))
+         (end (region-end))
+         (json-encoding-pretty-print t))
+     (kill-region begin end)
+     (condition-case-unless-debug err
+         (insert (json-encode (json-read-from-string (current-kill 0))))
+       (error
+         (insert (current-kill 0))
+         (error "Unable to parse JSON.")))))
+ 
  (provide 'json)
  
  ;;; json.el ends here

[-- Attachment #3: Type: text/plain, Size: 1815 bytes --]



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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: Patch for pretty-printing in json.el
  2012-11-17 17:37             ` Ryan Crum
@ 2012-11-20 18:52               ` Stefan Monnier
  2012-11-27 23:15                 ` Ryan Crum
  0 siblings, 1 reply; 18+ messages in thread
From: Stefan Monnier @ 2012-11-20 18:52 UTC (permalink / raw)
  To: Ryan Crum; +Cc: 12634

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

Hmm... AFAIK nowadays, you should receive the paperwork by email (it
used to be that it had to be printed by the FSF on their own paper, but
last I heard they now just send a PDF which you can print yourself), so
that should be fairly quick.

> New patch attached.

Looks fine, thank you.


        Stefan





^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: Patch for pretty-printing in json.el
  2012-11-20 18:52               ` Stefan Monnier
@ 2012-11-27 23:15                 ` Ryan Crum
  0 siblings, 0 replies; 18+ messages in thread
From: Ryan Crum @ 2012-11-27 23:15 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12634

Thanks. FYI, I've sent the signed paperwork in.

On Nov 20, 2012, at 1:52 PM, Stefan Monnier wrote:

>> I actually have not yet received anything regarding copyright assignment in
>> the mail, so I resubmitted the questionnaire to assign@gnu.org today.
> 
> Hmm... AFAIK nowadays, you should receive the paperwork by email (it
> used to be that it had to be printed by the FSF on their own paper, but
> last I heard they now just send a PDF which you can print yourself), so
> that should be fairly quick.
> 
>> New patch attached.
> 
> Looks fine, thank you.
> 
> 
>        Stefan






^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: Patch
  2012-10-13  8:39 bug#12634: 24.2; [FEATURE REQUEST] JSON pretty printer Leo
                   ` (2 preceding siblings ...)
  2012-10-25 14:55 ` bug#12634: Patch for pretty-printing in json.el Ryan Crum
@ 2012-12-14  2:56 ` Ryan Crum
  2012-12-14 14:59   ` Stefan Monnier
  3 siblings, 1 reply; 18+ messages in thread
From: Ryan Crum @ 2012-12-14  2:56 UTC (permalink / raw)
  To: 12634

[-- Attachment #1: Type: text/plain, Size: 78 bytes --]

The attached patch applied successfully for me against trunk at rev. 111221.


[-- Attachment #2: json-pretty-print.diff --]
[-- Type: application/octet-stream, Size: 8756 bytes --]

*** a/json.el
--- b/json.el
***************
*** 3,9 ****
  ;; Copyright (C) 2006-2012 Free Software Foundation, Inc.
  
  ;; Author: Edward O'Connor <ted@oconnor.cx>
! ;; Version: 1.3
  ;; Keywords: convenience
  
  ;; This file is part of GNU Emacs.
--- 3,9 ----
  ;; Copyright (C) 2006-2012 Free Software Foundation, Inc.
  
  ;; Author: Edward O'Connor <ted@oconnor.cx>
! ;; Version: 1.4
  ;; Keywords: convenience
  
  ;; This file is part of GNU Emacs.
***************
*** 48,53 ****
--- 48,54 ----
  ;; 2006-12-29 - XEmacs support, from Aidan Kehoe <kehoea@parhasard.net>.
  ;; 2008-02-21 - Installed in GNU Emacs.
  ;; 2011-10-17 - Patch `json-alist-p' and `json-plist-p' to avoid recursion -tzz
+ ;; 2012-10-25 - Added pretty-printed reformatting -Ryan Crum (ryan@ryancrum.org)
  
  ;;; Code:
  
***************
*** 99,104 **** If this has the same value as `json-false', you might not be able to
--- 100,122 ----
  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.")
+ 
+ (defvar json-encoding-pretty-print nil
+   "If non-nil, then the output of `json-encode' will be pretty-printed.")
+ 
+ (defvar json-encoding-lisp-style-closings nil
+   "If non-nil, ] and } closings will be formatted lisp-style, without indentation.")
+ 
  \f
  
  ;;; Utilities
***************
*** 124,129 **** this around your call to `json-read' instead of `setq'ing it.")
--- 142,155 ----
                   'not-plist)))
    (null list))
  
+ (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))
+ 
  ;; Reader utilities
  
  (defsubst json-advance (&optional n)
***************
*** 402,442 **** Please see the documentation of `json-object-type' and `json-key-type'."
  
  (defun json-encode-hash-table (hash-table)
    "Return a JSON representation of HASH-TABLE."
!   (format "{%s}"
            (json-join
             (let (r)
!              (maphash
!               (lambda (k v)
!                 (push (format "%s:%s"
!                               (json-encode-key k)
!                               (json-encode v))
!                       r))
!               hash-table)
               r)
!            ", ")))
  
  ;; List encoding (including alists and plists)
  
  (defun json-encode-alist (alist)
    "Return a JSON representation of ALIST."
!   (format "{%s}"
!           (json-join (mapcar (lambda (cons)
!                                (format "%s:%s"
!                                        (json-encode-key (car cons))
!                                        (json-encode (cdr cons))))
!                              alist)
!                      ", ")))
  
  (defun json-encode-plist (plist)
    "Return a JSON representation of PLIST."
    (let (result)
!     (while plist
!       (push (concat (json-encode-key (car plist))
!                     ":"
!                     (json-encode (cadr plist)))
!             result)
!       (setq plist (cddr plist)))
!     (concat "{" (json-join (nreverse result) ", ") "}")))
  
  (defun json-encode-list (list)
    "Return a JSON representation of LIST.
--- 428,497 ----
  
  (defun json-encode-hash-table (hash-table)
    "Return a JSON representation of HASH-TABLE."
!   (format "{%s%s}"
            (json-join
             (let (r)
!              (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 (or (not json-encoding-pretty-print)
!                   json-encoding-lisp-style-closings)
!               ""
!             json--encoding-current-indentation)))
  
  ;; List encoding (including alists and plists)
  
  (defun json-encode-alist (alist)
    "Return a JSON representation of ALIST."
!   (format "{%s%s}"
!           (json-join
!            (json--with-indentation
!             (mapcar (lambda (cons)
!                       (format (if json-encoding-pretty-print
!                                   "%s%s: %s"
!                                 "%s%s:%s")
!                               json--encoding-current-indentation
!                               (json-encode-key (car cons))
!                               (json-encode (cdr cons))))
!                     alist))
!            json-encoding-separator)
!           (if (or (not json-encoding-pretty-print)
!                   json-encoding-lisp-style-closings)
!               ""
!             json--encoding-current-indentation)))
  
  (defun json-encode-plist (plist)
    "Return a JSON representation of PLIST."
    (let (result)
!     (json--with-indentation
!       (while plist
!         (push (concat
!                json--encoding-current-indentation
!                (json-encode-key (car plist))
!                (if json-encoding-pretty-print
!                    ": "
!                  ":")
!                (json-encode (cadr plist)))
!               result)
!         (setq plist (cddr plist))))
!     (concat "{"
!             (json-join (nreverse result) json-encoding-separator)
!             (if (and json-encoding-pretty-print
!                      (not json-encoding-lisp-style-closings))
!                 json--encoding-current-indentation
!               "")
!             "}")))
  
  (defun json-encode-list (list)
    "Return a JSON representation of LIST.
***************
*** 475,481 **** become JSON objects."
  
  (defun json-encode-array (array)
    "Return a JSON representation of ARRAY."
!   (concat "[" (mapconcat 'json-encode array ", ") "]"))
  
  \f
  
--- 530,551 ----
  
  (defun json-encode-array (array)
    "Return a JSON representation of ARRAY."
!   (if (and json-encoding-pretty-print
!            (> (length array) 0))
!       (concat
!        (json--with-indentation
!          (concat (format "[%s" json--encoding-current-indentation)
!                  (json-join (mapcar 'json-encode array)
!                             (format "%s%s"
!                                     json-encoding-separator
!                                     json--encoding-current-indentation))))
!        (format "%s]"
!                (if json-encoding-lisp-style-closings
!                    ""
!                  json--encoding-current-indentation)))
!     (concat "["
!             (mapconcat 'json-encode array json-encoding-separator)
!             "]")))
  
  \f
  
***************
*** 542,547 **** Advances point just past JSON object."
--- 612,644 ----
          ((listp object)        (json-encode-list object))
          (t                     (signal 'json-error (list object)))))
  
+ ;; Pretty printing
+ 
+ (defun json-pretty-print-buffer ()
+   "Pretty-print current buffer."
+   (interactive)
+   (let* ((json-encoding-pretty-print t)
+          (json-string (json-encode (json-read-from-string (buffer-string))))
+          (buf (current-buffer)))
+     (with-current-buffer buf
+       (erase-buffer)
+       (insert json-string))))
+ 
+ (defun json-pretty-print ()
+   "Pretty-print selected region."
+   (interactive)
+   (unless mark-active
+     (error "No region selected."))
+   (let ((begin (region-beginning))
+         (end (region-end))
+         (json-encoding-pretty-print t))
+     (kill-region begin end)
+     (condition-case-unless-debug err
+         (insert (json-encode (json-read-from-string (current-kill 0))))
+       (error
+         (insert (current-kill 0))
+         (error "Unable to parse JSON.")))))
+ 
  (provide 'json)
  
  ;;; json.el ends here

[-- Attachment #3: Type: text/plain, Size: 2 bytes --]




^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#12634: Patch
  2012-12-14  2:56 ` bug#12634: Patch Ryan Crum
@ 2012-12-14 14:59   ` Stefan Monnier
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Monnier @ 2012-12-14 14:59 UTC (permalink / raw)
  To: Ryan Crum; +Cc: 12634-done

> The attached patch applied successfully for me against trunk at rev. 111221.

Thanks, installed, tho with the two commands rewritten (so as to not
affect the kill-ring, and so as to follow the usual convention of
receiving interactive args via the `interactive' spec, and so as to use
atomic-change-group).


        Stefan





^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2012-12-14 14:59 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-13  8:39 bug#12634: 24.2; [FEATURE REQUEST] JSON pretty printer Leo
2012-10-13 21:56 ` Stefan Monnier
2012-10-18 21:18 ` bug#12634: existing implementation Aurélien Aptel
2012-10-19  1:37   ` Stefan Monnier
2012-10-19 16:20     ` Aurélien Aptel
2012-10-19 18:04       ` Stefan Monnier
2012-10-19 21:28         ` Stefan Monnier
2012-10-25 14:55 ` bug#12634: Patch for pretty-printing in json.el Ryan Crum
2012-10-25 18:08   ` Stefan Monnier
2012-10-27 19:31     ` Ryan Crum
2012-10-30 20:03       ` Stefan Monnier
2012-10-30 21:04         ` Ryan Crum
2012-11-15  1:56           ` Stefan Monnier
2012-11-17 17:37             ` Ryan Crum
2012-11-20 18:52               ` Stefan Monnier
2012-11-27 23:15                 ` Ryan Crum
2012-12-14  2:56 ` bug#12634: Patch Ryan Crum
2012-12-14 14:59   ` Stefan Monnier

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).