From: Jeremy Spewock <jspewock@iol.unh.edu>
To: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
Cc: Luca Vizzarro <Luca.Vizzarro@arm.com>,
dev@dpdk.org, Paul Szczepanek <paul.szczepanek@arm.com>,
Jack Bond-Preston <jack.bond-preston@arm.com>
Subject: Re: [PATCH 2/5] dts: skip first line of send_command output
Date: Mon, 29 Apr 2024 11:18:06 -0400 [thread overview]
Message-ID: <CAAA20UQpmhUKQ1u3o0se6mPeEEDbe6SgLwCriKyJcxkMrsZOhg@mail.gmail.com> (raw)
In-Reply-To: <CAOb5WZa3MxFGpeJPOAioWRCCTcr2tcRb0pfyP0LTurQOwe-yPg@mail.gmail.com>
Apologies for the complications that this interactive shell provides
here. These problems didn't arise previously primarily because the
interactive shells were designed to receive commands, give you the raw
output, and then the developer extract specifically what they want
from the output and ignore the things they don't. I understand however
that in your case it might be beneficial to just consume everything.
Some of these changes seem universally good and overall not harmful to
include (like removing the trailing prompt, I can't really see a use
for it) but some others come with caveats, so if it is too complicated
this might be better to handle as something testpmd specific, and
leave the generic interactive shell to always just give you raw
output.
On Wed, Apr 17, 2024 at 9:18 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
<snip>
> > >> @@ -132,6 +135,9 @@ def send_command(self, command: str, prompt: str | None = None) -> str:
> > >> self._stdin.flush()
> > >> out: str = ""
> > >> for line in self._stdout:
> > >> + if skip_first_line:
> > >> + skip_first_line = False
> > >> + continue
> > >
> > > Is there ever a reason to distinguish between the first line and the
> > > line with the command on it?
> >
> > As above, not really sure. Would this always be a command prompt? The
Whether this first line is always the command prompt or not is
specific to the shell unfortunately. In "bash-like" shells where
commands you send are echoed into stdout for easy-of-use for the
developer (like testpmd), this first line will always be the command
you sent to it. It technically isn't always required for this to
happen however, so we could make this assumption, but it could be
slightly more limiting down the line.
> > doubt arises only because I don't understand why we'd need the command
> > prompt fed back.
> >
>
> The only thing I could think of is debugging. Maybe it could offer
> some extra insight in some corner cases.
I agree that it is useful for debugging, but we do also log it
separately before sending the command. I don't think the command could
change by simply being sent into the shell unless something strange
happens like the shell breaks it across multiple lines. I think it
would be fine to exclude it, but as mentioned, it isn't always safe to
do so.
>
> > >
> > >> if prompt in line and not line.rstrip().endswith(
> > >> command.rstrip()
> > >> ): # ignore line that sent command
> > >> --
> > >> 2.34.1
> > >>
> >
next prev parent reply other threads:[~2024-04-29 15:18 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-12 11:11 [PATCH 0/5] dts: testpmd show port info/stats Luca Vizzarro
2024-04-12 11:11 ` [PATCH 1/5] dts: fix InteractiveShell command prompt filtering Luca Vizzarro
2024-04-16 8:40 ` Juraj Linkeš
2024-04-16 12:12 ` Luca Vizzarro
2024-04-17 13:06 ` Juraj Linkeš
2024-04-17 14:17 ` Luca Vizzarro
2024-04-18 6:31 ` Juraj Linkeš
2024-04-29 16:16 ` Jeremy Spewock
2024-04-12 11:11 ` [PATCH 2/5] dts: skip first line of send_command output Luca Vizzarro
2024-04-16 8:48 ` Juraj Linkeš
2024-04-16 12:15 ` Luca Vizzarro
2024-04-17 13:18 ` Juraj Linkeš
2024-04-29 15:18 ` Jeremy Spewock [this message]
2024-04-12 11:11 ` [PATCH 3/5] dts: add parsing utility module Luca Vizzarro
2024-04-16 8:59 ` Juraj Linkeš
2024-04-16 12:16 ` Luca Vizzarro
2024-04-29 16:15 ` Jeremy Spewock
2024-04-30 10:49 ` Luca Vizzarro
2024-04-30 20:03 ` Jeremy Spewock
2024-04-12 11:11 ` [PATCH 4/5] dts: add `show port info` command to TestPmdShell Luca Vizzarro
2024-04-16 9:03 ` Juraj Linkeš
2024-04-16 12:24 ` Luca Vizzarro
2024-04-17 13:22 ` Juraj Linkeš
2024-04-17 14:25 ` Luca Vizzarro
2024-04-17 15:29 ` Luca Vizzarro
2024-04-18 6:41 ` Juraj Linkeš
2024-04-18 10:52 ` Luca Vizzarro
2024-04-12 11:11 ` [PATCH 5/5] dts: add `show port stats` " Luca Vizzarro
2024-04-16 9:04 ` Juraj Linkeš
2024-04-29 15:54 ` Jeremy Spewock
2024-04-30 10:51 ` Luca Vizzarro
2024-05-09 11:26 ` [PATCH v2 0/5] dts: testpmd show port info/stats Luca Vizzarro
2024-05-09 11:26 ` [PATCH v2 1/5] dts: fix InteractiveShell command prompt filtering Luca Vizzarro
2024-05-09 11:26 ` [PATCH v2 2/5] dts: skip first line of send command output Luca Vizzarro
2024-05-09 11:26 ` [PATCH v2 3/5] dts: add parsing utility module Luca Vizzarro
2024-05-09 11:26 ` [PATCH v2 4/5] dts: add `show port info` command to TestPmdShell Luca Vizzarro
2024-05-09 11:26 ` [PATCH v2 5/5] dts: add `show port stats` " Luca Vizzarro
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAAA20UQpmhUKQ1u3o0se6mPeEEDbe6SgLwCriKyJcxkMrsZOhg@mail.gmail.com \
--to=jspewock@iol.unh.edu \
--cc=Luca.Vizzarro@arm.com \
--cc=dev@dpdk.org \
--cc=jack.bond-preston@arm.com \
--cc=juraj.linkes@pantheon.tech \
--cc=paul.szczepanek@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).