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

[llunak]

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 .

[clayborg]

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?

[clayborg]

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?

[llunak]

In D68908#2174222, @clayborg wrote:

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 for current git, and clang-format-ed.

[llunak]

lldb/source/Core/IOHandler.cpp

4340–4349

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

lldb/source/Core/IOHandler.cpp

 
     eMenuID_Process,
     eMenuID_ProcessAttach,
-    eMenuID_ProcessDetach,
+    eMenuID_ProcessDetachResume,
+    eMenuID_ProcessDetachSuspended,
     eMenuID_ProcessLaunch,
     eMenuID_ProcessContinue,
     eMenuID_ProcessHalt,

     }
       return MenuActionResult::Handled;
 
-    case eMenuID_ProcessDetach: {
+    case eMenuID_ProcessDetachResume:
+    case eMenuID_ProcessDetachSuspended: {
       ExecutionContext exe_ctx =
           m_debugger.GetCommandInterpreter().GetExecutionContext();
       if (exe_ctx.HasProcessScope()) {
         Process *process = exe_ctx.GetProcessPtr();
         if (process && process->IsAlive())
-          process->Detach(false);
+          process->Detach(menu.GetIdentifier()==eMenuID_ProcessDetachSuspended);
       }
     }
       return MenuActionResult::Handled;

         {KEY_NPAGE, "Page down"},
         {'b', "Set breakpoint on selected source/disassembly line"},
         {'c', "Continue process"},
-        {'d', "Detach and resume process"},
         {'D', "Detach with process suspended"},
         {'h', "Show help dialog"},
-        {'k', "Kill process"},
         {'n', "Step over (source line)"},
         {'N', "Step over (single instruction)"},
         {'o', "Step out"},

       }
       return eKeyHandled;
 
-    case 'd': // 'd' == detach and let run
     case 'D': // 'D' == detach and keep stopped
     {
       ExecutionContext exe_ctx =
           m_debugger.GetCommandInterpreter().GetExecutionContext();
       if (exe_ctx.HasProcessScope())
-        exe_ctx.GetProcessRef().Detach(c == 'D');
+        exe_ctx.GetProcessRef().Detach(true);
     }
       return eKeyHandled;
 
-    case 'k':
-      // 'k' == kill
-      {
-        ExecutionContext exe_ctx =
-            m_debugger.GetCommandInterpreter().GetExecutionContext();
-        if (exe_ctx.HasProcessScope())
-          exe_ctx.GetProcessRef().Destroy(false);
-      }
-      return eKeyHandled;
-
     case 'c':
       // 'c' == continue
       {

     process_menu_sp->AddSubmenu(MenuSP(new Menu(
         "Attach", nullptr, 'a', ApplicationDelegate::eMenuID_ProcessAttach)));
     process_menu_sp->AddSubmenu(MenuSP(new Menu(
-        "Detach", nullptr, 'd', ApplicationDelegate::eMenuID_ProcessDetach)));
+        "Detach and resume", nullptr, 'd', ApplicationDelegate::eMenuID_ProcessDetachResume)));
+    process_menu_sp->AddSubmenu(MenuSP(new Menu(
+        "Detach suspended", nullptr, 's', ApplicationDelegate::eMenuID_ProcessDetachSuspended)));
     process_menu_sp->AddSubmenu(MenuSP(new Menu(
         "Launch", nullptr, 'l', ApplicationDelegate::eMenuID_ProcessLaunch)));
     process_menu_sp->AddSubmenu(MenuSP(new Menu(Menu::Type::Separator)));