[Lldb-commits] [PATCH] D77444: [commands] Support autorepeat in SBCommands

[Jim Ingham via lldb-commits]

Then only time that I can find where I customized the string based on the incoming full command was for “source list” because I needed to know whether the listing was going forward or backward, so I needed to write that into the “keep doing what you were doing” command.  That could have also been implemented by remembering the direction as well as the “current source location” internally, and then have “source list” mean “keep going the direction you were going.”  So I don’t think dynamically constructing the next command string is necessary to cover the needed cases.

BTW, I thought when a command returned the command string for the next command, the command interpreter prepended it with the chain of parent commands containing the command that was presenting the “next command string”.  If it doesn’t do that, it probably should.  A command shouldn’t really know where it is sitting in the command hierarchy, so somebody has to do that for it.  

I don’t know of a case where a command would want its repeat command to be a wholly different command.  That seems odd, but if we do find we need that internally, we could add some way of saying “this command is not a variant of me…”  But in any case, for these purposes, it seems like the three useful cases are really “no repeat”, “repeat the command I was given” and “invoke my continue-from-where-I-left-off” command - which by convention is the command with no arguments.  If we make sure that when the repeat command is actually used the command interpreter adds the command’s parents, then I think we could do this with an enum.

Jim


> On Apr 6, 2020, at 11:57 AM, walter erquinigo via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> wallace updated this revision to Diff 255426.
> wallace added a comment.
> Herald added a subscriber: mgorny.
> 
> - Moved the test to gtest. It's much better this way and I learned gtest
> - Changed the API. Some notes:
> 
> Within the scope of a subcommand, the subcommand doesn't know the parent's
> command name. It only knows its own name and it doesn't have any referrence to
> its parent. That makes it very difficult to implement an enum option for
> eCommandNameRepeat, as @jingham suggested. The GetRepeatCommand signature also
> doesn't provide useful info about the parsed arguments.
> 
> I took a look at the existing implementations for GetRepeatCommand, and it seems
> that most of them just return nullptr, "", or the same command without flags.
> 
> I don't want to change any existing core command interpreter function, so I
> think that a simple API that covers most of the cases is just providing nullptr
> for repeating the same command, "" for not repeating, or a custom string for
> any other case. If in the future anyone needs something very customized, a new
> override could be created, but I don't think this will happen anytime soon.
> 
> Another option is to provide a callback function instead of a string, but I
> don't know if it's too much.
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D77444/new/
> 
> https://reviews.llvm.org/D77444
> 
> Files:
>  lldb/include/lldb/API/SBCommandInterpreter.h
>  lldb/source/API/SBCommandInterpreter.cpp
>  lldb/unittests/Interpreter/CMakeLists.txt
>  lldb/unittests/Interpreter/TestAutoRepeat.cpp
> 
> <D77444.255426.patch>