GNU bug report logs - #8706
24.0.50; [PATCH] Function to build a URL query-string

Previous Next

Full log

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

From: Ian Eure <ian <at> simplegeo.com>
To: Ted Zlatanov <tzz <at> lifelogs.com>,
 bug-gnu-emacs <at> gnu.org
Subject: Re: bug#8706: 24.0.50; [PATCH] Function to build a URL query-string
Date: Wed, 25 May 2011 09:32:15 -0700

On May 25, 2011, at 6:55 AM, Ted Zlatanov wrote:

> Ian, did you see my followup?
> 
I didn't; I'm not on the list, so I only get direct replies.	

> On Mon, 23 May 2011 09:36:12 -0500 Ted Zlatanov <tzz <at> lifelogs.com> wrote: 
> 
> TZ> On Fri, 20 May 2011 11:37:45 -0700 Ian Eure <ian <at> simplegeo.com> wrote: 
> IE> This patch adds a url-build-query-string method, which performs the
> IE> opposite job of url-parse-query-string. I find myself needing this
> IE> method in many projects, and having it in url-util.el seems
> IE> beneficial.
> 
> TZ> I think this is useful.
> 
> IE> +  (c-concat-separated
> IE> +   (mapcar (lambda (pair) (apply 'format "%s=%s" pair)) query) "&"))
> 
> TZ> I would use `mapconcat' (it's what `c-concat-separated' uses under the
> TZ> hood anyhow).
> 
Done.


> TZ> Also `format' errors out if it doesn't have enough arguments and it's
> TZ> legitimate to build a URL query like "url?x;y;z" where x, y, and z don't
> TZ> have values.  So I would change the lambda to (untested):
> 
> TZ> (lambda (pair)
> TZ>   (if (nth 1 pair)
> TZ>       (apply 'format "%s=%s" pair)
> TZ>     (format "%s" (car-safe pair))))
> 
> TZ> This also handles the case where `pair' is nil by design or by accident.
> 
You can do this by passing a sequence such as '((x "")), which results in "x=". I think this is fine, since this mirrors what you're actually doing — sending an empty string. I think if we wanted to accept lots of different formats, we'd need to do something like:

 1. Two-element sequences should work as they do now.
 2. One-element sequences should get an empty string appended.
 3. Invalid sequences ignored.

I'm not sure it makes sense to support #2, since it seems somewhat opaque and you can do the same thing with an empty string in scenario #1. #3 I'm not sure how to do without using remove-if from cl-seq.

I don't know why someone would pass in nil in place of a k/v pair, and it seems better to raise an error about that rather than silently accepting it, since it seems likely to be an error in the calling code.

I'm happy to discuss further if you think there's a good reason to change it, but I feel like this iteration is acceptable.


> TZ> Finally, the key and the value should be URL-encoded.  Do you assume
> TZ> that will be done before the function is called?
> 
An oversight on my part; an updated patch is attached. Thank you for the feedback.

--- url-util.el.orig	2011-05-25 09:09:03.000000000 -0700
+++ url-util.el	2011-05-25 09:09:47.000000000 -0700
@@ -281,6 +281,25 @@
 	  (setq retval (cons (list key val) retval)))))
     retval))
 
+;;;###autoload
+(defun url-build-query-string (query)
+  "Build a query-string.
+
+Given a QUERY in the form:
+'((key1 val1)
+  (key2 val2))
+
+This will return a string `key1=val1&key2=val2'. Keys may be strings
+or symbols; if they are symbols, the string name will be used."
+
+   (mapconcat
+    (lambda (pair)
+      (apply 'format "%s=%s"
+             (mapcar
+              (lambda (sym)
+                (url-hexify-string (if (symbolp sym) (symbol-name sym) sym)))
+              pair))) query "&"))
+
 (defun url-unhex (x)
   (if (> x ?9)
       (if (>= x ?a)

Previous Next

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