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

H-k remapping throwing error when trying to use magit-discard on a diff hunk #692

Open
erganemic opened this issue Dec 10, 2024 · 11 comments

Comments

@erganemic
Copy link

Emacs version: GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.43, cairo version 1.18.2) of 2024-10-10
Meow version: meow-20241206.922/
Using the default QWERTY setup.

Steps to reproduce:

  • go to any git repo and make a change in a file that you don't mind discarding
  • start magit with M-x magit-status
  • navigate to the magit diff hunk with the change you made
  • SPC k to try and access the original keybind on k
  • you should get the following error:
magit-apply-hunk: Wrong type argument: symbolp, #[0 " \"�!�!�\0��\0�!�\0��\0!" [magit-delete-thing "k" lookup-key current-local-map command-remapping call-interactively commandp] 5 nil nil]

Note that this works fine if you try and discard the contents of an entire file; it's only when you are attempting to use the context-sensitive hunk discard that I get that error.

@erganemic erganemic changed the title H-k remapping not working for magit hunk discards H-k remapping throwing error when trying to use magit-discard on a diff hunk Dec 10, 2024
@kmontag
Copy link

kmontag commented Dec 10, 2024

I can confirm a similar error on emacs 29.4 on MacOS, with meow at 73f2dd2.

After pressing SPC k on a magit diff hunk:

Debugger entered--Lisp error: (wrong-type-argument symbolp #f(compiled-function () (interactive nil) #<bytecode -0x131a4eeb6457f1d8>))
  magit-apply-patch(#<magit-file-section "[redacted]" [file unstaged status] 111-[146-]273> ("--reverse") #("[redacted]" 59 145 (fontified nil)))
  magit-apply-hunk(#<magit-hunk-section (nil (1 5) (1 6)) [hunk file unstaged status] 146-[162-]273> "--reverse")
  magit-discard-apply(#<magit-hunk-section (nil (1 5) (1 6)) [hunk file unstaged status] 146-[162-]273> magit-apply-hunk)
  magit-discard-hunk(#<magit-hunk-section (nil (1 5) (1 6)) [hunk file unstaged status] 146-[162-]273>)
  magit-discard()
  funcall-interactively(magit-discard)
  call-interactively(magit-discard)
  #f(compiled-function () (interactive nil) #<bytecode -0x131a4eeb6457f1d8>)()
  funcall-interactively(#f(compiled-function () (interactive nil) #<bytecode -0x131a4eeb6457f1d8>))
  meow--execute-kbd-macro("H-k")
  (cond (meow-motion-mode (meow--execute-kbd-macro (format "H-%s" motion-key))) (user-cmd (funcall user-cmd)))
  rift--meow-leader-k()
  funcall-interactively(rift--meow-leader-k)
  meow--keypad-execute(rift--meow-leader-k)
  meow--keypad-try-execute()
  meow--keypad-handle-input-event(107)
  meow--keypad-handle-input-with-keymap(107)
  meow-keypad-start-with(nil)
  meow-keypad()
  funcall-interactively(meow-keypad)
  command-execute(meow-keypad)

With meow disabled, I'm able to discard the change via k without issue.

@kmontag
Copy link

kmontag commented Dec 10, 2024

Also, it looks like this was working as of 2d352b9.

The following commit 809b059 fails when discarding a hunk, though with a different error: meow-keypad-start-with: Wrong type argument: commandp, meow-keypad-self-insert.

@DogLooksGood
Copy link
Collaborator

DogLooksGood commented Dec 10, 2024

meow-keypad-self-insert should be removed in a recent commit. What's the problem that prevents you from upgrading to the latest commit?

@erganemic
Copy link
Author

erganemic commented Dec 10, 2024

I installed the latest commit of meow (73f2dd2) (+ verified that the commit in the repo in my elpa directory was up-to-date with the current state of master, + that there were no stale eln or elc files), and I'm still seeing the error in the original post when I attempt to magit-discard a hunk.

@DogLooksGood
Copy link
Collaborator

I cannot reproduce with my config. It will be nice if some one can make a single el file to reproduce.

;;; test.el
;;; Use it like emacs -Q -L . -l test.el

(add-to-list 'load-path "/path/to/latest/meow")

(require 'meow)

;;; Some configs

(meow-global-mode)

@erganemic
Copy link
Author

erganemic commented Dec 19, 2024

I can reproduce this with the following test.el file, after having cloned meow's latest master (f74017c). Apologies if this is a pain to reproduce, it requires magit, so I just added the libraries it needs from my normal install to the load-path. Not sure if there's a better way of doing that.

(add-to-list 'load-path "~/Scratch/meow")
(add-to-list 'load-path "/home/erganemic/.emacs.d/elpa/magit-20241219.950")
(add-to-list 'load-path "/home/erganemic/.emacs.d/elpa/dash-20240510.1327")
(add-to-list 'load-path "/home/erganemic/.emacs.d/elpa/magit-section-20241217.845")
(add-to-list 'load-path "/home/erganemic/.emacs.d/elpa/with-editor-20241201.1419")
(require 'magit)
(defun meow-setup ()
  (setq meow-cheatsheet-layout meow-cheatsheet-layout-qwerty)
  (meow-motion-overwrite-define-key
   '("j" . meow-next)
   '("k" . meow-prev)
   '("<escape>" . ignore))
  (meow-leader-define-key
   ;; SPC j/k will run the original command in MOTION state.
   '("j" . "H-j")
   '("k" . "H-k")
   ;; Use SPC (0-9) for digit arguments.
   '("1" . meow-digit-argument)
   '("2" . meow-digit-argument)
   '("3" . meow-digit-argument)
   '("4" . meow-digit-argument)
   '("5" . meow-digit-argument)
   '("6" . meow-digit-argument)
   '("7" . meow-digit-argument)
   '("8" . meow-digit-argument)
   '("9" . meow-digit-argument)
   '("0" . meow-digit-argument)
   '("/" . meow-keypad-describe-key)
   '("?" . meow-cheatsheet))
  (meow-normal-define-key
   '("0" . meow-expand-0)
   '("9" . meow-expand-9)
   '("8" . meow-expand-8)
   '("7" . meow-expand-7)
   '("6" . meow-expand-6)
   '("5" . meow-expand-5)
   '("4" . meow-expand-4)
   '("3" . meow-expand-3)
   '("2" . meow-expand-2)
   '("1" . meow-expand-1)
   '("-" . negative-argument)
   '(";" . meow-reverse)
   '("," . meow-inner-of-thing)
   '("." . meow-bounds-of-thing)
   '("[" . meow-beginning-of-thing)
   '("]" . meow-end-of-thing)
   '("a" . meow-append)
   '("A" . meow-open-below)
   '("b" . meow-back-word)
   '("B" . meow-back-symbol)
   '("c" . meow-change)
   '("d" . meow-delete)
   '("D" . meow-backward-delete)
   '("e" . meow-next-word)
   '("E" . meow-next-symbol)
   '("f" . meow-find)
   '("g" . meow-cancel-selection)
   '("G" . meow-grab)
   '("h" . meow-left)
   '("H" . meow-left-expand)
   '("i" . meow-insert)
   '("I" . meow-open-above)
   '("j" . meow-next)
   '("J" . meow-next-expand)
   '("k" . meow-prev)
   '("K" . meow-prev-expand)
   '("l" . meow-right)
   '("L" . meow-right-expand)
   '("m" . meow-join)
   '("n" . meow-search)
   '("o" . meow-block)
   '("O" . meow-to-block)
   '("p" . meow-yank)
   '("q" . meow-quit)
   '("Q" . meow-goto-line)
   '("r" . meow-replace)
   '("R" . meow-swap-grab)
   '("s" . meow-kill)
   '("t" . meow-till)
   '("u" . meow-undo)
   '("U" . meow-undo-in-selection)
   '("v" . meow-visit)
   '("w" . meow-mark-word)
   '("W" . meow-mark-symbol)
   '("x" . meow-line)
   '("X" . meow-goto-line)
   '("y" . meow-save)
   '("Y" . meow-sync-grab)
   '("z" . meow-pop-selection)
   '("'" . repeat)
   '("<escape>" . ignore)))
(require 'meow)
(meow-setup)
(meow-global-mode 1)

@Zilong-Li
Copy link

I second this

@DogLooksGood
Copy link
Collaborator

output.mp4

Am I missing something? I'm using the following test.el

(add-to-list 'load-path "~/.emacs.d/straight/build/meow/")
(add-to-list 'load-path "~/.emacs.d/straight/build/dash/")
(add-to-list 'load-path "~/.emacs.d/straight/build/magit/")
(add-to-list 'load-path "~/.emacs.d/straight/build/magit-section")
(add-to-list 'load-path "~/.emacs.d/straight/build/with-editor")

(require 'meow)
(require 'dash)
(require 'magit)

(meow-global-mode)

(meow-motion-overwrite-define-key
   '("j" . meow-next)
   '("k" . meow-prev))

(meow-leader-define-key
   ;; SPC j/k will run the original command in MOTION state.
   '("j" . "H-j")
   '("k" . "H-k"))

@erganemic
Copy link
Author

Apologies if this wasn't clear enough in my original post:

Note that this works fine if you try and discard the contents of an entire file; it's only when you are attempting to use the context-sensitive hunk discard that I get that error.

Here's a video demonstrating what I do to provoke this with the test.el I provided:

output.mp4

@DogLooksGood
Copy link
Collaborator

I think I have some clue. Maybe it's related to #700 .

But I'm thinking if we can remove the needs for H-j/H-k remapping, by letting the unbound leader dispatch to call the original command, without wrapping it with a lambda.

@DogLooksGood
Copy link
Collaborator

Note #705 for another fix.

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

4 participants