(patch) sort cycling completion candidates (was: bbdb-complete-name return value)

(patch) sort cycling completion candidates (was: bbdb-complete-name return value)

[Ted Zlatanov]

[-- Attachment #1: Type: text/plain, Size: 146 bytes --]

Attached please find a patch to sort cycling completion candidates by
the text property :completion-cycle-penalty (lower is better).

Thanks
Ted


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: completion-penalty.patch --]
[-- Type: text/x-diff, Size: 1091 bytes --]

=== modified file 'lisp/minibuffer.el'
--- lisp/minibuffer.el	2011-02-12 18:30:13 +0000
+++ lisp/minibuffer.el	2011-03-08 14:51:07 +0000
@@ -704,7 +704,19 @@
         (when last
           (setcdr last nil)
           ;; Prefer shorter completions.
-          (setq all (sort all (lambda (c1 c2) (< (length c1) (length c2)))))
+          (setq
+           all
+           (sort all
+                 (lambda (c1 c2)
+                   (let ((s1 (get-text-property
+                              0 :completion-cycle-penalty c1))
+                         (s2 (get-text-property
+                              0 :completion-cycle-penalty c2)))
+                     (cond ((and s1 s2) (cond ((< s1 s2) t)
+                                              ((> s1 s2) nil)
+                                              (t (< (length c1) (length c2)))))
+                           (s1 t)
+                           (s2 nil))))))
           ;; Prefer recently used completions.
           (let ((hist (symbol-value minibuffer-history-variable)))
             (setq all (sort all (lambda (c1 c2)


[-- Attachment #3: Type: text/plain, Size: 309 bytes --]

------------------------------------------------------------------------------
What You Don't Know About Data Connectivity CAN Hurt You
This paper provides an overview of data connectivity, details
its effect on application quality, and explores various alternative
solutions. http://p.sf.net/sfu/progress-d2d

[-- Attachment #4: Type: text/plain, Size: 179 bytes --]

_______________________________________________
bbdb-info@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bbdb-info
BBDB Home Page: http://bbdb.sourceforge.net/

Re: (patch) sort cycling completion candidates

[Antoine Levitt]

08/03/11 16:10, Ted Zlatanov
> Attached please find a patch to sort cycling completion candidates by
> the text property :completion-cycle-penalty (lower is better).
>
> Thanks
> Ted
>
>
> === modified file 'lisp/minibuffer.el'
> --- lisp/minibuffer.el	2011-02-12 18:30:13 +0000
> +++ lisp/minibuffer.el	2011-03-08 14:51:07 +0000
> @@ -704,7 +704,19 @@
>          (when last
>            (setcdr last nil)
>            ;; Prefer shorter completions.
> -          (setq all (sort all (lambda (c1 c2) (< (length c1) (length c2)))))
> +          (setq
> +           all
> +           (sort all
> +                 (lambda (c1 c2)
> +                   (let ((s1 (get-text-property
> +                              0 :completion-cycle-penalty c1))
> +                         (s2 (get-text-property
> +                              0 :completion-cycle-penalty c2)))
> +                     (cond ((and s1 s2) (cond ((< s1 s2) t)
> +                                              ((> s1 s2) nil)
> +                                              (t (< (length c1) (length c2)))))
> +                           (s1 t)
> +                           (s2 nil))))))
>            ;; Prefer recently used completions.
>            (let ((hist (symbol-value minibuffer-history-variable)))
>              (setq all (sort all (lambda (c1 c2)


What about the case when s1 and s2 are nil? Shouldn't there be another
(t (< (length c1) (length c2))) at the end of the second cond ? Am I
missing something?

It seems cleaner to sort two times, instead of once with both
predicates, the way it's already done in the code for recently used
completions.

Also, the comment needs changing.

What about this?

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 3c8628c..f4af7f3 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -710,6 +710,13 @@ scroll the window of possible completions."
             (setq all (sort all (lambda (c1 c2)
                                   (> (length (member c1 hist))
                                      (length (member c2 hist)))))))
+	  ;; Use the completion-cycle-penalty property
+	  (setq all (sort all (lambda (c1 c2)
+				(let ((s1 (get-text-property
+					   0 :completion-cycle-penalty c1))
+				      (s2 (get-text-property
+					   0 :completion-cycle-penalty c2)))
+				  (and s1 (or (not s2) (< s1 s2)))))))
           ;; Cache the result.  This is not just for speed, but also so that
           ;; repeated calls to minibuffer-force-complete can cycle through
           ;; all possibilities.


------------------------------------------------------------------------------
What You Don't Know About Data Connectivity CAN Hurt You
This paper provides an overview of data connectivity, details
its effect on application quality, and explores various alternative
solutions. http://p.sf.net/sfu/progress-d2d
_______________________________________________
bbdb-info@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bbdb-info
BBDB Home Page: http://bbdb.sourceforge.net/

Re: (patch) sort cycling completion candidates

[Ted Zlatanov]

On Tue, 08 Mar 2011 16:32:00 +0100 Antoine Levitt <antoine.levitt@gmail.com> wrote: 

AL> What about the case when s1 and s2 are nil? Shouldn't there be another
AL> (t (< (length c1) (length c2))) at the end of the second cond ? Am I
AL> missing something?

Oops.  I had it in my original patch but Stefan suggested a change
before that line and I accidentally deleted it.  Sorry!

AL> It seems cleaner to sort two times, instead of once with both
AL> predicates, the way it's already done in the code for recently used
AL> completions.

That seems OK to me.

Your patch is cleaner so I'm in favor of using it.

Ted


------------------------------------------------------------------------------
What You Don't Know About Data Connectivity CAN Hurt You
This paper provides an overview of data connectivity, details
its effect on application quality, and explores various alternative
solutions. http://p.sf.net/sfu/progress-d2d
_______________________________________________
bbdb-info@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bbdb-info
BBDB Home Page: http://bbdb.sourceforge.net/

Re: (patch) sort cycling completion candidates

[Ted Zlatanov]

On Tue, 08 Mar 2011 16:32:00 +0100 Antoine Levitt <antoine.levitt@gmail.com> wrote: 

AL> 08/03/11 16:10, Ted Zlatanov
>> Attached please find a patch to sort cycling completion candidates by
>> the text property :completion-cycle-penalty (lower is better).
>> 
>> Thanks
>> Ted
>> 
>> 
>> === modified file 'lisp/minibuffer.el'
>> --- lisp/minibuffer.el	2011-02-12 18:30:13 +0000
>> +++ lisp/minibuffer.el	2011-03-08 14:51:07 +0000
>> @@ -704,7 +704,19 @@
>> (when last
>> (setcdr last nil)
>> ;; Prefer shorter completions.
>> -          (setq all (sort all (lambda (c1 c2) (< (length c1) (length c2)))))
>> +          (setq
>> +           all
>> +           (sort all
>> +                 (lambda (c1 c2)
>> +                   (let ((s1 (get-text-property
>> +                              0 :completion-cycle-penalty c1))
>> +                         (s2 (get-text-property
>> +                              0 :completion-cycle-penalty c2)))
>> +                     (cond ((and s1 s2) (cond ((< s1 s2) t)
>> +                                              ((> s1 s2) nil)
>> +                                              (t (< (length c1) (length c2)))))
>> +                           (s1 t)
>> +                           (s2 nil))))))
>> ;; Prefer recently used completions.
>> (let ((hist (symbol-value minibuffer-history-variable)))
>> (setq all (sort all (lambda (c1 c2)


AL> What about the case when s1 and s2 are nil? Shouldn't there be another
AL> (t (< (length c1) (length c2))) at the end of the second cond ? Am I
AL> missing something?

AL> It seems cleaner to sort two times, instead of once with both
AL> predicates, the way it's already done in the code for recently used
AL> completions.

AL> Also, the comment needs changing.

AL> What about this?

AL> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
AL> index 3c8628c..f4af7f3 100644
AL> --- a/lisp/minibuffer.el
AL> +++ b/lisp/minibuffer.el
AL> @@ -710,6 +710,13 @@ scroll the window of possible completions."
AL>              (setq all (sort all (lambda (c1 c2)
AL>                                    (> (length (member c1 hist))
AL>                                       (length (member c2 hist)))))))
AL> +	  ;; Use the completion-cycle-penalty property
AL> +	  (setq all (sort all (lambda (c1 c2)
AL> +				(let ((s1 (get-text-property
AL> +					   0 :completion-cycle-penalty c1))
AL> +				      (s2 (get-text-property
AL> +					   0 :completion-cycle-penalty c2)))
AL> +				  (and s1 (or (not s2) (< s1 s2)))))))
AL>            ;; Cache the result.  This is not just for speed, but also so that
AL>            ;; repeated calls to minibuffer-force-complete can cycle through
AL>            ;; all possibilities.


AL> ------------------------------------------------------------------------------
AL> What You Don't Know About Data Connectivity CAN Hurt You
AL> This paper provides an overview of data connectivity, details
AL> its effect on application quality, and explores various alternative
AL> solutions. http://p.sf.net/sfu/progress-d2d
AL> _______________________________________________
AL> bbdb-info@lists.sourceforge.net
AL> https://lists.sourceforge.net/lists/listinfo/bbdb-info
AL> BBDB Home Page: http://bbdb.sourceforge.net/

For some reason the original message didn't come through to
emacs-devel.  Resending.

I am in favor of Antoine's patch: it fixes a bug and is cleaner, but I'm
citing both for completeness.

Thanks
Ted

Re: (patch) sort cycling completion candidates

[Stefan Monnier]

> I am in favor of Antoine's patch: it fixes a bug and is cleaner, but I'm
> citing both for completeness.

Taking a closer look at those patches, I now wonder: should it take
precedence over the "recently used" preference, as Antoine's does,
or not?  I'd tend to think it should not, as in the patch below.


        Stefan


=== modified file 'lisp/minibuffer.el'
--- lisp/minibuffer.el	2011-02-12 18:30:13 +0000
+++ lisp/minibuffer.el	2011-03-15 02:58:25 +0000
@@ -704,7 +704,15 @@
         (when last
           (setcdr last nil)
           ;; Prefer shorter completions.
-          (setq all (sort all (lambda (c1 c2) (< (length c1) (length c2)))))
+          (setq all (sort all (lambda (c1 c2)
+                                (let ((s1 (get-text-property
+                                           0 :completion-cycle-penalty c1))
+                                      (s2 (get-text-property
+                                           0 :completion-cycle-penalty c2)))
+                                  (if (eq s1 s2)
+                                      (< (length c1) (length c2))
+                                    (< (or s1 (length c1))
+                                       (or s2 (length c2))))))))
           ;; Prefer recently used completions.
           (let ((hist (symbol-value minibuffer-history-variable)))
             (setq all (sort all (lambda (c1 c2)

Re: (patch) sort cycling completion candidates

[Ted Zlatanov]

On Mon, 14 Mar 2011 22:59:15 -0400 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>> I am in favor of Antoine's patch: it fixes a bug and is cleaner, but I'm
>> citing both for completeness.

SM> Taking a closer look at those patches, I now wonder: should it take
SM> precedence over the "recently used" preference, as Antoine's does,
SM> or not?  I'd tend to think it should not, as in the patch below.

I think either order is OK so whatever you think is right...

Ted

Re: (patch) sort cycling completion candidates

[Stefan Monnier]

>>> I am in favor of Antoine's patch: it fixes a bug and is cleaner, but I'm
>>> citing both for completeness.

SM> Taking a closer look at those patches, I now wonder: should it take
SM> precedence over the "recently used" preference, as Antoine's does,
SM> or not?  I'd tend to think it should not, as in the patch below.

> I think either order is OK so whatever you think is right...

I've installed something,


        Stefan

Re: (patch) sort cycling completion candidates

[Ted Zlatanov]

On Sat, 19 Mar 2011 22:40:24 -0400 Stefan Monnier <monnier@iro.umontreal.ca> wrote: 

>>>> I am in favor of Antoine's patch: it fixes a bug and is cleaner, but I'm
>>>> citing both for completeness.

SM> Taking a closer look at those patches, I now wonder: should it take
SM> precedence over the "recently used" preference, as Antoine's does,
SM> or not?  I'd tend to think it should not, as in the patch below.

>> I think either order is OK so whatever you think is right...

SM> I've installed something,

I see it came with r103674 but the notes are just in the ChangeLog and
in the source code.  Do you want a doc patch or is it too early for
that?  

Also what about the completion candidate sorting (with
:completion-penalty and :completion-candidate-penalty properties to
supplement :completion-cycle-penalty)?  Do you think that's worthwhile?

Thanks
Ted

Re: (patch) sort cycling completion candidates

[Stefan Monnier]

> I see it came with r103674 but the notes are just in the ChangeLog and
> in the source code.  Do you want a doc patch or is it too early
> for that?

I think it's too early.

> Also what about the completion candidate sorting (with
> :completion-penalty and :completion-candidate-penalty properties to
> supplement :completion-cycle-penalty)?  Do you think that's worthwhile?

Not for now.


        Stefan