[Lldb-commits] [PATCH] D143695: [lldb] Make repeat commands work for regex commands

[Dave Lee via Phabricator via lldb-commits]

kastiglione added inline comments.


================
Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2005
     // stored "repeat command" which we should give a chance to produce it's
     // repeat command, even though we don't add repeat commands to the history.
+    Args command_args(command_string);
----------------
jingham wrote:
> You at least need to fix this comment, since you aren't checking empty_command anymore.
> 
> It would also be good to explain the logic here, since I'm not entirely sure why this patch fixes the problem.  It should just mean that we are setting the "previous repeat command" twice, once above and once here, in the case where we didn't set "empty_command", so it's not clear why that helps.
> 
> I also think it's wrong to set a repeat command when add_to_history is false.  That generally happens if some code, like a breakpoint command, uses HandleCommand, i.e. that's a command the user didn't directly execute.  So it doesn't make sense that that non-user command should set the user repeat command.
> 
I've updated the logic here, keeping `add_to_history || empty_command` and adding a third check, which is the caller can specific an argument for `allow_repeat_command`. This let's `CommandObjectRegexCommand` opt-in to the repeat behavior without changing the semantics of `add_to_history`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143695/new/

https://reviews.llvm.org/D143695