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

hydra menu #6

Closed
chmouel opened this issue Oct 21, 2023 · 16 comments
Closed

hydra menu #6

chmouel opened this issue Oct 21, 2023 · 16 comments

Comments

@chmouel
Copy link

chmouel commented Oct 21, 2023

It would be nice to include a hydra menu, i found it a litlle bit more user friendly than keybinding or having to issue command in console, I have started a simple one loosely based on https://github.com/emacs-lsp/dap-mode/blob/master/dap-hydra.el

(defhydra dape-hydra (:color pink :hint nil :foreign-keys run)
  "
^Stepping^          ^Breakpoints^         ^Info
^^^^^^^^-----------------------------------------------------------
_n_: Next           _bb_: Toggle          _si_: Info
_i_: Step in        _bd_: Delete          _sm_: Memory
_o_: Step out       _ba_: Add             _ss_: Select Stack
_c_: Continue       _bD_: Delete all       _R_: Repl
_r_: Restart        _bl_: Set log message
_Q_: Disconnect
"
  ("n" dape-next)
  ("i" dape-step-in)
  ("o" dape-step-out)
  ("c" dape-continue)
  ("r" dape-restart)
  ("bb" dape-toggle-breakpoint)
  ("be" dape-expression-breakpoint)
  ("bd" dape-remove-breakpoint-at-point)
  ("bD" dape-remove-all-breakpoints)
  ("bl" dape-log-breakpoint)
  ("si" dape-info)
  ("sm" dape-read-memory)
  ("ss" dape-select-stack)
  ("R"  dape-repl)
  ("q" nil "quit" :color blue)
  ("Q" dape-kill :color red))

;;;###autoload
(defun dape-hydra ()
  "Run `dape-hydra/body'."
  (interactive)
  (dape-hydra/body))
@OlMon
Copy link

OlMon commented Oct 21, 2023

With the nature of using built-ins as much as possible I would prefer a transient:

(transient-define-prefix dape-transient ()
       "Transient for dape."
        [["Stepping"
         ("n" "Next" dape-next :transient t)
         ("i" "Step in" dape-step-in :transient t)
         ("o" "Step out" dape-step-out :transient t)
         ("c" "Continue" dape-continue :transient t)
         ("r" "restart" dape-restart :transient t)]
         ["Breakpoints"
         ("bb" "Next" dape-toggle-breakpoint :transient t)
         ("bd" "Step in" dape-remove-breakpoint-at-point :transient t)
         ("bD" "Continue" dape-remove-all-breakpoints :transient t)
         ("bl" "restart" dape-log-breakpoint :transient t)]
         ["Info"
         ("si" "Info" dape-toggle-breakpoint :transient t)
         ("sm" "Memory" dape-remove-breakpoint-at-point :transient t)
         ("ss" "Select Stack" dape-remove-all-breakpoints :transient t)
         ("R" "Repl" dape-log-breakpoint :transient t)]
         ["Quit"
         ("qq" "Quit" dape-quit :transient nil)
         ("qk" "Kill" dape-kill :transient nil)]])

@chmouel
Copy link
Author

chmouel commented Oct 21, 2023

@OlMon this looks much better thanks, it would be useful i think if this is included by default (or at least documented in README)

@svaante
Copy link
Owner

svaante commented Oct 21, 2023

Hi @chmouel and thanks for your input!
Have you tried enabling repeat-mode? In d6d7967 the repeat keymap is enabled after running dape which helps a lot with usability IMO.
Hydra or transient seams to win on the discoverability axis though...

As @OlMon stated I am going to add hydra as an dependency, but I am not against having it in the wiki for hydra users.

@svaante
Copy link
Owner

svaante commented Oct 21, 2023

With the nature of using built-ins as much as possible I would prefer a transient:

(transient-define-prefix dape-transient ()
       "Transient for dape."
        [["Stepping"
         ("n" "Next" dape-next :transient t)
         ("i" "Step in" dape-step-in :transient t)
         ("o" "Step out" dape-step-out :transient t)
         ("c" "Continue" dape-continue :transient t)
         ("r" "restart" dape-restart :transient t)]
         ["Breakpoints"
         ("bb" "Next" dape-toggle-breakpoint :transient t)
         ("bd" "Step in" dape-remove-breakpoint-at-point :transient t)
         ("bD" "Continue" dape-remove-all-breakpoints :transient t)
         ("bl" "restart" dape-log-breakpoint :transient t)]
         ["Info"
         ("si" "Info" dape-toggle-breakpoint :transient t)
         ("sm" "Memory" dape-remove-breakpoint-at-point :transient t)
         ("ss" "Select Stack" dape-remove-all-breakpoints :transient t)
         ("R" "Repl" dape-log-breakpoint :transient t)]
         ["Quit"
         ("qq" "Quit" dape-quit :transient nil)
         ("qk" "Kill" dape-kill :transient nil)]])

