GNU bug report logs - #21798
25.0.50; [PATCH] Add support for retrieving paths to JSON elements

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 21798 in the body.
You can then email your comments to 21798 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Simen Heggestøyl <simenheg <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.0.50; [PATCH] Add support for retrieving paths to JSON elements
Date: Sat, 31 Oct 2015 09:46:02 +0100

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

Sometimes when working with a JSON structure, one needs to find the
path to a particular element. That can be tiresome to do manually,
especially if the structure is deep.

The proposed patch extends json.el with the ability to retrieve the
path to a particular JSON element.

See the following video for an example of how it can be used by an
interactive command, to show the path to the JSON element at point:

http://folk.uio.no/simenheg/json-mode.webm

The patch follows!

-- Simen


From f6ddd3b797d6b0d92a1ffa0f5db59543ac7cdc11 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Simen=20Heggest=C3=B8yl?= <simenheg <at> gmail.com>
Date: Sun, 25 Oct 2015 14:44:59 +0100
Subject: [PATCH] Add support for retrieving paths to JSON elements

Add support for retrieving the path to a JSON element. This can for
instance be useful to retrieve paths in deeply nested JSON
structures.

* lisp/json.el (json-path-to-position): New function for retrieving the
path to a JSON object at a given position.
(json--path-to-position, json--path): New variables, used internally by
`json-path-to-position'.
(json-read-object, json-read-array): Respect `json--path-to-position'.

* test/automated/json-tests.el (test-json-path-to-position-with-objects)
(test-json-path-to-position-with-arrays): New tests for
`json-path-to-position'.
---
lisp/json.el | 67 ++++++++++++++++++++++++++++++++++++++++++--
test/automated/json-tests.el | 14 +++++++++
2 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/lisp/json.el b/lisp/json.el
index b23d12a..2c82eee 100644
--- a/lisp/json.el
+++ b/lisp/json.el
@@ -196,6 +196,49 @@ 'json-end-of-file



+;;; Paths
+
+(defvar json--path-to-position nil
+ "When set to a position, `json-read' will return the path to
+the JSON element at the specified position, instead of returning
+the parsed JSON object.")
+
+(defvar json--path '()
+ "Used internally by `json-path-to-position' to keep track of
+the path during recursive calls to `json-read'.")
+
+(defun json-path-to-position (position &optional string)
+ "Return the path to the JSON element at POSITION.
+
+When STRING is provided, return the path to the position in the
+string, else to the position in the current buffer.
+
+The return value is a property list with the following
+properties:
+
+:path -- A list of strings and numbers forming the path to
+ the JSON element at the given position. Strings
+ denote object names, while numbers denote array
+ indexes.
+
+:match-start -- Position where the matched JSON element begins.
+
+:match-end -- Position where the matched JSON element ends.
+
+This can for instance be useful to determine the path to a JSON
+element in a deeply nested structure."
+ (save-excursion
+ (unless string
+ (goto-char (point-min)))
+ (let* ((json--path '())
+ (json--path-to-position position)
+ (path (catch :json-path
+ (if string
+ (json-read-from-string string)
+ (json-read)))))
+ (when (plist-get path :path)
+ path))))
+
;;; Keywords

(defvar json-keywords '("true" "false" "null")
@@ -399,11 +442,21 @@ json-read-object
    (while (not (char-equal (json-peek) ?}))
      (json-skip-whitespace)
      (setq key (json-read-string))
+ (when json--path-to-position
+ (push key json--path))
      (json-skip-whitespace)
      (if (char-equal (json-peek) ?:)
          (json-advance)
        (signal 'json-object-format (list ":" (json-peek))))
- (setq value (json-read))
+ (json-skip-whitespace)
+ (let ((start (point)))
+ (setq value (json-read))
+ (when json--path-to-position
+ (when (< start json--path-to-position (+ (point) 1))
+ (throw :json-path (list :path (nreverse json--path)
+ :match-start start
+ :match-end (point))))
+ (pop json--path)))
      (setq elements (json-add-to-object elements key value))
      (json-skip-whitespace)
      (unless (char-equal (json-peek) ?})
@@ -509,7 +562,17 @@ json-read-array
  ;; read values until "]"
  (let (elements)
    (while (not (char-equal (json-peek) ?\]))
- (push (json-read) elements)
+ (json-skip-whitespace)
+ (when json--path-to-position
+ (push (length elements) json--path))
+ (let ((start (point)))
+ (push (json-read) elements)
+ (when json--path-to-position
+ (when (< start json--path-to-position (+ (point) 1))
+ (throw :json-path (list :path (nreverse json--path)
+ :match-start start
+ :match-end (point))))
+ (pop json--path)))
      (json-skip-whitespace)
      (unless (char-equal (json-peek) ?\])
        (if (char-equal (json-peek) ?,)
diff --git a/test/automated/json-tests.el b/test/automated/json-tests.el
index d1b7a2f..e0672dd 100644
--- a/test/automated/json-tests.el
+++ b/test/automated/json-tests.el
@@ -49,5 +49,19 @@
  (should (equal (json-read-from-string 
"\"\\nasd\\u0444\\u044b\\u0432fgh\\t\"")
                 "\nasdфывfgh\t")))

+(ert-deftest test-json-path-to-position-with-objects ()
+ (let* ((json-string "{\"foo\": {\"bar\": {\"baz\": \"value\"}}}")
+ (matched-path (json-path-to-position 32 json-string)))
+ (should (equal (plist-get matched-path :path) '("foo" "bar" "baz")))
+ (should (equal (plist-get matched-path :match-start) 25))
+ (should (equal (plist-get matched-path :match-end) 32))))
+
+(ert-deftest test-json-path-to-position-with-arrays ()
+ (let* ((json-string "{\"foo\": [\"bar\", [\"baz\"]]}")
+ (matched-path (json-path-to-position 20 json-string)))
+ (should (equal (plist-get matched-path :path) '("foo" 1 0)))
+ (should (equal (plist-get matched-path :match-start) 18))
+ (should (equal (plist-get matched-path :match-end) 23))))
+
(provide 'json-tests)
;;; json-tests.el ends here
--
2.6.1

[Message part 2 (text/html, inline)]

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Simen Heggestøyl <simenheg <at> gmail.com>,
 21798 <at> debbugs.gnu.org
Subject: Re: bug#21798: 25.0.50; [PATCH] Add support for retrieving paths to
 JSON elements
Date: Sat, 31 Oct 2015 16:23:35 +0200

On 10/31/2015 10:46 AM, Simen Heggestøyl wrote:

> The proposed patch extends json.el with the ability to retrieve the
> path to a particular JSON element.
>
> See the following video for an example of how it can be used by an
> interactive command, to show the path to the JSON element at point:
>
> http://folk.uio.no/simenheg/json-mode.webm

Hi Simen,

The video looks great, but the inline patch lacks indentation, and 
doesn't apply (dunno if the former is the cause of the latter). Please 
resend it as an attachment.

Without trying it, my main concern would be any performance regression 
to json-read (it's not particularly fast already). Have you done any 
benchmarking?

Splitting the new changes into new functions might be more optimal.

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

From: Simen Heggestøyl <simenheg <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 21798 <at> debbugs.gnu.org
Subject: Re: bug#21798: 25.0.50; [PATCH] Add support for retrieving paths to
 JSON elements
Date: Sun, 01 Nov 2015 20:52:33 +0100

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

Hi Dmitry, thanks for the feedback!

On Sat, Oct 31, 2015 at 3:23 PM, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> The video looks great, but the inline patch lacks indentation, and 
> doesn't apply (dunno if the former is the cause of the latter). 
> Please resend it as an attachment.

Ah, yes, it appear that the whitespace got lost in the email for some
reason. I'll try attaching it to this email.

To test it, I've been using the following interactive function:

 (defun json-mode-show-path ()
   "Show the path to the JSON value under point."
   (interactive)
   (let ((path (json-path-to-position (point))))
     (if path
         (let ((formatted-path
                (json-mode--format-path (plist-get path :path))))
           (pulse-momentary-highlight-region
            (plist-get path :match-start)
            (plist-get path :match-end))
           (message formatted-path)
           (kill-new formatted-path))
       (message "Not a JSON value"))))

 (defun json-mode--format-path (path)
   "Return PATH formatted as a JSON data selector.
 PATH should be a list of keys, which can be either strings or
 integers."
   (mapconcat (lambda (key) (format "[%S]" key)) path ""))

It's also included it in json-mode.el:
http://folk.uio.no/simenheg/json-mode.el.

> Without trying it, my main concern would be any performance 
> regression to json-read (it's not particularly fast already). Have 
> you done any benchmarking?

I agree that would be bad. I'll try to produce some benchmarks when
I've got some free time on my hands!

-- Simen

[Message part 2 (text/html, inline)]

[0001-Add-support-for-retrieving-paths-to-JSON-elements.patch (text/x-patch, attachment)]

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

From: Simen Heggestøyl <simenheg <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 21798 <at> debbugs.gnu.org
Subject: Re: bug#21798: 25.0.50; [PATCH] Add support for retrieving paths to
 JSON elements
Date: Mon, 02 Nov 2015 00:27:46 +0100

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

Hello again, Dmitry.

I managed to produce a benchmark with the following JSON file (560K,
~10,000 lines): http://folk.uio.no/simenheg/huge.json.

I read it into `huge-json', and ran the following before the patch:

(benchmark-run 100 (json-read-from-string huge-json))
    ⇒ (19.421527195 938 7.448190372000017)

And after the patch:

(benchmark-run 100 (json-read-from-string huge-json))
    ⇒ (19.321667537 997 7.218977015999971)

-- Simen

On Sun, Nov 1, 2015 at 8:52 PM, Simen Heggestøyl <simenheg <at> gmail.com> 
wrote:
> Hi Dmitry, thanks for the feedback!
> 
> On Sat, Oct 31, 2015 at 3:23 PM, Dmitry Gutov <dgutov <at> yandex.ru> 
> wrote:
>> The video looks great, but the inline patch lacks indentation, and 
>> doesn't apply (dunno if the former is the cause of the latter). 
>> Please resend it as an attachment.
> 
> Ah, yes, it appear that the whitespace got lost in the email for some
> reason. I'll try attaching it to this email.
> 
> To test it, I've been using the following interactive function:
> 
>   (defun json-mode-show-path ()
>     "Show the path to the JSON value under point."
>     (interactive)
>     (let ((path (json-path-to-position (point))))
>       (if path
>           (let ((formatted-path
>                  (json-mode--format-path (plist-get path :path))))
>             (pulse-momentary-highlight-region
>              (plist-get path :match-start)
>              (plist-get path :match-end))
>             (message formatted-path)
>             (kill-new formatted-path))
>         (message "Not a JSON value"))))
> 
>   (defun json-mode--format-path (path)
>     "Return PATH formatted as a JSON data selector.
>   PATH should be a list of keys, which can be either strings or
>   integers."
>     (mapconcat (lambda (key) (format "[%S]" key)) path ""))
> 
> It's also included it in json-mode.el:
> http://folk.uio.no/simenheg/json-mode.el.
> 
>> Without trying it, my main concern would be any performance 
>> regression to json-read (it's not particularly fast already). Have 
>> you done any benchmarking?
> 
> I agree that would be bad. I'll try to produce some benchmarks when
> I've got some free time on my hands!
> 
> -- Simen

[Message part 2 (text/html, inline)]

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Simen Heggestøyl <simenheg <at> gmail.com>
Cc: 21798 <at> debbugs.gnu.org
Subject: Re: bug#21798: 25.0.50; [PATCH] Add support for retrieving paths to
 JSON elements
Date: Tue, 3 Nov 2015 04:00:56 +0200

Hello Simen,

On 11/02/2015 01:27 AM, Simen Heggestøyl wrote:

> I managed to produce a benchmark with the following JSON file (560K,
> ~10,000 lines): http://folk.uio.no/simenheg/huge.json.
>
> I read it into `huge-json', and ran the following before the patch:  ...

Thanks. In my testing, too, the difference seems to be statistically 
insignificant. That's good.

I have to say, I'm still not very comfortable with mixing it sort of 
alien logic inside json-read-object and json-read-array (would anyone 
else like to chime in with their opinion?).

I do believe we want this functionality, though. One option to tighten 
the implementation is to extract common pieces from json-read-object and 
json-read-array, and implement two new functions using them, but the 
while-loops used there will make avoiding just copying code somewhat 
difficult.

Here's an idea: both json-read-object-1 and json-read-array-2 will 
advise json-read to add the new logic around calls to it (there will 
have to be some guard in the advice, so that recursive calls are run 
unmodified).

And json-path-to-position will locally modify json-readtable to use 
json-read-object-1 and json-read-array-2.

That's just a suggestion, though.

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

From: Simen Heggestøyl <simenheg <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 21798 <at> debbugs.gnu.org
Subject: Re: bug#21798: 25.0.50; [PATCH] Add support for retrieving paths to
 JSON elements
Date: Fri, 06 Nov 2015 17:31:28 +0100

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

Hello Dmitry, thanks for the feedback.

On Tue, Nov 3, 2015 at 3:00 AM, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> I have to say, I'm still not very comfortable with mixing it sort of 
> alien logic inside json-read-object and json-read-array (would anyone 
> else like to chime in with their opinion?).

I understand your uneasiness, I feel the same way after you pointed it
out.

> Here's an idea: both json-read-object-1 and json-read-array-2 will 
> advise json-read to add the new logic around calls to it (there will 
> have to be some guard in the advice, so that recursive calls are run 
> unmodified).

I'm reluctant to use advises for it, not based on my own experience, but
based on advice from the Elisp manual:

> [...] advice should be reserved for the cases where you cannot modify
> a function’s behavior in any other way. [...] In particular, 
> Emacs’s
> own source files should not put advice on functions in Emacs. (There
> are currently a few exceptions to this convention, but we aim to
> correct them.)

Here we do have a chance to modify the functions' behavior.

How about a sort of compromise between our approaches: provide
'json-read-object' and 'json-read-array' with pre- and post-read
callback functions, that are only called when they're set. That would
make it possible to leverage the power of 'json-read-object' and
'json-read-array' by binding the callback functions, without mixing
alien logic into them.

That would also make it a lot cleaner when adding other extensions to
them in the future, compared to my original suggestion.

If that sounds good to you, I'll cook up a new patch!

-- Simen

[Message part 2 (text/html, inline)]

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Simen Heggestøyl <simenheg <at> gmail.com>
Cc: 21798 <at> debbugs.gnu.org
Subject: Re: bug#21798: 25.0.50; [PATCH] Add support for retrieving paths to
 JSON elements
Date: Fri, 6 Nov 2015 19:15:47 +0200

On 11/06/2015 06:31 PM, Simen Heggestøyl wrote:

> I'm reluctant to use advises for it, not based on my own experience, but
> based on advice from the Elisp manual:
>
>> [...] advice should be reserved for the cases where you cannot modify
>> a function’s behavior in any other way. [...] In particular, Emacs’s
>> own source files should not put advice on functions in Emacs. (There
>> are currently a few exceptions to this convention, but we aim to
>> correct them.)
>
> Here we do have a chance to modify the functions' behavior.

I happen to think that an advice with a short lifetime is okay, but yes, 
these are the guidelines.

> How about a sort of compromise between our approaches: provide
> 'json-read-object' and 'json-read-array' with pre- and post-read
> callback functions, that are only called when they're set. That would
> make it possible to leverage the power of 'json-read-object' and
> 'json-read-array' by binding the callback functions, without mixing
> alien logic into them.

That sounds fine to me in terms of design, but it might add some 
performance overhead. So some testing is needed.

> That would also make it a lot cleaner when adding other extensions to
> them in the future, compared to my original suggestion.
>
> If that sounds good to you, I'll cook up a new patch!

Please go ahead.

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

From: Richard Stallman <rms <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 21798 <at> debbugs.gnu.org, simenheg <at> gmail.com
Subject: Re: bug#21798: 25.0.50;
 [PATCH] Add support for retrieving paths to JSON elements
Date: Sat, 07 Nov 2015 08:23:29 -0500

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > I happen to think that an advice with a short lifetime is okay, but yes, 
  > these are the guidelines.

The reason that Emacs sources should not use advice is that advice is
bad for debugging.  If you look at function foo's source code, it
won't tell you that advice is causing the function to do something not
in that source.

This problem occurs no matter how long the advice is on function foo.

The right thing to do, instead of using advice on foo, is to change
foo to call a hook, and put things in that hook.

This is better because the source code of foo will include code
to run the hook.  When you see that, you'll know you should check
what is in the hook.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: rms <at> gnu.org
Cc: 21798 <at> debbugs.gnu.org, simenheg <at> gmail.com
Subject: Re: bug#21798: 25.0.50; [PATCH] Add support for retrieving paths to
 JSON elements
Date: Sat, 7 Nov 2015 15:43:12 +0200

On 11/07/2015 03:23 PM, Richard Stallman wrote:

> The reason that Emacs sources should not use advice is that advice is
> bad for debugging.  If you look at function foo's source code, it
> won't tell you that advice is causing the function to do something not
> in that source.

Yes still, if we support advice in the user code, we should be able to 
debug the result. One doesn't usually just reads the source code when 
debugging Emacs; edebug is very helpful in that, and it seems to support 
advice.

> This problem occurs no matter how long the advice is on function foo.

I don't think it's black-and-white: if an advice is only active during 
execution of just one or two commands, we don't have to worry about it 
most of the time. Neither when debugging, nor when running the code.

> The right thing to do, instead of using advice on foo, is to change
> foo to call a hook, and put things in that hook.

I agree it will be a simpler solution, if we can choose a meaningful 
place and name for the hook, and if there's no performance hit.

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

From: Simen Heggestøyl <simenheg <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 21798 <at> debbugs.gnu.org
Subject: Re: bug#21798: 25.0.50; [PATCH] Add support for retrieving paths to
 JSON elements
Date: Sat, 07 Nov 2015 19:50:15 +0100

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

On Fri, Nov 6, 2015 at 6:15 PM, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> That sounds fine to me in terms of design, but it might add some 
> performance overhead. So some testing is needed.

Good! A revised patch is attached.

Benchmarks follow below, with the same setup as last time.

Before the patch:

(benchmark-run 100 (json-read-from-string huge-json))
    ⇒ (16.84457266 1007 4.886441912999999)

After the patch:

(benchmark-run 100 (json-read-from-string huge-json))
    ⇒ (16.905379125000003 1007 4.722544520000007)

-- Simen

[Message part 2 (text/html, inline)]

[0001-Add-support-for-retrieving-paths-to-JSON-elements.patch (text/x-patch, attachment)]

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Simen Heggestøyl <simenheg <at> gmail.com>
Cc: 21798 <at> debbugs.gnu.org, Richard Stallman <rms <at> gnu.org>
Subject: Re: bug#21798: 25.0.50; [PATCH] Add support for retrieving paths to
 JSON elements
Date: Sat, 7 Nov 2015 21:18:11 +0200

On 11/07/2015 08:50 PM, Simen Heggestøyl wrote:

> Before the patch:
>
> (benchmark-run 100 (json-read-from-string huge-json))
>       ⇒ (16.84457266 1007 4.886441912999999)
>
> After the patch:
>
> (benchmark-run 100 (json-read-from-string huge-json))
>       ⇒ (16.905379125000003 1007 4.722544520000007)

Looks good enough, thanks.

> +(defvar json-pre-read-function nil
> +  "Function called (if non-nil) by `json-read-array' and
> +`json-read-object' right before reading a JSON array or object,
> +respectively.")
> +
> +(defvar json-post-read-function nil
> +  "Function called (if non-nil) by `json-read-array' and
> +`json-read-object' right after reading a JSON array or object,
> +respectively.")

Shouldn't they be called by `json-read' itself?

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

From: Simen Heggestøyl <simenheg <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 21798 <at> debbugs.gnu.org
Subject: Re: bug#21798: 25.0.50; [PATCH] Add support for retrieving paths to
 JSON elements
Date: Sun, 08 Nov 2015 13:32:13 +0100

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

Thanks again for your feedback, Dmitry.

On Sat, Nov 7, 2015 at 8:18 PM, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> Shouldn't they be called by `json-read' itself?

Yes, maybe they should. The only drawback I see is that 'json-read' then
needs to accept an optional parameter, which is the JSON key of the
current element.

A revised patch implementing your suggestion is attached.

Benchmarks follow below, with the usual setup!

Before the patch:

(benchmark-run 100 (json-read-from-string huge-json))
    ⇒ (18.782874379000003 1007 5.674178575000008)

After the patch:

(benchmark-run 100 (json-read-from-string huge-json))
    ⇒ (18.233328517999997 1007 4.907621599000008)

-- Simen

[Message part 2 (text/html, inline)]

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

From: Simen Heggestøyl <simenheg <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 21798 <at> debbugs.gnu.org
Subject: Re: bug#21798: 25.0.50; [PATCH] Add support for retrieving paths to
 JSON elements
Date: Sun, 08 Nov 2015 13:34:14 +0100

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

On Sun, Nov 8, 2015 at 1:32 PM, Simen Heggestøyl <simenheg <at> gmail.com> 
wrote:
> A revised patch implementing your suggestion is attached.

Sorry, I forgot to attach the patch.

Here it is!

-- Simen

[Message part 2 (text/html, inline)]

[0001-Add-support-for-retrieving-paths-to-JSON-elements.patch (text/x-patch, attachment)]

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Simen Heggestøyl <simenheg <at> gmail.com>
Cc: 21798 <at> debbugs.gnu.org
Subject: Re: bug#21798: 25.0.50; [PATCH] Add support for retrieving paths to
 JSON elements
Date: Sun, 8 Nov 2015 18:16:32 +0200

Hi again, Simen.

On 11/08/2015 02:32 PM, Simen Heggestøyl wrote:

> Yes, maybe they should. The only drawback I see is that 'json-read' then
> needs to accept an optional parameter, which is the JSON key of the
> current element.
>
> A revised patch implementing your suggestion is attached.

Thank you, but since `json-read' needs an optional argument in this case 
(which is not trivial to describe), I'd rather we go back to the 
previous patch.

How about renaming json-pre-read-function to 
json-pre-element-read-function, maybe? And same for the other one.

If you're fine with that, please install the result. (Or without the 
rename, it's up to you).

> Benchmarks follow below, with the usual setup!
>
> Before the patch:
>
> (benchmark-run 100 (json-read-from-string huge-json))
>       ⇒ (18.782874379000003 1007 5.674178575000008)
>
> After the patch:
>
> (benchmark-run 100 (json-read-from-string huge-json))
>       ⇒ (18.233328517999997 1007 4.907621599000008)

Nice! The extra code made it faster. :)

Apparently, the difference is within the margin of error.

Message #49 received at 21798-done <at> debbugs.gnu.org (full text, mbox):

From: Simen Heggestøyl <simenheg <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 21798-done <at> debbugs.gnu.org
Subject: Re: bug#21798: 25.0.50; [PATCH] Add support for retrieving paths to
 JSON elements
Date: Sun, 08 Nov 2015 22:12:39 +0100

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

On Sun, Nov 8, 2015 at 5:16 PM, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> Thank you, but since `json-read' needs an optional argument in this 
> case (which is not trivial to describe), I'd rather we go back to the 
> previous patch.
> 
> How about renaming json-pre-read-function to 
> json-pre-element-read-function, maybe? And same for the other one.
> 
> If you're fine with that, please install the result. (Or without the 
> rename, it's up to you).

I installed the previous patch with the renaming you suggested.

Thanks for your reviews!

By the way, if you find the time, could you give bug #21616 a quick
look as well? It teaches the JSON encoder how to sort objects by their
keys. I think it would be nice to get it in before the feature freeze,
since it's the last piece of the puzzle to enable the new json-mode to
achieve feature parity with the old one.

-- Simen

[Message part 2 (text/html, inline)]

Message #52 received at 21798-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Simen Heggestøyl <simenheg <at> gmail.com>
Cc: 21798-done <at> debbugs.gnu.org
Subject: Re: bug#21798: 25.0.50; [PATCH] Add support for retrieving paths to
 JSON elements
Date: Mon, 9 Nov 2015 02:20:28 +0200

On 11/08/2015 11:12 PM, Simen Heggestøyl wrote:

> I installed the previous patch with the renaming you suggested.
>
> Thanks for your reviews!

Thank you for the code.

> By the way, if you find the time, could you give bug #21616 a quick
> look as well? It teaches the JSON encoder how to sort objects by their
> keys. I think it would be nice to get it in before the feature freeze,
> since it's the last piece of the puzzle to enable the new json-mode to
> achieve feature parity with the old one.

I agree, and done.

Previous Next

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