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

[Michaël Cadilhac]

YAMAMOTO Mitsuharu <address@hidden> writes:

>>>>>> On Wed, 12 Sep 2007 15:13:06 +0200, address@hidden (Michaël Cadilhac) 
>>>>>> said:
>
>>> %-escaping in browse-url-file-url (filename -> url) and those in
>>> other places such as browse-url-netscape (url -> url) are
>>> inherently different operations.
>
>> Not quite.  I may have a too low-level point of view, but they are
>> the same operations, they are just not used for the same purpose.
>
> "Inherently different" might have been an exaggeration.  Do you agree
> that they are semantically different operations?

I'm not sure we will agree on the terms, but we're probably saying the
same thing :-)

>>> I think consolidating these two operations into one function only
>>> because they look similar is over-refactoring and shouldn't be done
>>> in order to avoid re-escaping or re-unescaping by mistake.
>
>> Well, I'm not so sure.  Their purpose is to escape characters in a
>> way we don't want to duplicate too much.  The «problem» you're
>> pointing out is the reason why I made the escaping function take the
>> set of characters to escape in a first place.
>
> IMO, differentiating them just by an argument makes the semantical
> difference more or less implicit.

IMO, the semantical difference is to be taken care of by the caller, not
this tool function.  All the callers need a function to url-encode a set
of chars in a string, so there's a point in creating such a function,
nop ?

> What is worse, the function name `browse-url-encode-url' looks as if
> it takes a URL as an argument.

I do agree that the function name doesn't tell the right thing.  At
first, I'd have preferred something like
`browse-url-encode-chars-in-string' (when the change consisted in
calling this function with the set of chars to be escaped), but if
factoring the regexp is not considered as over-factoring (which I'm not
sure), this name is too generic.

> I think operations on filename and URL should be deliberately
> separated (except for purely basic string operations).

This is IMO a basic string operation, but the current function is not
generic enough.

>> 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!

-- 
 |   Michaël `Micha' Cadilhac       |    Le second degré,                    |
 |   http://michael.cadilhac.name   |       c'est un peu                     |
 |   JID/MSN:                       |   le verlan sémantique.                |
 `----  address@hidden  |                                   -  --'

pgpZgz_vv0xYN.pgp
Description: PGP signature