bug#21488: json-pretty-print-buffer works,but...

[Simen Heggestøyl]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
> Don't we also need to reverse (tho differently) if json-object-type
> is `plist'?
>
>
>         Stefan

Yes, updated:


>From cbe22b3bab6770512c43c2214ed11447d2a5b120 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg@gmail.com>
Date: Sat, 3 Oct 2015 11:31:16 +0200
Subject: [PATCH] Maintain ordering of JSON object keys by default

* lisp/json.el (json-object-type): Mention order handling in doc-string.
(json--plist-reverse): New utility function.
(json-read-object): Maintain ordering for alists and plists.
(json-pretty-print): Ensure that ordering is maintained.

* test/automated/json-tests.el (test-json-plist-reverse): New test for
`json--plist-reverse'.
(json-read-simple-alist): Update test to accommodate for changes in
`json-read-object'.

* etc/NEWS: Document the new behavior of the pretty printing functions.
---
 etc/NEWS                     |  5 +++++
 lisp/json.el                 | 21 +++++++++++++++++++--
 test/automated/json-tests.el | 13 ++++++++++---
 3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 26c478e..dbe0de3 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -314,6 +314,11 @@ standards.
 
 * Changes in Specialized Modes and Packages in Emacs 25.1
 
+** JSON
+---
+*** `json-pretty-print' and `json-pretty-print-buffer' now maintain
+the ordering of object keys by default.
+
 ** You can recompute the VC state of a file buffer with `M-x vc-refresh-state'
 ** Prog mode has some support for multi-mode indentation.
 See `prog-indentation-context' and `prog-widen'.
diff --git a/lisp/json.el b/lisp/json.el
index daa0c94..e2c7cc7 100644
--- a/lisp/json.el
+++ b/lisp/json.el
@@ -57,7 +57,8 @@
 (defvar json-object-type 'alist
   "Type to convert JSON objects to.
 Must be one of `alist', `plist', or `hash-table'.  Consider let-binding
-this around your call to `json-read' instead of `setq'ing it.")
+this around your call to `json-read' instead of `setq'ing it.  Ordering
+is maintained for `alist' and `plist', but not for `hash-table'.")
 
 (defvar json-array-type 'vector
   "Type to convert JSON arrays to.
@@ -136,6 +137,17 @@ without indentation.")
                  'not-plist)))
   (null list))
 
+(defun json--plist-reverse (plist)
+  "Return a copy of PLIST in reverse order.
+Unlike `reverse', this keeps the property-value pairs intact."
+  (let (res)
+    (while plist
+      (let ((prop (pop plist))
+            (val (pop plist)))
+        (push val res)
+        (push prop res)))
+    res))
+
 (defmacro json--with-indentation (body)
   `(let ((json--encoding-current-indentation
           (if json-encoding-pretty-print
@@ -400,7 +412,10 @@ Please see the documentation of `json-object-type' and 
`json-key-type'."
           (signal 'json-object-format (list "," (json-peek))))))
     ;; Skip over the "}"
     (json-advance)
-    elements))
+    (pcase json-object-type
+      (`alist (nreverse elements))
+      (`plist (json--plist-reverse elements))
+      (_ elements))))
 
 ;; Hash table encoding
 
@@ -602,6 +617,8 @@ Advances point just past JSON object."
   (interactive "r")
   (atomic-change-group
     (let ((json-encoding-pretty-print t)
+          ;; Ensure that ordering is maintained
+          (json-object-type 'alist)
           (txt (delete-and-extract-region begin end)))
       (insert (json-encode (json-read-from-string txt))))))
 
diff --git a/test/automated/json-tests.el b/test/automated/json-tests.el
index fd89b7a..d1b7a2f 100644
--- a/test/automated/json-tests.el
+++ b/test/automated/json-tests.el
@@ -22,15 +22,22 @@
 (require 'ert)
 (require 'json)
 
+(ert-deftest test-json-plist-reverse ()
+  (should (equal (json--plist-reverse '()) '()))
+  (should (equal (json--plist-reverse '(:a 1)) '(:a 1)))
+  (should (equal (json--plist-reverse '(:a 1 :b 2 :c 3))
+                 '(:c 3 :b 2 :a 1))))
+
 (ert-deftest json-encode-simple-alist ()
   (should (equal (json-encode '((a . 1)
                                 (b . 2)))
                  "{\"a\":1,\"b\":2}")))
 
 (ert-deftest json-read-simple-alist ()
-  (should (equal (json-read-from-string "{\"a\": 1, \"b\": 2}")
-                 '((b . 2)
-                   (a . 1)))))
+  (let ((json-object-type 'alist))
+    (should (equal (json-read-from-string "{\"a\": 1, \"b\": 2}")
+                   '((a . 1)
+                     (b . 2))))))
 
 (ert-deftest json-encode-string-with-special-chars ()
   (should (equal (json-encode-string "a\n\fb")
-- 
2.5.3