Re: Your last change to browse-url is bogus.

[YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>]

Your point of view is low-level (string operation), and mine is
high-level (URL operation).  The problems is that the current
implementation of browse-url-encode-url is neither of them: it looks
like a result of superficial code factoring.

(defun browse-url-encode-url (url &optional filename-p)
  "Encode all `confusing' characters in URL.
If FILENAME-P is nil, the confusing characters are [,)$].
Otherwise, the confusing characters are [*\"()',=;?% ]."
  (let ((conf-char (if filename-p "[*\"()',=;?% ]" "[,)$]"))
	(encoded-url (copy-sequence url))
	(s 0))
    (while (setq s (string-match conf-char encoded-url s))
      (setq encoded-url
	    (replace-match (format "%%%x"
				   (string-to-char (match-string 0 encoded-url)))
			   t t encoded-url)
	    s (1+ s)))
    encoded-url))

It is inappropriate from a high-level view because it is a mixture of
two semantically different operations.  Also inappropriate from a
low-level view because it is not general enough and the function name
doesn't represent what it does.

>>>>> On Thu, 13 Sep 2007 00:50:59 +0200, michael@cadilhac.name (Michaël Cadilhac) said:

>>> What would you do?
>> 
>> Maybe I would revert browse-url-file-url to the one that doesn't
>> use browse-url-encode-url [...].  Also I would use more explicit
>> and specific name in place of browse-url-encode-url (e.g.,
>> browse-url-escape-confusing-characters).

>> An alternative way would be, as you suggested, to give characters
>> to be escaped as an argument to browse-url-encode-url, but rename
>> the function so it looks like a low-level string operation.

> Well, for now on I'm not sure which one of the possibilities is the
> good one (and I'm getting sleepy :)).  If someone has a strong
> feeling about that around here, it'd be nice to hear him!

I propose the combination of above two, which is similar to what Davis
suggested:

  1. Add a low-level string replacing function (say,
     browse-url-encode-chars-in-string as you mentioned) that takes an
     argument representing the characters to be escaped.

  2. In browse-url-file-url, directly call the above low-level
     function instead of calling browse-url-encode-url.

  3. Change browse-url-encode-url so it calls the the low-level
     function introduced in 1, and just do one task (URL -> URL).

The difference between direct and indirect calls to the low-level
function comes from whether it is meaningful as a self-contained task
or not.  Escaping confusing characters in a URL deserves a specialized
function, but escaping characters in a filename string is just a part
of the conversion from the filename to a URL.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp