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

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

[Junqi Cai]

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

See https://github.com/pashky/restclient.el/issues/97

Sent from Mail for Windows 10

[-- Attachment #2: Type: text/html, Size: 1691 bytes --]

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

[Stefan Monnier]

> See https://github.com/pashky/restclient.el/issues/97

Please include a description of the problem right here, so it's archived
here and is available no matter what github turns into.


        Stefan

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

[Simen Heggestøyl]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> See https://github.com/pashky/restclient.el/issues/97
>
> Please include a description of the problem right here, so it's archived
> here and is available no matter what github turns into.
>
>
>         Stefan

The reporter's concern seems to be that `json-pretty-print' and
`json-pretty-print-buffer' reverse the order of JSON object keys. So
for instance, when you pretty print the following object:

{
  "a": 1,
  "b": 2
}

It becomes:

{
  "b": 2,
  "a": 1
}

JSON objects are by definition unordered, but I agree that it would be
nicer if the prettification functions maintained the ordering by
default.

Please consider the following patch, which makes the prettification
functions maintain the original ordering:


From f994203f746df3aa9fcf10592fd03d7aabe1f4f3 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-read-object): Maintain ordering for alists.
(json-pretty-print): Ensure that ordering is maintained.

* test/automated/json-tests.el (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                 | 9 +++++++--
 test/automated/json-tests.el | 7 ++++---
 3 files changed, 16 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.
 \f
 * 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..b0c7e9e 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 only for `alist'.")

 (defvar json-array-type 'vector
   "Type to convert JSON arrays to.
@@ -400,7 +401,9 @@ 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))
+    (if (eq json-object-type 'alist)
+        (nreverse elements)
+      elements)))

 ;; Hash table encoding

@@ -602,6 +605,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..c5a9c6c 100644
--- a/test/automated/json-tests.el
+++ b/test/automated/json-tests.el
@@ -28,9 +28,10 @@
                  "{\"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

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

[Stefan Monnier]

> JSON objects are by definition unordered, but I agree that it would be
> nicer if the prettification functions maintained the ordering by
> default.

Agreed.

> @@ -400,7 +401,9 @@ 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))
> +    (if (eq json-object-type 'alist)
> +        (nreverse elements)
> +      elements)))

Don't we also need to reverse (tho differently) if json-object-type
is `plist'?


        Stefan

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.
 \f
 * 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

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

[Stefan Monnier]

> Yes, updated:

Looks good, please install,


        Stefan

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

[Simen Heggestøyl]

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

On Sat, Oct 3, 2015 at 11:01 PM, Stefan Monnier 
<monnier@iro.umontreal.ca> wrote:
> Looks good, please install,
> 
> 
>         Stefan

Done. Thanks for reviewing!

-- Simen

[-- Attachment #2: Type: text/html, Size: 319 bytes --]

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

[蔡君琦]

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

Hi,
I copy json.el from github(https://github.com/emacs-mirror/emacs/tree/master/lisp/json.el), override local json.el and compile it.
After I restart emacs, and emacs shows this error in startup:
---
Warning (initialization): An error occurred while loading `c:/Users/caijunqi/AppData/Roaming/.emacs.d/init.el':

error: Attempt to modify read-only object, (error)

To ensure normal operation, you should investigate and remove the
cause of the error in your initialization file. Start Emacs with
the `--debug-init' option to view a complete error backtrace.
---

At 2015-10-04 05:58:11, "Simen Heggestøyl" <simenheg@gmail.com> wrote:
On Sat, Oct 3, 2015 at 11:01 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

Looks good, please install, Stefan


Done. Thanks for reviewing!


-- Simen

[-- Attachment #2: Type: text/html, Size: 1239 bytes --]

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

[Simen Heggestøyl]

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

On Fri, Oct 9, 2015 at 11:38 AM, 蔡君琦 <caijq4ever@163.com> wrote:
> Hi,
> I copy json.el from 
> github(https://github.com/emacs-mirror/emacs/tree/master/lisp/json.el), 
> override local json.el and compile it.
> After I restart emacs, and emacs shows this error in startup:
> ---
> Warning (initialization): An error occurred while loading 
> `c:/Users/caijunqi/AppData/Roaming/.emacs.d/init.el':
> 
> error: Attempt to modify read-only object, (error)

Hi again,

I'm not sure what that error comes from. Can you produce a backtrace?

It looks like the crash happens in your init file. Can you reproduce it
starting from 'emacs -Q'?

Also, I'm unsure whether it has to do with with the commit that fixed
this bug. Could you test with a version of json.el pre commit 6b663751?

-- Simen

[-- Attachment #2: Type: text/html, Size: 1057 bytes --]

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

[蔡君琦]

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

1. I'm sure this error comes from json.el.


2. I restart emacs with the 'emacs -Q' option, and emacs shows this buffer: 



3. I find this error comes from commit 6aa04a5(https://github.com/emacs-mirror/emacs/blob/6aa04a5570b50a68665e69231f3e36b2a4625f40/lisp/json.el). Please check it. 

在 2015-10-14 01:49:31，"Simen Heggestøyl" <simenheg@gmail.com> 写道：
On Fri, Oct 9, 2015 at 11:38 AM, 蔡君琦 <caijq4ever@163.com> wrote:
Hi,
I copy json.el from github(https://github.com/emacs-mirror/emacs/tree/master/lisp/json.el), override local json.el and compile it.
After I restart emacs, and emacs shows this error in startup:
---
Warning (initialization): An error occurred while loading `c:/Users/caijunqi/AppData/Roaming/.emacs.d/init.el':

error: Attempt to modify read-only object, (error)

Hi again,


I'm not sure what that error comes from. Can you produce a backtrace?


It looks like the crash happens in your init file. Can you reproduce it
starting from 'emacs -Q'?


Also, I'm unsure whether it has to do with with the commit that fixed
this bug. Could you test with a version of json.el pre commit 6b663751?


-- Simen

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

[-- Attachment #2: 截图1.png --]
[-- Type: image/png, Size: 31676 bytes --]

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

[Simen Heggestøyl]

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

Sorry, but I'm unable to tell what's wrong from the provided
information.

Since Emacs starts fine from 'emacs -Q', you could try to track down
which part of your initialization file is causing the error.

Either way, if you've found that the error stems from commit 6aa04a5,
I think that it deserves it's own bug ticket instead of discussing it
here, since it's unrelated to bug #21488.

When you report it, I think it would be good to include a backtrace to
make it easier to track down the cause of the error.

-- Simen

[-- Attachment #2: Type: text/html, Size: 668 bytes --]

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

[蔡君琦]

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

Hi,Simen:
This error caused by ‘end-of-file’ from 6b6637 at point (195,6).
When I remove ‘end-of-file’, it’s works.

194  (define-error 'json-end-of-file "End of file while parsing JSON" 'json-error)


-- Junqi

At 2015-10-17 21:25:33, “Simen Heggestøyl” simenheg@gmail.com wrote:

Sorry, but I'm unable to tell what's wrong from the provided
information.


Since Emacs starts fine from 'emacs -Q', you could try to track down
which part of your initialization file is causing the error.


Either way, if you've found that the error stems from commit 6aa04a5,
I think that it deserves it's own bug ticket instead of discussing it
here, since it's unrelated to bug #21488.


When you report it, I think it would be good to include a backtrace to
make it easier to track down the cause of the error.


-- Simen

[-- Attachment #2: Type: text/html, Size: 6012 bytes --]