[Lldb-commits] [PATCH] D37923: Implement interactive command interruption

[Leonard Mosescu via lldb-commits]

So, how about I look into exposing WasInterrupted() through SB APIs in a
follow up change?

On Tue, Sep 19, 2017 at 11:36 AM, Jim Ingham <jingham at apple.com> wrote:

> Xcode does, I don't know about other UI's.
>
> Jim
>
> > On Sep 19, 2017, at 11:35 AM, Leonard Mosescu <mosescu at google.com>
> wrote:
> >
> > I agree Jim. I'd like like to build thing incrementally - checking in
> the current change as is does not preclude adding the SB APIs while it does
> provide the foundation.
> >
> > I think that going after the scenarios you mention is a significant
> increase in scope. Are people even hooking up DispatchInterrupt() from
> whatever interactive driver they use?
> >
> > On Tue, Sep 19, 2017 at 11:27 AM, Jim Ingham <jingham at apple.com> wrote:
> > We agreed to forwards compatibility because people write big scripts
> that use the SB API, implement GUI's on top of them (more than just Xcode)
> etc.  So we try not to jerk those folks around.  That adds a little more
> responsibility on our part to think carefully about what we add, but the
> notion that we should refrain from making useful functionality available
> because we'd rather not be beholden to our decisions seems really
> wrong-headed to me.
> >
> > And in this case there's a clear use for this. For instance the xnu
> macros have a bunch of Python based commands that spew out pages and pages
> of output.  Those guys would love to make their commands interruptible.  To
> do that they would need to call WasInterrupted.  So this is 100% something
> that should be available at the SB API layer.
> >
> > Jim
> >
> >
> >
> > > On Sep 19, 2017, at 11:18 AM, Zachary Turner <zturner at google.com>
> wrote:
> > >
> > > Also, it avoids polluting the SB interface with another function that
> probably nobody is ever going to use outside of testing.  Adding to the SB
> API should take an act of God, given that once it gets added it has to stay
> until the end of time.
> > >
> > > On Tue, Sep 19, 2017 at 11:15 AM Zachary Turner <zturner at google.com>
> wrote:
> > > That would work, but I think it's adding many more pieces to the
> test.  Now there's threads, a Python layer, and multiprocess dotest
> infrastructure in the equation.  Each providing an additional source of
> flakiness and instability.
> > >
> > > If all it is is a pass-through, then just calling the function it
> passes through to is a strictly better test, since it's focusing the test
> on the underlying piece of functionality and removing additional sources of
> failure and instability from the test.
> > >
> > > On Tue, Sep 19, 2017 at 11:02 AM Jim Ingham <jingham at apple.com> wrote:
> > > I'd really prefer to do this as/along with an SB API test since we
> also need commands made through the SB API to be interruptible, and we
> should test that that also works.  So this kills two birds with one stone.
> > >
> > > In general, when developing any high-level features in lldb one of the
> questions you should ask yourself is whether this is useful at the SB API
> layer.  In this case it obviously is (actually I'm a little embarrassed I
> didn't think of this requirement).  If so, then it's simplest to test it at
> that layer.  If the SB API is anything more than a pass-through, then you
> might also want to write a unit test for the underlying C++ API's.  But in
> this case the SB function will just call the underlying CommandInterpreter
> one, so testing at the SB layer is sufficient.
> > >
> > > Jim
> > >
> > > > On Sep 19, 2017, at 10:45 AM, Zachary Turner via Phabricator <
> reviews at reviews.llvm.org> wrote:
> > > >
> > > > zturner added a comment.
> > > >
> > > > In https://reviews.llvm.org/D37923#875322, @clayborg wrote:
> > > >
> > > >> We should have a test. The test would need to use pexpect or
> something similar. If anyone has any pointer on how to make a test for
> this, please chime in. I was thinking just a pexpect based test.
> > > >
> > > >
> > > > This kind of thing is perfect for a unit test, but I'm not sure how
> easy that would be with the current design.  You'd basically do something
> like:
> > > >
> > > >  struct MockStream {
> > > >    explicit MockStream(CommandInterpreter &Interpreter, int
> InterruptAfter)
> > > >      : CommandInterpreter(Interpreter),
> InterruptAfter(InterruptAfter) {}
> > > >
> > > >    CommandInterpreter &Interpreter;
> > > >    const int InterruptAfter;
> > > >    int Lines = 0;
> > > >    std::string Buffer;
> > > >
> > > >    void Write(StringRef S) {
> > > >      ++Lines;
> > > >      if (Lines >= InterruptAfter) {
> > > >        Interpreter.Interrupt();
> > > >        return;
> > > >      }
> > > >      Buffer += S;
> > > >    }
> > > >  };
> > > >
> > > >  TEST_F(CommandInterruption) {
> > > >    CommandInterpreter Interpreter;
> > > >    MockStream Stream(Interpreter, 3);
> > > >    Interpreter.PrintCommandOutput(Stream, "a\nb\nc\nd\ne\nf\n");
> > > >    EXPECT_EQ(Stream.Lines == 3);
> > > >    EXPECT_EQ(Stream.Buffer == "a\nb\nc\n");
> > > >  }
> > > >
> > > >
> > > > https://reviews.llvm.org/D37923
> > > >
> > > >
> > > >
> > >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170919/ac445166/attachment-0001.html>