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

[Walter via lldb-commits]

> 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”.

That is correct. The GetRepeatCommand function gets the full command with
the type Args. I could look for the current subcommand name within the Args
and use the tokens up to that position as the repeat command, however, it
would fail if there's token with the same value, like "memory read read".
It might be a very degenerate case and I'm okay with implementing this and
support enums if you agree with it. If you have a better idea, it'd be
great.

On Mon, Apr 6, 2020 at 12:23 PM Jim Ingham <jingham at apple.com> wrote:

> 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>
>
>

-- 
Walter Erquínigo Pezo

Every problem has a simple, fast and wrong solution.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200406/2b98b01b/attachment.html>