I have not used transient so I will give this a spin and get back to the issue. As of now I will be sticking with repeat-mode bindings but this might change. I do like that repeat-mode is lightweight and is all ready integrated with several other of emacs packages.

Hydra or transient seams to win on the discoverability axis though...

@OlMon
Copy link

OlMon commented Oct 26, 2023

I could be wrong, but I don't think that transient and repeat-mode are in any conflict. repeat-mode triggers after using a keybind, while transient uses this additional small buffer that "catches" all keypresses.

Personally for me I would prefer to have both available.

I also just saw that I copy pasted the "Info" section to fast and forget to change the functions in the individual calls, sorry for that....

@svaante
Copy link
Owner

svaante commented Dec 6, 2023

Sorry for taking so long to respond.

I was unaware that transient was merged into emacs, so it makes sense to add it.
Info mode buttons has been removed.

Transient are giving me some troubles, the default window action is in conflict with dapes. For all the dape-buffer-window-arrangment left/right and gud. If somebody has any good example on to setup transient to play nice with both those configuration options I will include it into dape.

@svaante
Copy link
Owner

svaante commented Dec 8, 2023

If somebody could solve these issues I am for adding an transient map to dape.

@OlMon
Copy link

OlMon commented Dec 10, 2023

I am not sure if this is something dape.el should solve. This seems a "bug" between transient and gdb-mi and should be something discussed upstream. Not even really a bug, but more like they use the same window functions and this causes to not "look-good" together.

@svaante
Copy link
Owner

svaante commented Dec 11, 2023

It's not really an gdb-mi issue really as dape does not use gdb-mi in that way. Really only the table function from gdb-mi.
The most sensible approach would be to use display-buffer-below-selected but that still breaks ui. Can't really figure out why thou.

@bertulli
Copy link

bertulli commented Jan 17, 2024

With the nature of using built-ins as much as possible I would prefer a transient:

(transient-define-prefix dape-transient ()
       "Transient for dape."
        [["Stepping"
         ("n" "Next" dape-next :transient t)
         ("i" "Step in" dape-step-in :transient t)
         ("o" "Step out" dape-step-out :transient t)
         ("c" "Continue" dape-continue :transient t)
         ("r" "restart" dape-restart :transient t)]
         ["Breakpoints"
         ("bb" "Next" dape-toggle-breakpoint :transient t)
         ("bd" "Step in" dape-remove-breakpoint-at-point :transient t)
         ("bD" "Continue" dape-remove-all-breakpoints :transient t)
         ("bl" "restart" dape-log-breakpoint :transient t)]
         ["Info"
         ("si" "Info" dape-toggle-breakpoint :transient t)
         ("sm" "Memory" dape-remove-breakpoint-at-point :transient t)
         ("ss" "Select Stack" dape-remove-all-breakpoints :transient t)
         ("R" "Repl" dape-log-breakpoint :transient t)]
         ["Quit"
         ("qq" "Quit" dape-quit :transient nil)
         ("qk" "Kill" dape-kill :transient nil)]])

I noticed some of the functions have changed name and others don't correspond to their section: may I suggest?

(transient-define-prefix dape-transient ()
       "Transient for dape."
        [["Stepping"
         ("n" "Next" dape-next :transient t)
         ("i" "Step in" dape-step-in :transient t)
         ("o" "Step out" dape-step-out :transient t)
         ("c" "Continue" dape-continue :transient t)
         ("r" "Restart" dape-restart :transient t)]
         ["Breakpoints"
         ("bb" "Toggle" dape-breakpoint-toggle :transient t)
         ("bd" "Delete" dape-breakpoint-remove-at-point :transient t)
         ("bD" "Delete all" dape-breakpoint-remove-all :transient t)
         ("bl" "Log" dape-breakpoint-log :transient t)]
         ["Info"
         ("si" "Info" dape-info :transient t)
         ("sm" "Memory" dape-read-memory :transient t)
         ("ss" "Select Stack" dape-select-stack :transient t)
         ("R" "Repl" dape-repl :transient t)]
         ["Quit"
         ("qq" "Quit" dape-quit :transient nil)
         ("qk" "Kill" dape-kill :transient nil)]])

@svaante
Copy link
Owner

svaante commented Jan 21, 2024

