From: Stefan Monnier <monnier@iro.umontreal.ca>
To: Ryan Crum <ryan.j.crum@gmail.com>
Cc: 12634@debbugs.gnu.org
Subject: bug#12634: Patch for pretty-printing in json.el
Date: Wed, 14 Nov 2012 20:56:30 -0500 [thread overview]
Message-ID: <jwvtxsry70o.fsf-monnier+emacs@gnu.org> (raw)
In-Reply-To: <CAMiu-qDh+hfZh=o7tfz4MTC+yLeBVzHG6-5i=W+tG8y_z+9dxA@mail.gmail.com> (Ryan Crum's message of "Tue, 30 Oct 2012 17:04:52 -0400")
> 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
next prev parent reply other threads:[~2012-11-15 1:56 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvtxsry70o.fsf-monnier+emacs@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=12634@debbugs.gnu.org \
--cc=ryan.j.crum@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.