Page MenuHomePhabricator

remove somewhat dangerous 'd'(etach) and 'k'(ill) shortcuts
ClosedPublic

Authored by llunak on Oct 12 2019, 4:41 AM.

Details

Summary
'd' would be much better used for up/down shortcuts, and this also removes the possibility of ruining the whole debugging session by accidentally hitting 'd' or 'k'. Also change menu to have both 'detach and resume' and 'detach suspended' to make it clear which one is which. See discussion at https://reviews.llvm.org/D68541 .

Diff Detail

Repository
rLLDB LLDB

Event Timeline

llunak created this revision.Oct 12 2019, 4:41 AM

So it seems like we should be either moving everything to the process menu, or keeping everything and introducing a confirmation dialog for any dangerous commands (kill, both detaches). This patch seems to keep some dangerous commands (detach suspended), remove one (detach), and add another (kill).

lldb/source/Core/IOHandler.cpp
3766

Do we really want to remove 'd' and not 'D' as well? We now handle this in the process menu right?

4331

Do we really want to remove 'd' and not 'D' as well? We now handle this in the process menu right?

4340–4349

Is this not handled in the process menu? 'k' seems dangerous. Either that or we can introduce a confirmation dialog?

If removing 'd' was only to make it available for "up" and "down" in https://reviews.llvm.org/D68541, then maybe we should switch 'D' to "detach and let run? Or is that what we did, but the comment on line 4331 is now out of date?

If removing 'd' was only to make it available for "up" and "down" in https://reviews.llvm.org/D68541, then maybe we should switch 'D' to "detach and let run? Or is that what we did, but the comment on line 4331 is now out of date?

Making 'd' available for "down" was the original motivation, but not having single-key shortcuts with "dangerous" seemed like a good thing on its own (or maybe I thought I needed strong arguments for reassigning 'd', I don't quite remember). I don't have a strong opinion on 'D'. On the one hand shift+d is harder to hit by accident. On the other hand that's still somewhat possible, the action is still available as 'F3,d' in the menu, and gdb tui does not seem to have a single "dangerous" shortcut (according to https://sourceware.org/gdb/onlinedocs/gdb/TUI-Single-Key-Mode.html). And I've never used the detach action, so I'd prefer if somebody else decided this.

llunak updated this revision to Diff 281706.Jul 29 2020, 12:39 PM

Updated for current git, and clang-format-ed.

llunak added inline comments.Jul 29 2020, 12:44 PM
lldb/source/Core/IOHandler.cpp
4340–4349

I think you got this backwards, it doesn't add a shortcut for kill, it removes it.

clayborg accepted this revision.Jul 29 2020, 1:00 PM
This revision is now accepted and ready to land.Jul 29 2020, 1:00 PM