Preserve case for dabbrev and ispell

minad commented 1 year ago

Originally posted by @ddoherty03 at https://github.com/minad/corfu/issues/301

For text-mode completions and the like, when one is writing the selection of case is invariabaly intential. For example, if I am starting a sentence and I type

Accordi^

Completions from things like ispell, dabbrev, and the like are all offered as lower case. So if I select the completion, my buffer looks like this:

accordingly^

That means I have to back up, correct the case, then move past the word to continue typing. This makes the completion largely a losing proposition: it takes more key strokes to correct the completion than it would have taken just to type the whole word.

I would really like to see corfu allow an option to mimic the case of the partial input in the completion candidates, so that it would only offer things like "Accordingly, Accordion, etc" in the above scenario. For consistency, if I type "ACCORD", it would upcase all the candidates.

This is how query-replace works and is a very Emacs-y way to do things, and a real time saver.

Not a bug, just a wish. Thanks for all your fine emacs work, by the way.

ddoherty03 commented 1 year ago

I was wondering if this was the right place for it. I'll watch for it. Thanks.

hubisan commented 1 year ago

This would be a great feature especially when using cape-dabbrev, cape-ispell or cape-dict.

I have a list of english and german words sorted by frequency to use with cape-dict. As they are all lower case I use cape-capf-case-fold to also get the candidates for upper case. But unfortunately after completion the word is downcased.

