bug#13322: 24.3.50; `completion-all-sorted-completions': sorting and duplicate deletion

bug#13322: 24.3.50; `completion-all-sorted-completions': sorting and duplicate deletion

[Drew Adams]

1. Please provide for `completion-all-sorted-completions' to sort using
a function other than what is returned by `cycle-sort-function' from the
metadata.  E.g., an optional arg SORT-FUN.
 
2. Please make duplicate deletion optional.  E.g., an optional arg
DONT-REMOVE-DUPS.
 
Suggested code below - it just adds the args and respects them.
But please also add a doc string in any case.
 
(defun completion-all-sorted-completions (&optional sort-fun dont-remove-dups)
  "FIXME - I NEED A HELPFUL DOC STRING"
  (or completion-all-sorted-completions
      (let* ((start (field-beginning))
             (end (field-end))
             (string (buffer-substring start end))
             (md (completion--field-metadata start))
             (all (completion-all-completions
                   string
                   minibuffer-completion-table
                   minibuffer-completion-predicate
                   (- (point) start)
                   md))
             (last (last all))
             (base-size (or (cdr last) 0))
             (all-md (completion--metadata (buffer-substring-no-properties
                                            start (point))
                                           base-size md
                                           minibuffer-completion-table
                                           minibuffer-completion-predicate)))
        (unless sort-fun
          (setq sort-fun (completion-metadata-get all-md 'cycle-sort-function)))
        (when last
          (setcdr last nil)
 
          ;; Delete duplicates: do it after setting last's cdr to nil (so
          ;; it's a proper list), and be careful to reset `last' since it
          ;; may be a different cons-cell.
          (unless dont-remove-dups (setq all (delete-dups all)))
          (setq last (last all))
 
          (setq all (if sort-fun (funcall sort-fun all)
                      ;; Prefer shorter completions, by default.
                      (sort all (lambda (c1 c2) (< (length c1) (length c2))))))
          ;; Prefer recently used completions.
          (when (minibufferp)
            (let ((hist (symbol-value minibuffer-history-variable)))
              (setq all (sort all (lambda (c1 c2)
                                    (> (length (member c1 hist))
                                       (length (member c2 hist))))))))
          ;; Cache the result.  This is not just for speed, but also so that
          ;; repeated calls to minibuffer-force-complete can cycle through
          ;; all possibilities.
          (completion--cache-all-sorted-completions (nconc all base-size))))))
 

`completion-all-sorted-completions' currently does a mix of things.
Perhaps those things should be separated into different functions that
users could use on their own.  Is there a good reason to couple these
things?
 
a. calculation of the completions
b. duplicate deletion
c. sorting
d. caching
 
Dunnow whether caching should be separated out, but perhaps b and c
could be.

In GNU Emacs 24.3.50.1 (i386-mingw-nt5.1.2600)
 of 2012-12-18 on MS-W7-DANI
Bzr revision: 111265 eliz@gnu.org-20121218190556-x9wmq083vwecgu0f
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --with-gcc (4.7) --no-opt --enable-checking --cflags
 -Ic:/emacs/libs/libXpm-3.5.10/include -Ic:/emacs/libs/libXpm-3.5.10/src
 -Ic:/emacs/libs/libpng-dev_1.4.3-1_win32/include
 -Ic:/emacs/libs/zlib-dev_1.2.5-2_win32/include
 -Ic:/emacs/libs/giflib-4.1.4-1-lib/include
 -Ic:/emacs/libs/jpeg-6b-4-lib/include
 -Ic:/emacs/libs/tiff-3.8.2-1-lib/include
 -Ic:/emacs/libs/libxml2-2.7.8-w32-bin/include/libxml2
 -Ic:/emacs/libs/gnutls-3.0.9-w32-bin/include
 -Ic:/emacs/libs/libiconv-1.9.2-1-lib/include'

bug#13322: 24.3.50; `completion-all-sorted-completions': sorting and duplicate deletion

[Drew Adams]

Oops.  Forgot to handle the second sorting occurrence also.
If a user supplies a sort function then we should not
overrule that to re-sort by history.

This code should do it (but the code is just a suggestion, again):

(defun completion-all-sorted-completions (&optional sort-function
dont-remove-dups)
  "Like `c-a-s-c', but with added optional args."
  (or completion-all-sorted-completions
      (let* ((start (field-beginning))
             (end (field-end))
             (string (buffer-substring start end))
             (md (completion--field-metadata start))
             (all (completion-all-completions
                   string
                   minibuffer-completion-table
                   minibuffer-completion-predicate
                   (- (point) start)
                   md))
             (last (last all))
             (base-size (or (cdr last) 0))
             (all-md (completion--metadata (buffer-substring-no-properties
                                            start (point))
                                           base-size md
                                           minibuffer-completion-table
                                           minibuffer-completion-predicate))
             (sort-fun (or sort-function
                           (completion-metadata-get all-md
'cycle-sort-function))))
        (when last
          (setcdr last nil)
          ;; Delete duplicates: do it after setting last's cdr to nil (so
          ;; it's a proper list), and be careful to reset `last' since it
          ;; may be a different cons-cell.
          (unless dont-remove-dups (setq all (delete-dups all)))
          (setq last (last all))
          (setq all (if sort-fun (funcall sort-fun all)
                      ;; Prefer shorter completions, by default.
                      (sort all (lambda (c1 c2) (< (length c1) (length c2))))))
          ;; Prefer recently used completions.
          (when (and (minibufferp)  (not sort-fun))
            (let ((hist (symbol-value minibuffer-history-variable)))
              (setq all (sort all (lambda (c1 c2)
                                    (> (length (member c1 hist))
                                       (length (member c2 hist))))))))
          ;; Cache the result.  This is not just for speed, but also so that
          ;; repeated calls to minibuffer-force-complete can cycle through
          ;; all possibilities.
          (completion--cache-all-sorted-completions (nconc all base-size))))))

bug#13322: 24.3.50; `completion-all-sorted-completions': sorting and duplicate deletion

[Stefan Monnier]

> 1. Please provide for `completion-all-sorted-completions' to sort using
> a function other than what is returned by `cycle-sort-function' from the
> metadata.  E.g., an optional arg SORT-FUN.
 
> 2. Please make duplicate deletion optional.  E.g., an optional arg
> DONT-REMOVE-DUPS.

Since the completion-all-sorted-completions function just returns the
value of the completion-all-sorted-completions cache when that cache is
populated, it clearly can't take arguments since they'd not always
be obeyed.


        Stefan

bug#13322: 24.3.50; `completion-all-sorted-completions': sorting and duplicate deletion

[Drew Adams]

> Since the completion-all-sorted-completions function just returns the
> value of the completion-all-sorted-completions cache when 
> that cache is populated, it clearly can't take arguments since they'd
> not always be obeyed.

They would be obeyed whenever the function does not simply return the cached
value.  That's the point.

Code can call `completion--flush-all-sorted-completions' when it wants
`completion-all-sorted-completions' to update the cache.  That function
(`c--f-a-s-c') should not be "internal", BTW, IMHO.

(It might also be helpful for `completion-all-sorted-completions' itself to be
able to decide whether to reuse the cached value - other than testing its mere
presence.  It could do so based on another optional argument or on a(nother)
global variable value.)

FWIW, as an example, in `icomplete+.el' I use this version, which is essentially
what I sent here before, but this also handles `completion-ignored-extensions'
and an alist COLLECTION arg of absolute file names (for Icicles).  (See also bug
#12939, which has gotten no reply, about handling
`completion-ignored-extensions'.)

(defun icompletep-completion-all-sorted-completions
         (&optional sort-function dont-remove-dups)
  "Like `completion-all-sorted-completions', but with added optional args.
If SORT-FUNCTION is nil, sort per `completion-all-sorted-completions':
 * per property `cycle-sort-function', if defined
 * else by shorter length, then by recent use."
  (or completion-all-sorted-completions
      (let* ((start      (field-beginning))
             (end        (field-end))
             (string     (buffer-substring start end))
             (md         (completion--field-metadata start))
             (all        (completion-all-completions
                          string  minibuffer-completion-table
                          minibuffer-completion-predicate
                          (- (point) start)  md))
             (last       (last all))
             (base-size  (or (cdr last)  0))
             (all-md     (completion--metadata
                          (buffer-substring-no-properties start (point))
                          base-size md minibuffer-completion-table
                          minibuffer-completion-predicate))
             (sort-fun   (or sort-function
                             (completion-metadata-get
                               all-md 'cycle-sort-function))))
        (when last
          (setcdr last ())
          ;; Exclude files for `completion-ignored-extensions'.
          (when (or minibuffer-completing-file-name
                    (and (boundp 'icicle-abs-file-candidates)
                         icicle-abs-file-candidates))
            (setq all  (delete-if
                        (lambda (fl)
                          (string-match-p
                            (regexp-opt
                              completion-ignored-extensions)
                            fl))
                        all)))
          (unless dont-remove-dups (setq all  (delete-dups all)))
          (setq last  (last all)
                all   (if sort-fun
                          (funcall sort-fun all)
                        (sort all (lambda (c1 c2)
                                    (< (length c1)
                                       (length c2))))))
          (when (and (minibufferp)  (not sort-fun))
            (let ((hist  (symbol-value minibuffer-history-variable)))
              (setq all  (sort all (lambda (c1 c2)
                                     (> (length (member c1 hist))
                                        (length (member c2 hist))))))))
          (completion--cache-all-sorted-completions
            (nconc all base-size))))))

I call `completion--flush-all-sorted-completions' from Icicles when a user hits
`C-,' in the minibuffer, which cycles to the next Icicles candidate sort order.
This cache flushing makes Icomplete re-order the candidates, so the order in
Icomplete reflects the new current (Icicles) sort order.

bug#13322: 24.3.50; `completion-all-sorted-completions': sorting and duplicate deletion

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> 1. Please provide for `completion-all-sorted-completions' to sort using
>> a function other than what is returned by `cycle-sort-function' from the
>> metadata.  E.g., an optional arg SORT-FUN.
>
>> 2. Please make duplicate deletion optional.  E.g., an optional arg
>> DONT-REMOVE-DUPS.
>
> Since the completion-all-sorted-completions function just returns the
> value of the completion-all-sorted-completions cache when that cache is
> populated, it clearly can't take arguments since they'd not always
> be obeyed.

And as I understand it from recent discussion, it doesn't really matter
whether duplicates are removed or not, since they are removed at a later
stage.

So I'm not seeing anything to be done here.  Closing.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#13322: 24.3.50; `completion-all-sorted-completions': sorting and duplicate deletion

[Drew Adams]

> >> 1. Please provide for `completion-all-sorted-completions' to sort using
> >> a function other than what is returned by `cycle-sort-function' from
> the
> >> metadata.  E.g., an optional arg SORT-FUN.
> >
> >> 2. Please make duplicate deletion optional.  E.g., an optional arg
> >> DONT-REMOVE-DUPS.
> 
> And as I understand it from recent discussion, it doesn't really matter
> whether duplicates are removed or not, since they are removed at a later
> stage.  So I'm not seeing anything to be done here.  Closing.

Nonsense.  The request is for Emacs not to force duplicate removal.
You are replying that it doesn't matter whether it removes duplicates
because it removes duplicates anyway.  Silly.