svaante / dape

Debug Adapter Protocol for Emacs
GNU General Public License v3.0
489 stars 31 forks source link

hydra menu #6

Closed chmouel closed 2 months ago

chmouel commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 11 months ago

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 commented 11 months ago

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

OlMon commented 11 months ago

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 commented 11 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 3 months ago

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

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 commented 2 months ago

Thanks @vibrys added to wiki https://github.com/svaante/dape/wiki#ergonomic-keybindings

svaante commented 2 months ago

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.

vibrys commented 2 months ago

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:

svaante commented 1 month ago

@vibrys feel free to update, sounds like good changes :+1: