minad / cape

🦸cape.el - Completion At Point Extensions
GNU General Public License v3.0
603 stars 22 forks source link

Possible issue in cape--case-replace #116

Closed luavreis closed 6 months ago

luavreis commented 6 months ago

I'm having problems with cape-dabbrev not downcasing the expansions when the input is lowercase, while dabbrev-expand replaces the lowercase abbreviation with downcased text as expected.

By "expected", I mean that if dabbrev-case-replace is non-nil (or equal to 'case-replace and case-replace is non-nil) then the reference text (expansion) case should be ignored and replaced by the input case. Currently, Cape correctly handles the case when the reference is lowercase and the input begins with an uppercase letter, but fails in the opposite situation.

I've narrowed down the issue to the cape--case-replace function:

https://github.com/minad/cape/blob/30133e41ccc5a4bb72998b19c26a664e3c3bfc65/cape.el#L155-L167

First (but possibly unrelated), I'm not sure if the (string-match-p "\\`[[:upper:]]" input) check is working as intended here, because if case-fold-search is t (as it by default is) then it would also match downcase letters.

Also, replace-match does not ignore uppercase letters in str, in the sense that

(let ((input "pro") (str "Product")) 
  (string-match input input) 
  (replace-match str nil nil input))

returns "Product". I think to get a behaviour closer to dabbrev it would make sense to remove the check and downcase str:

(defun cape--case-replace (flag input str)
  "Replace case of STR depending on INPUT and FLAG."
  (or (and (if (eq flag 'case-replace) case-replace flag)
           (string-prefix-p input str t)
           (save-match-data
             ;; Ensure that single character uppercase input does not lead to an
             ;; all uppercase result.
             (when (and (= (length input) 1) (> (length str) 1))
               (setq input (concat input (substring str 1 2))))
             (and (string-match input input)
                  (replace-match (downcase str) nil nil input))))
      str))
minad commented 6 months ago

First (but possibly unrelated), I'm not sure if the (string-match-p "\`[[:upper:]]" input) check is working as intended here, because if case-fold-search is t (as it by default is) then it would also match downcase letters.

I fixed this.

Otherwise Cape works as expected. Uppercase input turns lowercase candidates to uppercase. The other direction is not intended - uppercase candidates should stay uppercase. cape--case-replace is also used by other backends like cape-dict, which only produce uppercase candidates when the only correct form of the word is uppercase.

luavreis commented 6 months ago

The other direction is not intended - uppercase candidates should stay uppercase.

For the cape-dict capf I agree this makes sense, but for cape-dabbrev shouldn't it work like dabbrev-expand when possible? The other direction is useful because sometimes the candidate word for the input being typed is not a proper noun but appears in the beginning of a sentence, so it appears uppercased in the candidates list.

Edit: another thing that makes this worse is that if the first occurrence of the candidate in the buffer is capitalized, then even if the candidate also appears in lowercase in the buffer later, only capitalized suggestions are shown by cape-dabbrev.

Edit 2: a suggestion:

(defun cape--dabbrev-list (input)
  "Find all Dabbrev expansions for INPUT."
  (cape--silent
    (let ((dabbrev-check-other-buffers (not (null cape-dabbrev-check-other-buffers)))
          (dabbrev-check-all-buffers (eq cape-dabbrev-check-other-buffers t)))
      (dabbrev--reset-global-variables))
    (cons
     (apply-partially #'string-prefix-p input)
     (cl-loop with min-len = (+ cape-dabbrev-min-length (length input))
               with ic = (cape--case-fold-p dabbrev-case-fold-search)
               for w in (dabbrev--find-all-expansions input ic)
               if (>= (length w) min-len) collect
-              (cape--case-replace (and ic dabbrev-case-replace) input w)))))
+              (let ((dw (if (let (case-fold-search) (not (string-match-p "[[:lower:]]" w)))
+                            w (downcase w))))
+                (cape--case-replace (and ic dabbrev-case-replace) input dw))))))
minad commented 6 months ago

The other direction is useful because sometimes the candidate word for the input being typed is not a proper noun but appears in the beginning of a sentence, so it appears uppercased in the candidates list.

Edit: another thing that makes this worse is that if the first occurrence of the candidate in the buffer is capitalized, then even if the candidate also appears in lowercase in the buffer later, only capitalized suggestions are shown by cape-dabbrev.

Yes, of course. This are known problems. Note that the casing rules are also language dependent. Dabbrev is not a particularly good backend and I don't intend to work around all its issues in Cape. I think there is a need for a better and much more performant replacement for Dabbrev.