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

Add a hover-minor-mode to premit user can define key map on it (And can remove keybinding when disable it) #12

Closed
zw963 opened this issue Jan 29, 2022 · 11 comments

Comments

@zw963
Copy link
Contributor

zw963 commented Jan 29, 2022

What i means is, like this:

(define-key hover-minor-mode-map (kbd "C-M-x") 'hover-run-or-hot-reload)
(hover-minor-mode 1)

;; I can disable above `C-M-x` hotkey when i disable hover-minor-mode. e.g. i want to use lsp-dart-run
@ericdallo
Copy link
Owner

hum, we do have a map for hover to add custom keybindings hover-mode-map, doesn't that would be enough?
for example we already have this at the code:

(define-key hover-mode-map (kbd "C-x q") #'hover-kill)

@zw963
Copy link
Contributor Author

zw963 commented Jan 29, 2022

hum, we do have a map for hover to add custom keybindings hover-mode-map, doesn't that would be enough? for example we already have this at the code:

(define-key hover-mode-map (kbd "C-x q") #'hover-kill)

So, maybe i misunderstood somethings, hover-mode-map, is for hover-mode, right? it for the buffer of running hover, not for dart file which i am editing? what i want is, i can always run hover-hot-reload-or-restart use C-M-x after i edit done on current dart file, and, cursor not leave dart-mode buffer, sure, i expect i can always disable those keybinding if i want to use lsp-dart-hot-reload if i disable this minor-mode. (we assume binding latter into global binding or dart-mode-key-map, i can override it to use hover when i enable hover-minor-mode)

Anyway, because hover not support Chinese character, i have to switch to and back ...

@ericdallo
Copy link
Owner

Well, you can define your own bindings in any map you want, like dart-mode-map for example.
I don't see why we would create a empty hover-minor-mode just for people adding binding which they can already to dart-mode-map.

@zw963
Copy link
Contributor Author

zw963 commented Jan 29, 2022

I don't see why we would create a empty hover-minor-mode just for people adding binding which they can already to dart-mode-map.

The reason is, i want to use same keybinding when work with lsp-dart or hover.

please check my config:

;; for lsp-dart

(require 'dart-mode)
(add-to-list 'auto-mode-alist '("\\.dart\\'" . dart-mode))

(with-eval-after-load 'lsp-mode
  (setq lsp-signature-auto-activate nil)
  ;; (setq lsp-dart-dap-flutter-hot-reload-on-save t)
  (define-key dart-mode-map (kbd "C-M-x") 'lsp-dart-dap-flutter-hot-reload)
  (define-key dart-mode-map (kbd "<escape>") 'lsp-dart-show-flutter-outline)
  (add-hook 'dart-mode-hook 'lsp)
  )

;; for hover

;; (require 'hover)

(with-eval-after-load 'hover
  (define-key dart-mode-map (kbd "C-M-x") 'hover-run-or-hot-restart)
  (define-key hover-mode-map (kbd "C-M-x") 'hover-run-or-hot-reload)

  (setq
   hover-screenshot-path (concat (getenv "HOME") "/Pictures")
   hover-screenshot-prefix "magpie-"
   hover-observatory-uri "http://127.0.0.1:50300"
   hover-clear-buffer-on-hot-restart t
   hover-hot-reload-on-save t
        )
  )

you can see in my config, both lsp-dart-dap-flutter-hot-reload and hover-run-or-hot-restart defined on dart-mode-map, and when i want to use former, i have to comment (require 'hover) out, then restart my entire emacs.

sure i can define use different keybinding for hover restart and lsp-dart reload, but that not good, because they do almost same things, right?

if we have hover-minor-mode like this:

(defvar hover-minor-mode-map
  (let ((map (make-sparse-keymap)))
  (define-key map (kbd "C-M-x") 'hover-run-or-hot-restart)
    map)
  "Keymap for function `hover-minor-mode'.")

;;;###autoload
(define-minor-mode hover-minor-mode
  "add keybinding."
  :lighter " hover"
  :keymap hover-minor-mode-map
  :group 'hover)

When i enable this minor mode, i can use C-M-x to restart hover, when i disable it, i can use old dart-mode-map defined command to reload too.

user can always select disable or enable this minor mode when starting emacs use config.

@ericdallo
Copy link
Owner

Alright, my point is that we would create a empty minor-mode on hover that does nothing (the define-key or yours makes sense for you but not for everyone), it seems you can already solve that with the code you provided in your config right?

@zw963
Copy link
Contributor Author

zw963 commented Jan 29, 2022

Alright, my point is that we would create a empty minor-mode on hover that does nothing (the define-key or yours makes sense for you but not for everyone), it seems you can already solve that with the code you provided in your config right?

Yes, anyway, i expected to write less code in my config. i consider this is a quite usual way to resolve this issue, it very suitable for current case, you can add the default binding of yours as hover-minor-mode default keybinding, that fine, because i can always rebinding/unbinding it, even, let user to enable this minor mode (set minor mode disable as default)

@ericdallo
Copy link
Owner

ericdallo commented Jan 29, 2022

yeah, anyway, I think we can have this common minor mode on hover.el for users tweak as well, sounds ok to me

@zw963
Copy link
Contributor Author

zw963 commented Jan 29, 2022

yeah, anyway, I think we can have this common minor mode on hover.el for users tweak as well, sounds ok to me

Okay, thank you, we need improve README.md too for mention it, i can contribute a PR for this, but mostly, my bad english need tweak a lot i guess, so, please do it. 😄

@zw963
Copy link
Contributor Author

zw963 commented Jan 29, 2022

I create a PR for this, please check

https://github.com/ericdallo/hover.el/pull/13/files

@zw963
Copy link
Contributor Author

zw963 commented Feb 7, 2022

@ericdallo , BTW, i don't know if flutter linux desktop if works on your's NixOS, for recent days, i found our project start to working on linux desktop, use linux desktop with lsp-dart, give me a much better experience then android emulator too, so, if you never try it, pleave give a try, it even more better then hover on some aspects, e.g. chinese input method support.

@ericdallo
Copy link
Owner

Makes sense @zw963, we use at work because of complex plugins setups, but it should work as well the flutter desktop, thanks for the info

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