With company it is possible to keep the prefix as it is with for instance company-dabbrev and (setq company-dabbrev-ignore-case 'keep-prefix company-dabbrev-downcase nil). But seems to be backend specific.

minad commented 1 year ago

@hubisan Yes, the problem is only that I don't know how to implement this feature properly. Usually candidate transformations after insertion are performed by the :exit-function. Unfortunately the :exit-function doesn't have access to the input. As an alternative we may be able to implement this on the level of the completion style, such that all candidates are immediately transformed, but this will be expensive.

hubisan commented 1 year ago

[8.6.2023, update > use this hack instead]

I see the problem.

Hack to implement this feature

Update: Use the hack from later comment instead.

This hack does the job for me by overriding corfu--insert:

  (defun my-corfu--insert (status)
    "Insert current candidate, exit with STATUS if non-nil.
The case of the inserted candidate is preserved according to the prefix that
triggered the completion if the property :corfu-preserve-case is set to t."
    (pcase-let* ((`(,beg ,end . ,_) completion-in-region--data)
                 (str (buffer-substring-no-properties beg end)))
      (setq str (concat corfu--base (substring-no-properties
                                     (nth corfu--index corfu--candidates))))
      (if (plist-get corfu--extra :corfu-preserve-case)
          (completion--replace beg end (my-corfu--candidate-with-preserved-case
                                        str (buffer-substring-no-properties beg end)))
        (completion--replace beg end (concat str)))
      (corfu--goto -1)
      (when status (corfu--done str status))))

  (defun my-corfu--candidate-with-preserved-case (candidate prefix)
    "Return the CANDIDATE with preserved case depending on the PREFIX.
If the PREFIX is all caps and longer than one character return the upcased
CANDIDATE. If the PREFIX matches the beginning of the CANDIDATE return the
PREFIX and the remaining characters of the CANDIDATE. Else preserve the case of
the first character."
    (let* ((prefix-all-caps (let ((case-fold-search nil))
                              (string-match-p "^[[:upper:]]+$" prefix)))
           (prefix-matches-candidate-beg
            (string-match-p (concat "^" prefix ".*$") candidate)))
      (cond
       ((and prefix-all-caps (length> prefix 1))
        (upcase candidate))
       (prefix-matches-candidate-beg
        (concat prefix (substring-no-properties candidate (length prefix))))
       (t
        (if (string-match-p "[[:upper:]]" (substring-no-properties prefix  0 1))
            (concat (upcase (substring-no-properties candidate 0 1))
                    (substring-no-properties candidate ))
          (concat (downcase (substring-no-properties candidate 0 1))
                    (substring-no-properties candidate)))))))

  (advice-add 'corfu--insert :override #'my-corfu--insert)

Usage

Then I can use this with for instance cape-dabbrev.

Add the property :corfu-preserve-case:

(setq cape--dabbrev-properties
        (list :annotation-function (lambda (_) " Dabbrev")
              :company-kind (lambda (_) 'text)
              :exclusive 'no
              ;; Make it preserve case.
              :corfu-preserve-case t))

Set capf in buffer:

(setq-local completion-at-point-functions (list (cape-capf-case-fold #'cape-dabbrev)))

Feels much better now with preserving the case.

minad commented 1 year ago

Yes, this is a possibility. But I am not really interested in introducing metadata extensions like :corfu-preserve-case. Maybe there is no alternative, but in any case, I have to think more about this.

minad commented 1 year ago

I consider this wontfix for now due to the lack of a good and simple solution.

ddoherty03 commented 1 year ago

Darn. You mentioned above that "Unfortunately the :exit-function doesn't have access to the input." Is that something that could be fixed at the emacs level?

minad commented 1 year ago

Darn. You mentioned above that "Unfortunately the :exit-function doesn't have access to the input." Is that something that could be fixed at the emacs level?

Yes, or with an extension as proposed by @hubisan.

ddoherty03 commented 1 year ago

@hubisan, despite being a bit intimidated by your fix, I sucked it up and am trying to integrate it into my init file. Thanks for this fancy bit of coding. I was able to get dabbrev working, as you did, but what I want is a completion function that combines dabbrev, ispell, and dict when I am working on text documents and their derivatives, like AucTeX.

I tried this, but it doesn't seem to work:

  (setq cape--dabbrev-properties
        (list :annotation-function (lambda (_) " Dabbrev")
              :company-kind (lambda (_) 'text)
              :exclusive 'no
              ;; Make it preserve case.
              :corfu-preserve-case t))

  (setq cape--dict-properties
        (list :annotation-function (lambda (_) " Dict")
              :company-kind (lambda (_) 'text)
              :exclusive 'no
              ;; Make it preserve case.
              :corfu-preserve-case t))

  (setq cape--ispell-properties
        (list :annotation-function (lambda (_) " Ispell")
              :company-kind (lambda (_) 'text)
              :exclusive 'no
              ;; Make it preserve case.
              :corfu-preserve-case t))

    (defalias 'cape-english-words
      (cape-super-capf (cape-capf-case-fold #'cape-dabbrev)
                       (cape-capf-case-fold #'cape-dict)
                       (cape-capf-case-fold #'cape-ispell)))

And then, in my text-mode-hook:

    (setq-local completion-at-point-functions
                '(cape-file
                  cape-english-words))

But no joy. Any ideas on how to deploy your hack for this scenario?

hubisan commented 1 year ago

@ddoherty03 You can copy the cape-super-capf function and name it for instance my-cape-super-capf and change the last line in the copied function from

:annotation-function (funcall extra-fun :annotation-function)
:exit-function (funcall extra-fun :exit-function))))))

to

:annotation-function (funcall extra-fun :annotation-function)
:exit-function (funcall extra-fun :exit-function)
:corfu-preserve-case (funcall extra-fun :corfu-preserve-case))))))

Then use my-cape-super-capf instead of cape-super-capf.

minad commented 1 year ago

I implemented case replacement in https://github.com/minad/cape/commit/15e0e61addd320972265b03ac6e692edd9468a0b. Please give it a test!

hubisan commented 1 year ago

Thanks for the changes.

Here my findings after a simple tests with cape-dabbrev.

If I enter the word testing I get the completions as expected for testing, Testing and TESTING.
If I enter the word Testing I get the completions for Testing and TESTING but not testing, this is probably to be expected. But if I use (cape-capf-case-fold #'cape-dabbrev) the completion for testing is also not working. I get Testing as completion with input test.
After entering just T I get the completion TESTING. If the input is just one character it probably should give Testing as completion instead of TESTING. And only consider all caps once the input is longer than one character.

Tested with an empty org-mode file with Emacs 29, completion-at-point-functions set to (cape-dabbrev), minimal packages loaded, (setq cape-dabbrev-check-other-buffers nil).

minad commented 1 year ago

2) This specific filtering may depend on your completion style, e.g., the setting orderless-smart-case. 3) I can try to push a fix for this. But you use overly aggressive completion settings if you want to complete single characters. One possibility would be to not generate candidates at all in this case.

minad commented 1 year ago

@hubisan Pushed a fix. Please try again.

hubisan commented 1 year ago

Thank you very much for taking the time to improve this.

Yeah, 3. is an edge case and I only tested it to see what completions it provides, not using it.

Still behaves the same. To reproduce (barebone Emacs 29 with just straight and leaf):

Cape commit adb80db319d3b2b7075ab18e4431167f6055f763

(progn 
  (leaf corfu :straight t)
  (leaf cape :straight t)
  (require 'corfu)
  (require 'cape)
  (setq completion-styles '(basic)
        dabbrev-case-fold-search t
        dabbrev-case-replace t)
  (let ((buf (generate-new-buffer "*cape-testing*")))
    (switch-to-buffer buf)
    (text-mode)
    (corfu-mode)
    (setq-local completion-at-point-functions (list #'cape-dabbrev))
    (insert "Testing")))

Completions: Te > Testing TE > TESTING te > no completion

Does dabbrev-case-fold-search not mean that case is ignored when collecting candidates? So for te there should also be a completion?

If I change to (setq-local completion-at-point-functions (list (cape-capf-case-fold #'cape-dabbrev))) this happens:

Te > Testing TE > TESTING te > Testing

minad commented 1 year ago

I don't know why it doesn't work. Please try to debug this in more detail. You can check if the completions are found and filtered out later on or if they aren't generated in the first place.

minad commented 1 year ago

You probably forget to set completion-ignore-case=t?

minad commented 1 year ago

Please try https://github.com/minad/cape/commit/41e05124edcdb2d97f9baf4005707121a37498e3. Hopefully this fixes all the issues. Unfortunately the interaction of completion and dabbrev generation is a bit of a mess.

hubisan commented 1 year ago

Still turns te into Testing.

Will try to debug this. I'm currently experiencing limitations due to an injury so it will take a while.

minad commented 1 year ago

Still turns te into Testing.

This is expected if Testing appears first in your file. Dabbrev doesn't distinguish candidates with different casing. Dabbrev is kind of crap tbh. If you want to check if the casing logic of Cape works properly you could try cape-dict with a test dictionary file.

Will try to debug this. I'm currently experiencing limitations due to an injury so it will take a while.

Thanks for your help. I hope you get better soon. Take care!

hubisan commented 1 year ago

[Edit 2023-08-07 Adapted to recent changes]

Downcasing the expansions returned from dabbrev--find-all-expansions seems to be the only way.

That's what I am using now, works well:

  (defun my-cape--dabbrev-fix-expansion (expansion)
    "Return the downcased EXPANSION.
 Removes trailing non-alphanumeric characters if present."
    (let ((downcased (downcase expansion)))
      (substring downcased 0 (string-match-p "[^[:alnum:]]+$" downcased))))

  (defun my-cape--dabbrev-list (input)
    "Find all dabbrev expansions for INPUT. "
    (cape--silent
      ;; Don't search all buffers. Only those with the same major-mode.
      (let ((dabbrev-check-other-buffers t)
            (dabbrev-check-all-buffers nil))
        (dabbrev--reset-global-variables))
      (cons
       (apply-partially #'string-prefix-p input)
       (cl-loop for w in (mapcar #'my-cape--dabbrev-fix-expansion
                                 (dabbrev--find-all-expansions input t))
                if (>= (length w) cape-dabbrev-min-length) collect
                (cape--case-replace t input w)))))

  (advice-add 'cape--dabbrev-list :override #'my-cape--dabbrev-list)

ddoherty03 commented 1 year ago

@minad and @hubisan, thanks for working away on this. I have not been paying attention, and just caught up on the more recent comments. I am not really adept at elisp, and am a bit lost at this point on a few points:

Is the fix available in the MELPA version, or do I need to pull in something other than the master branch to try this out?
As an end-user, how do I actually use this at this point?
Will the README be updated with instructions? I think a lot of people would really like this feature.
Is @hubisan fix to super-capf still relevant and needed?

Many thanks.

hubisan commented 1 year ago

@ddoherty03 my comments:

The fix is in the main branch and therefore available in the MELPA version.
Should just work but it still depends on how the candidates are collected.
There is no need I think as it's not a feature you can enable or disable.
The hack is all you need to make cape-dabbrev work as expected. It downcases the expansions returned by dabbrev--find-all-expansions and removes trailing non-alphanumeric characters if present.

ddoherty03 commented 1 year ago

@hubisan, the hack seems to work for dabbrev, but how about cape-dict and cape-ispell? Do you have hacks for those?

hubisan commented 1 year ago

Not using cape-dict. But after trying out it seems to work as expected out of the box:

(setq-local completion-at-point-functions (list #'cape-dict))
(corfu-mode 1)

completions for hou > house, hour, hound ...
completions for Hou > House, Hour, Houma, Hound ...
completions for HOU > HOUSE, HOUR, HOUMA, HOUND ... (Houma is only in capital letters in the file it is using).

Check your customization:

cape-dict-file points to "/usr/share/dict/words" which is used for the completions. In my case this file has most words in both uppercase as lowercase and some in only lowercase. This seems to work for all of the above cases.
cape-dict-case-replace 'case-replace which is t as case-replace defaults to t
cape-dict-case-fold 'case-fold-search which is t as case-fold-search defaults to t

cape-ispell is deprecated. From the changelog:

cape-ispell: Deprecate in favor of improved cape-dict. Note that cape-ispell and ispell-lookup-words did not really consult ispell or aspell, but only grep through the word list specified by ispell-alternate-dictionary.

ddoherty03 commented 1 year ago

@hubisan thanks. I didn't realize I didn't have the most recent version of cape or that cape-ispell was deprecated. Fixing those solved my problem. I appreciate your help.

minad / cape

Preserve case for dabbrev and ispell #77

Hack to implement this feature

Usage