I am still having issues with transient and the *dape-*buffers and can't find a suitable value for transient-display-buffer-action. If anybody would take this issue on I am still open to include it in dape.el

@vibrys
Copy link

vibrys commented Aug 2, 2024

I've just tried both transient and hydra methods (hydra config update below). Hydra version works way better for me due to:

  • while hydra is active it just overrides only those keys that it uses, while leaving the rest keys for other activities (code browsing for example)
  • it stays persistent on the screen until I press quit key.
  • both the items above look to be default for hydra, so no time spent on figuring on how to do this.

Here's the updated version of hydra:

(defhydra dape-hydra (:color pink :hint nil :foreign-keys run)
    "
^Stepping^          ^Breakpoints^               ^Info
^^^^^^^^-----------------------------------------------------------
_d_: init           _bb_: Toggle (add/remove)   _si_: Info
_n_: Next           _bd_: Delete                _sm_: Memory
_i_: Step in        _bD_: Delete all            _ss_: Select Stack
_o_: Step out       _bl_: Set log message       _R_: Repl
_c_: Continue
_r_: Restart
_Q_: Disconnect
"
    ("d" dape)
    ("n" dape-next)
    ("i" dape-step-in)
    ("o" dape-step-out)
    ("c" dape-continue)
    ("r" dape-restart)
    ("ba" dape-breakpoint-toggle)
    ("bb" dape-breakpoint-toggle)
    ("be" dape-breakpoint-expression)
    ("bd" dape-breakpoint-remove-at-point)
    ("bD" dape-breakpoint-remove-all)
    ("bl" dape-breakpoint-log)
    ("si" dape-info)
    ("sm" dape-read-memory)
    ("ss" dape-select-stack)
    ("R"  dape-repl)
    ("q" nil "quit" :color blue)
    ("Q" dape-kill :color red))

@svaante and other dape contributors, thank you for dape. I've been looking for your solution for longer time. For now I have configured it to debug C++ code (Linux) and it is reliable, fast, does not require other heavy weight packages (!) and .... it works out of the box!!!

I'm looking forward to use it with python code (my next step). I appreciate it alot. Thank you.

@svaante
Copy link
Owner

svaante commented Aug 10, 2024

@svaante
Copy link
Owner

svaante commented Aug 10, 2024

As stated before repeat-mode is sufficient and is built in.

Transient might be an alternative, as it's built in. But as it messes with 'side windows by default it doesn't seam to be worth the hassle at this point.

Closing this issue! Feel free to populate the wiki with an transient alternative.

@svaante svaante closed this as completed Aug 10, 2024
@vibrys
Copy link

vibrys commented Sep 7, 2024

As stated before repeat-mode is sufficient and is built in.

agreed

Hydra or transient seams to win on the discoverability axis though...

agreed

As hydra emerged on Wiki page, some time has been spent to polish it a little bit. What do you think?:

(defun ccpp/post-init-hydra ()
  (defhydra dape-hydra (:hint nil)
    "
^Stepping^           ^Breakpoints^               ^Info
^^^^^^^^-----------------------------------------------------------
_n_: Next            _bb_: Toggle (add/remove)   _si_: Info
_i_/_o_: Step in/out   _bd_: Delete                _sm_: Memory
_<_/_>_: Stack </>     _bD_: Delete all            _ss_: Select Stack
_c_: Continue        _bl_: Set log message       _R_: Repl
_r_: Restart
            _d_: Init   _k_: kill   _q_: quit
"
    ("n" dape-next)
    ("i" dape-step-in)
    ("o" dape-step-out)
    ("<" dape-stack-select-up)
    (">" dape-stack-select-down)
    ("c" dape-continue)
    ("r" dape-restart)
    ("ba" dape-breakpoint-toggle)
    ("bb" dape-breakpoint-toggle)
    ("be" dape-breakpoint-expression)
    ("bd" dape-breakpoint-remove-at-point)
    ("bD" dape-breakpoint-remove-all)
    ("bl" dape-breakpoint-log)
    ("si" dape-info)
    ("sm" dape-read-memory)
    ("ss" dape-select-stack)
    ("R"  dape-repl)
    ("d" dape)
    ("k" dape-kill :color blue)
    ("q" dape-quit :color blue)))

changes + rationale:

  • any other key than defined in Hydra, closes the dialog. Then the key pressed is used in emacs.
    • Less error prone for beginners.
    • "q" is no longer needed
  • stack up/down added.
  • dialog made more compact

@svaante
Copy link
Owner

svaante commented Sep 11, 2024

@vibrys feel free to update, sounds like good changes 👍

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

5 participants