abo-abo / avy

Jump to things in Emacs tree-style
1.72k stars 110 forks source link

Feature request: a public interface similar to avy--generic-jump #265

Closed yangsheng6810 closed 5 years ago

yangsheng6810 commented 5 years ago

avy--generic-jump is supposed for internal use, but other packages (e.g. ace-pinyin which builds regex when you type in letters, and is able to also jump to chinese characters with matching pinyin) depend on it. The API of avy--generic-jump went through an incompatible change in commit 16482e00. While ace-pinyin has updated in reaction to the change of API for avy--generic-jump, a public interface with stable API would be more desirable to avoid problem of future API change.

abo-abo commented 5 years ago

Thanks.

yangsheng6810 commented 5 years ago

Thank you for your effort!

abo-abo commented 5 years ago

You're welcome. But I don't think the code in your PR is truly future-proof.

The new avy-jump API gives the following guarantees:

It does not give any guarantee about the order of the keyword args, so calling (avy-jump regex nil) assuming that nil will be :window-flip is dangerous. But if you use (avy-jump regex :window-flip nil), or simply (avy-jump regex) (since keyword args default to nil), it's future-proof even if new keyword arguments are added to avy-jump, or their order changes.

yangsheng6810 commented 5 years ago

Thank you for your explanation. It is really helpful for a new emacs-lisp programmer like me. What is a proper way to add backward compatibility with older version of avy? Is adding a version of avy-jump like the following OK?

(when (not (boundp 'avy-jump))
  (cl-defun avy-jump (regex &key window-flip beg end action)
    "Jump to REGEX.
The window scope is determined by `avy-all-windows'.
When WINDOW-FLIP is non-nil, do the opposite of `avy-all-windows'.
BEG and END narrow the scope where candidates are searched.
ACTION is a function that takes point position as an argument."
    (setq avy-action (or action avy-action))
    (let ((avy-all-windows
           (if window-flip
               (not avy-all-windows)
             avy-all-windows)))
      (avy--process
       (avy--regex-candidates regex beg end)))))
abo-abo commented 5 years ago

Yes, this method is pefectly OK. You could maybe also re-use avy--generic-jump to make the code shorter.