Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Embark and consult-line are not working properly #637

Closed
rickalex21 opened this issue May 19, 2023 · 12 comments
Closed

Embark and consult-line are not working properly #637

rickalex21 opened this issue May 19, 2023 · 12 comments

Comments

@rickalex21
Copy link

Hello,

I seem to have a problem with embark and consult-line.

When I:

  1. Search a file with consult--find.
  2. Open the file with C-v
  3. Search with consult-line mapped to "/".

I'm not in the minibuffer, I end up deleting deleting characters with x because
I have evil. See video below. I think the problem seems to be with
windmove-display-right but I'm not sure.

My overall goals are to:

  • open a window to the right with C-v
  • open a window to the bottom with C-s

Here is my relevant configuration for embark.

I was taking a pos argument and pcase it but first I need to get this fixed.

(defun my/embark-find-file-other-window-vsplit ()
  "Open the file in the other window."
  (interactive)
  ;;(pcase pos
  ;;  ('vsplit (windmove-display-right))
  ;;  ('split (windmove-display-down))
  ;;  (_ (windmove-display-right)))
    (windmove-display-right)
  (embark--act 'find-file-other-window (car (embark--targets)) embark-quit-after-action))

;;(define-key minibuffer-local-map (kbd "C-v") #'(lambda ()(interactive)(my/embark-find-file-other-window 'vsplit)))
(define-key minibuffer-local-map (kbd "C-v") 'my/embark-find-file-other-window-vsplit)
(define-key minibuffer-local-map (kbd "C-s") 'my/embark-find-file-other-window-split)
;;(define-key minibuffer-local-map (kbd "C-s") #'(lambda ()(interactive)(my/embark-find-file-other-window 'split)))

This is a minimal configuration.

This is my main configuration.


Thanks

@oantolin
Copy link
Owner

This a strange one! I think somehow windmove-display-right's clean up code isn't running, because it doesn't notice that embark--act runs a command. Try this version instead:

(defun my/embark-find-file-other-window-vsplit ()
  "Open the file in the other window."
  (interactive)
  (embark--quit-and-run (lambda (file)
                          (windmove-display-right)
                          (find-file-other-window file)) 
                        (plist-get (car (embark--targets)) :target)))

@rickalex21
Copy link
Author

That's weird, your solution does work. Do I need to report this to emacs?
How would I incorporate pcase into it? I get void variable pos.

embark PCH: (void-variable pos)
(keymap-set minibuffer-local-map "C-v" #'(lambda ()(interactive)(my/embark-find-file-other-window 'vsplit)))
(defun my/embark-find-file-other-window (pos)
  "Open the file in the other window."
  (interactive)
  (embark--quit-and-run (lambda (file)
                          (pcase pos
                            ('vsplit (windmove-display-right))
                            ('split (windmove-display-down))
                            (_ (windmove-display-right)))
                          (find-file-other-window file)) 
                        (plist-get (car (embark--targets)) :target)))

@oantolin
Copy link
Owner

That's weird, your solution does work. Do I need to report this to emacs?

No, embark--act does some pretty weird stuff, so I'm not that surprised that windmove-display-right isn't picking up on the command that embark--act runs.

embark PCH: (void-variable pos)

That error message sounds like you are not using lexical binding! Make sure the code is in a file whose first line is:

;;; -*- lexical-binding: t -*-

@oantolin
Copy link
Owner

But honestly, instead of pcase pos, I'd recommend just writing two different functions for horizontal and vertical splits. You wind up writing two different functions anyway when you bind the keys!

(define-key minibuffer-local-map (kbd "C-v") #'(lambda () (interactive) (my/embark-find-file-other-window 'vsplit)))
(define-key minibuffer-local-map (kbd "C-s") #'(lambda () (interactive) (my/embark-find-file-other-window 'split)))

By the way, just use (lambda, not #'(lambda.

@oantolin
Copy link
Owner

I'm closing because I don't think this issue requires any changes in Embark, but feel free to continue the conversation.

@rickalex21
Copy link
Author

That error message sounds like you are not using lexical binding!

I did try wrapping with let with the variable lexical-binding but that did not work.
You're right I will use two functions.

No, embark--act does some pretty weird stuff

It's not embark only, why does this code cause the same problem?

(define-key minibuffer-local-map (kbd "C-v") (lambda ()(interactive)(progn (windmove-display-right) (call-interactively (key-binding (kbd "RET"))) )))

By the way, just use (lambda, not #'(lambda

What's the difference? byte-compiled? According to the Emacs docs Anonymous Functions

The read syntax #' is a short-hand for using function. The following forms are all equivalent:

(lambda (x) (* x x))
(function (lambda (x) (* x x)))
#'(lambda (x) (* x x))

Thanks

@oantolin
Copy link
Owner

I did try wrapping with let with the variable lexical-binding but that did not work.

Did you try, instead of (let ((lexical-binding t)) ...), putting this comment at the top of the file the code is in, before loading that file?

;;; -*- lexical-binding: t -*-

What's the difference?

I am not aware of any difference, which means the more complicated and ugly-looking #'(lambda is completely unnecessary.

@rickalex21
Copy link
Author

Yea the buffer variable does work, I usually try to avoid lexical-binding as a buffer
variable unless I need it. I try to use let when possible, to avoid conflicts with
other functions. This is what I was trying:

(defun my/embark-find-file-other-window-vsplit (test)
  "Open the file in the other window."
  (interactive)
  (let ((pos test))
  (embark--quit-and-run (lambda (file)
                          (pcase pos
                            ('vsplit (windmove-display-right))
                            ('split (windmove-display-down))
                            (_ (windmove-display-right)))
                          (find-file-other-window file)) 
                        (plist-get (car (embark--targets)) :target))))

@oantolin
Copy link
Owner

I usually try to avoid lexical-binding as a buffer variable unless I need it.

I put it at the top of every single elisp file. 🙃

@rickalex21
Copy link
Author

Omar, my knowledge if elisp is limited. Hence, my response. Perhaps one day
I will make a 4000+ line package like you :). I always thought it would mess up
my x's in all my code if I put it on top as a file buffer 🤣. I haven't looked into
lexical-binding, I understand what it is. It changes the scope.

(lambda (x) (message "do something with %s" x))
(lambda (x) (message "Another function using x %s" x))

@rickalex21
Copy link
Author

rickalex21 commented May 21, 2023

Hey Omar, I have another problem. Your solution does not work with consult
ripgrep. It tries to open up a new file named the name of the line found in
the file.

(keymap-set minibuffer-local-map "C-v" 'my/embark-find-file-other-window-vsplit)

I've always used this and it works fine to open up ripgreps but had the problem that it did not say in consult-line.

(define-key minibuffer-local-map (kbd "C-v") (lambda ()(interactive)(progn (windmove-display-right) (call-interactively (key-binding (kbd "RET"))) )))

Thanks

@rickalex21
Copy link
Author

rickalex21 commented May 23, 2023

Update: consult-buffer does not work with this also. It tries to open a new file
in the current buffer directory instead of opening the file where it's located.

I also tried to act on it:

(define-key embark-buffer-map "v" 'my/embark-find-file-other-window-vsplit)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants