8:38 p.m.
>>>> "Hrvoje" == Hrvoje Niksic
<hniksic(a)iskon.hr> writes:
Hrvoje> I'm not sure that patch is right. The DEFAULT argument
Hrvoje> didn't exist in pre-21.2 XEmacs, and this patch leaves it
Hrvoje> in. That looks wrong.
Hi Hrvoje, Andreas, Yoshiki!
upon further examination I agree with Hrvoje. My quick patch is
wrong.
Doing it right (following Kyle's suggestion) would in this particular
case lead to awkward code bloat compared to the version before
Yoshiki's patch. The new feature of `completing-read', the DEFAULT
argument, cannot be relied upon (because XEmacs-21.1.8 does not have
it). Consequently the defaulting of an empty completion has to be
done just as before. Only, handling a possible
wrong-number-of-arguments makes the code unnecessarily hard to read
and understand.
I vote for reverting to the `bookmark-completing-read' version of
bookmark.el revision 1.2.
Please advise,
Adrian
Possible new version (bad, bad, bad):
(defun bookmark-completing-read (prompt &optional default)
"Prompting with PROMPT, read a bookmark name in completion.
PROMPT will get a \": \" stuck on the end no matter what, so you
probably don't want to include one yourself.
Optional second arg DEFAULT is a string to return if the user enters
the empty string."
(bookmark-maybe-load-default-file) ; paranoia
(let* ((completion-ignore-case bookmark-completion-ignore-case)
(default default)
(prompt (if default
(concat prompt (format " (%s): " default))
(concat prompt ": ")))
(str
(condition-case nil
(completing-read prompt
bookmark-alist
nil
0
nil
'bookmark-history
default)
(wrong-number-of-arguments
(completing-read prompt
bookmark-alist
nil
0
nil
'bookmark-history)))))
(if (string-equal "" str)
(list default)
(list str))))
`bookmark-completing-read' of version 1.2 of bookmark.el:
(defun bookmark-completing-read (prompt &optional default)
"Prompting with PROMPT, read a bookmark name in completion.
PROMPT will get a \": \" stuck on the end no matter what, so you
probably don't want to include one yourself.
Optional second arg DEFAULT is a string to return if the user enters
the empty string."
(bookmark-maybe-load-default-file) ; paranoia
(let* ((completion-ignore-case bookmark-completion-ignore-case)
(default default)
(prompt (if default
(concat prompt (format " (%s): " default))
(concat prompt ": ")))
(str
(completing-read prompt
bookmark-alist
nil
0
nil
'bookmark-history)))
(if (string-equal "" str)
(list default)
(list str))))
--
Adrian Aichner <adrian(a)xemacs.org>
10:49 p.m.
New subject: [PATCH] bookmark.el of edit-utils 1.43 (completing-read usage error)
Adrian Aichner writes:
>>>>> "Hrvoje" == Hrvoje Niksic
<hniksic(a)iskon.hr> writes:
Hrvoje> I'm not sure that patch is right. The DEFAULT argument
Hrvoje> didn't exist in pre-21.2 XEmacs, and this patch leaves it
Hrvoje> in. That looks wrong.
Hi Hrvoje, Andreas, Yoshiki!
upon further examination I agree with Hrvoje. My quick patch is
wrong.
Doing it right (following Kyle's suggestion) would in this particular
case lead to awkward code bloat compared to the version before
Yoshiki's patch.
I think we should get used to using and seeing code like this.
Otherwise we'll never use new API features in the packages.
1:07 a.m.
New subject: [PATCH] bookmark.el of edit-utils 1.43 (completing-read usage error)
>>>> "Kyle" == Kyle Jones
<kyle_jones(a)wonderworks.com> writes:
Kyle> Adrian Aichner writes:
> Doing it right (following Kyle's suggestion) would in this
> particular case lead to awkward code bloat compared to the
> version before Yoshiki's patch.
How can "doing it right" (your wording ;) be WRONG? :-)
Kyle> I think we should get used to using and seeing code like
Kyle> this. Otherwise we'll never use new API features in the
Kyle> packages.
Having seen Kyle's explanation of how it works, I don't have trouble
reading that code.
In fact, most such code is going to be very stylized (drop the last
few arguments). One possible solution would be to create a
`backwards-compat' package that would provide wrappers for such
functions (I don't know how to code this offhand so I'm not giving
examples) that (1) somehow transfers the current definition of (eg)
#'completing-read to someplace it can be found (maybe a property of
'completing-read?) and (2) redefines #'completing-read to call the old
definition inside a condition-case of the stylized form.
If this was done as a macro (I think it could easily be), functions
whose APIs changed in incompatible ways would stand out like a sore
thumb because the macro wouldn't be used.
Then a package would simply do
(unless (>= emacs-version api-version-package-uses)
(require 'backwards-compat))
at load.
New code bears very little burden. Old code doesn't break, it just
runs slow enough that the users demands an upgrade ;-)
And the `backwards-compat' package itself would thus document API
changes.
What do you think?
--
University of Tsukuba Tennodai 1-1-1 Tsukuba 305-8573 JAPAN
Institute of Policy and Planning Sciences Tel/fax: +81 (298) 53-5091
_________________ _________________ _________________ _________________
What are those straight lines for? "XEmacs rules."
5:43 a.m.
New subject: [PATCH] bookmark.el of edit-utils 1.43 (completing-read usage error)
First, sorry for inconvinience.
I think I had spent too much time hacking 21.2.
And sorry for not responding soon. I was too tired after
applying my patches so I went home before reading
xemacs-beta. And I don't have internet connection there. :-(
Adrian Aichner <aichner(a)ecf.teradyne.com> writes:
Doing it right (following Kyle's suggestion) would in this
particular
case lead to awkward code bloat compared to the version before
Yoshiki's patch. The new feature of `completing-read', the DEFAULT
argument, cannot be relied upon (because XEmacs-21.1.8 does not have
it). Consequently the defaulting of an empty completion has to be
done just as before. Only, handling a possible
wrong-number-of-arguments makes the code unnecessarily hard to read
and understand.
I vote for reverting to the `bookmark-completing-read' version of
bookmark.el revision 1.2.
In this particular case, I think removing DEFAULT argument
is the best solution.
DEFAULT argument has three meanings.
1. The value is returned when user inputs empty string.
2. It will be put into the history when user enters empty
string.
3. Magical future history by M-n.
Probably 2 and 3 should be documented at Lispref.
First one can be easily simulated by something like
(setq val (completing-read "Prompt:" nil))
(if (string= val "")
(setq val default))
Second one can be simulates as:
(when (string= val "")
(setq val default)
(setq bookmark-history (cdr bookmark-history))
(or (eq bookmark-history t)
(null val)
(and bookmark-history (equal histval (car bookmark-history)))
(and (stringp val)
minibuffer-history-minimum-string-length
(< (length val)
minibuffer-history-minimum-string-length))
(set minibuffer-history-variable
(if minibuffer-history-uniquify
(cons histval (remove histval bookmark-history))
(cons histval bookmark-history)))))
And thrid one can be achieved by binding minibuffer-default
to default value.
First one is already done in bookmark.el. You can also do 2
and 3 but I don't think most one cares. It's relatively new
feature and I don't think supporing that is worth making
lisp code ugly.
Adrian, could you write a new patch?
--
Yoshiki Hayashi
8887
days inactive
8888
days old
3 comments
4 participants
participants (4)
-
Adrian Aichner -
Kyle Jones -
Stephen J. Turnbull -
Yoshiki Hayashi