Re: (patch) sort cycling completion candidates

[Ted Zlatanov]

On Tue, 08 Mar 2011 16:32:00 +0100 Antoine Levitt <address@hidden> 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> address@hidden
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