[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Your last change to browse-url is bogus.
From: |
YAMAMOTO Mitsuharu |
Subject: |
Re: Your last change to browse-url is bogus. |
Date: |
Mon, 17 Sep 2007 13:40:49 +0900 |
User-agent: |
Wanderlust/2.14.0 (Africa) SEMI/1.14.6 (Maruoka) FLIM/1.14.8 (Shijō) APEL/10.6 Emacs/23.0.50 (sparc-sun-solaris2.8) MULE/5.0 (SAKAKI) |
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, address@hidden (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
address@hidden
- Re: Your last change to browse-url is bogus., (continued)
- Re: Your last change to browse-url is bogus., YAMAMOTO Mitsuharu, 2007/09/12
- Re: Your last change to browse-url is bogus., Michaël Cadilhac, 2007/09/12
- Re: Your last change to browse-url is bogus., YAMAMOTO Mitsuharu, 2007/09/12
- Re: Your last change to browse-url is bogus., Michaël Cadilhac, 2007/09/12
- Re: Your last change to browse-url is bogus.,
YAMAMOTO Mitsuharu <=
- Re: Your last change to browse-url is bogus., Michaël Cadilhac, 2007/09/17
- Re: Your last change to browse-url is bogus., Stephen J. Turnbull, 2007/09/12
- Re: Your last change to browse-url is bogus., Stefan Monnier, 2007/09/12
- Re: Your last change to browse-url is bogus., Stephen J. Turnbull, 2007/09/13
- Re: Your last change to browse-url is bogus., Stefan Monnier, 2007/09/13
- Re: Your last change to browse-url is bogus., Davis Herring, 2007/09/12