radian-software / ctrlf

⌨️ Emacs finally learns how to ctrl+F.
MIT License
348 stars 22 forks source link

Other messages in minibuffer #123

Open haji-ali opened 2 years ago

haji-ali commented 2 years ago

When using ctrlf, if a message is displayed while text is being inputted, the point position appears after the message rather than before. See this example for an illustration:

(minibuffer-with-setup-hook
    (lambda ()
      (add-hook 'post-command-hook
                (lambda () (message "Is that it?"))
                nil t))
  (ctrlf-forward-default))

When inputting text, the point is set after "Is that it?" but before the ctrlf indicator. If I use read-string instead of ctrlf-forward-default or when the ctrlf indicator disappears, the point position is correct.

Is there a way to fix this?

raxod502 commented 2 years ago

Ugh, this code is a pain in the neck. Yeah, it's probably possible to fix, it just requires screwing around some more with the order that overlays are applied. Emacs uses the text properties on those, and possibly some undocumented heuristics, to decide which side point is going to be on with regard to overlays (and it might differ between versions, as well). Happy to accept changes.

haji-ali commented 2 years ago

I tried playing around with it to figure out a working arrangement but wasn’t able to find it (it’s all fairly new to me so I might be missing something).

it’s something to do with a correct ‘cursor’ property right? Or are there other properties that can affect this as well?

raxod502 commented 2 years ago

Yes, it's the cursor property. E.g. here: https://github.com/radian-software/ctrlf/blob/cefb0aff9d316bd03e911f7f483f8d01f15cf5a2/ctrlf.el#L477-L479

It also matters what order you create the overlays in.

See documentation here - https://www.gnu.org/software/emacs/manual/html_node/elisp/Special-Properties.html

haji-ali commented 2 years ago

I figured out that the problem is that I am using message instead of minibuffer-message, if the code was

(minibuffer-with-setup-hook
    (lambda ()
      (add-hook 'post-command-hook
                (lambda () (minibuffer-message "Is that it?"))
                nil t))
  (ctrlf-forward-default))

Then everything works. This is because of the advice ctrlf--minibuffer-message-condense that is added to minibuffer-message which condenses all overlays.

I think a similar advice is needed for set-minibuffer-message which is called when message is called. I tried this:

(defun ctrlf--set-minibuffer-message-condense (func &rest args)
  "Apply `ctrlf--condense-overlays' after `set-minibuffer-message'.

This is an `:around' advice for `set-minibuffer-message'. FUNC and
ARGS the original function and its arguments, as usual."
  (if (not ctrlf--minibuffer)
      (apply func args)
    (cl-letf* ((make-overlay (symbol-function #'make-overlay))
               ((symbol-function #'make-overlay)
                (lambda (&rest args)
                  (let ((ol (apply make-overlay args)))
                    (prog1 ol
                      ;; Assume ownership of this overlay so we can mess
                      ;; with it :D
                      (overlay-put ol 'ctrlf t)
                      (overlay-put ol 'ctrlf--transient t)
                      (push ol ctrlf--overlays))))))
      (prog1 (apply func args)
        (ctrlf--condense-overlays)))))

(advice-add #'set-minibuffer-message :around
            #'ctrlf--set-minibuffer-message-condense)

But for some reason the overlay is not displayed. I tried debugging the issue but couldn't figure out the reason. Any idea where the overlay is being deleted? It doesn't seem to be in ctrlf--condense-overlays at least. There is also the problem that minibuffer-message-overlay would need to be unset or something (I am not sure) when the overlays are condensed.

PS: Is there another reason why condensing overlays is needed besides the positioning of the cursor? If not, is condensing absolutely needed?

raxod502 commented 2 years ago

Urgh, unfortunately, I do not have any insight without digging into it extensively. This took me a long time to figure out the first time. I do not believe that condensing was needed for any reason other than to fix the cursor positioning bug, so if there is another way that the cursor can be displayed in the right place, then we can certainly get rid of it. The current code was just the result of my trying to get things working properly in a desperate sort of way, and not any sort of best practice.