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

[Ian Eure]

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@lifelogs.com> wrote: 
> 
> TZ> On Fri, 20 May 2011 11:37:45 -0700 Ian Eure <ian@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)