Ivy integration (can make pr)

[oantolin]

Ivy integration (can make pr)

[oantolin]

I already added functions for selectrum integration, so this seems reasonable too. Please make a PR. I'll add a section on the README too on this.

[oantolin]

For the re-builder, I think this works:

(defun orderless--ivy-re-builder (str)
  (or
   (mapcar (lambda (x) (cons x t))
           (orderless--component-regexps str))
   ""))

(Why on earth doesn't an empty list mean "everything matches"? Weird API)

The highlighting is apparently supposed to be done my mutating the string. I guess we can separate the highlighting from the copying...

[oantolin]

I'm so sorry, I didn't mean to reject your offer to help, but I just got stuck on thinking about the ivy integration and went ahead and did it (2750e15). If you like your version better, let's talk about it.

[noctuid]

I did things a little differently. I stored the regexps in the match function and then had this for the highlight function:

(defvar orderless--ivy-regexps nil
  "Store generated regexps to use for highlighting later.")

(defun orderless-ivy-highlight-function (string)
  "Highlight STRING.
This function is automatically added to `ivy-highlight-functions-alist'."
  (car (orderless-highlight-matches orderless--regexps (list string))))

Your version is better I think. The only thing I would maybe add is that (if you're okay with the eval-after-load), you can prevent the need for the user to do the highlighting configuration (update this for the actual function names you used):

(eval-after-load 'ivy
  (lambda ()
    (cl-pushnew (cons #'orderless-ivy-re-builder
                      #'orderless-ivy-highlight-function)
                ivy-highlight-functions-alist
                :test 'equal)))

:test needs to be 'equal; with the default, you'll still get duplicates.

I'd also add an autoload for the re-builder, so it can automatically load the package when ivy calls it.

[oantolin]

I don't want an extra global variable, I'll stick with ivy-regex (which is what the highlighters that come with ivy use anyway).

I think when I was testing I forgot car in the highlighting function. :D And I thought it was because the highlighting needed to be done in place, so I refactored. :D I think I'll keep the separate mutating single-string highlighter, because it seems to work with ivy, which means ivy is making a copy anyway and it would be wasteful to make another.

The eval-after-load is a good idea, I'll autoload it too, and update the README.

This is done in 46e4ecd, please check I didn't screw it up.

[noctuid]

I don't want an extra global variable, I'll stick with ivy-regex (which is what the highlighters that come with ivy use anyway).

Yes I agree this method is better.

The eval-after-load is a good idea, I'll autoload it too, and update the README.

Having an autoload for this was unnecessary in my testing since orderless is already loaded by the time highlighting happens.

Sorry I didn't actually edit the names in my code before; it's setting the re builder function to the highlight function which needs to be fixed. My bad.

Edit: Nevermind, I misread it. Will report back if something isn't working.

[noctuid]

If you're going to use eval-after-load to avoid the 24.4 dependency, the cl-pushnew should be inside a lambda. Right now things are broken for me. I'd remove the autoload for highlighting as well.

[oantolin]

If you're going to use eval-after-load to avoid the 24.4 dependency the cl-pushnew should be inside a lambda. Right now things are broken for me.

No, sorry! I just mixed up eval-after-load and with-eval-after-load, depending on 24.4 is fine by me. I've fixed that now.

I'd remove the autoload for highlighting as well.

orderless-ivy-highlight has no autoload. Do you mean the one on orderless-highlight-matches? I think that's safe to remove, so I will.

[oantolin]

Let me know if the ivy integration is working now, so we can close this. (I closed it accidentally earlier.)

[noctuid]

Yes, everything is working correctly. Thanks!

[oantolin]

Good, thanks! (I feel awful about breaking the package for a while because I wrote eval-after-load instead of with-eval-after-load.)