all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: michael@cadilhac.name (Michaël Cadilhac)
To: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
Cc: hannes@saeurebad.de, emacs-devel@gnu.org
Subject: Re: Your last change to browse-url is bogus.
Date: Thu, 13 Sep 2007 00:50:59 +0200	[thread overview]
Message-ID: <87fy1j1orw.fsf@cadilhac.name> (raw)
In-Reply-To: <20070912.231800.210864712.mituharu@math.s.chiba-u.ac.jp> (YAMAMOTO Mitsuharu's message of "Wed, 12 Sep 2007 23:18:00 +0900 (JST)")


[-- Attachment #1.1: Type: text/plain, Size: 3173 bytes --]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

>>>>>> On Wed, 12 Sep 2007 15:13:06 +0200, michael@cadilhac.name (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.                |
 `----  michael.cadilhac@gmail.com  |                                   -  --'

[-- Attachment #1.2: Type: application/pgp-signature, Size: 188 bytes --]

[-- Attachment #2: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

  reply	other threads:[~2007-09-12 22:50 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-12  9:13 Your last change to browse-url is bogus Michaël Cadilhac
2007-09-12 10:20 ` Johannes Weiner
2007-09-12 10:38   ` Michaël Cadilhac
2007-09-12 10:49     ` Johannes Weiner
2007-09-12 11:09       ` Michaël Cadilhac
2007-09-12 11:25         ` Johannes Weiner
2007-09-12 11:46           ` Michaël Cadilhac
2007-09-12 12:54     ` YAMAMOTO Mitsuharu
2007-09-12 13:13       ` Michaël Cadilhac
2007-09-12 14:18         ` YAMAMOTO Mitsuharu
2007-09-12 22:50           ` Michaël Cadilhac [this message]
2007-09-17  4:40             ` YAMAMOTO Mitsuharu
2007-09-17 11:52               ` Michaël Cadilhac
2007-09-12 23:29         ` Stephen J. Turnbull
2007-09-13  2:30           ` Stefan Monnier
2007-09-13 18:15             ` Stephen J. Turnbull
2007-09-13 21:44               ` Stefan Monnier
2007-09-13  2:39           ` Davis Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87fy1j1orw.fsf@cadilhac.name \
    --to=michael@cadilhac.name \
    --cc=emacs-devel@gnu.org \
    --cc=hannes@saeurebad.de \
    --cc=mituharu@math.s.chiba-u.ac.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.