From: Luca Vizzarro <Luca.Vizzarro@arm.com>
To: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
Cc: dev@dpdk.org, Jeremy Spewock <jspewock@iol.unh.edu>,
Paul Szczepanek <paul.szczepanek@arm.com>,
Jack Bond-Preston <jack.bond-preston@arm.com>
Subject: Re: [PATCH 1/5] dts: fix InteractiveShell command prompt filtering
Date: Tue, 16 Apr 2024 13:12:02 +0100 [thread overview]
Message-ID: <e39b5bad-dcf7-424b-a74a-742e15235d19@arm.com> (raw)
In-Reply-To: <CAOb5WZZYbO2jEQJ-GMJQ3NbKvzMy0Ob=knMML2qkzOcDegvu4g@mail.gmail.com>
Thank you for your review Juraj!
On 16/04/2024 09:40, Juraj Linkeš wrote:
>> When sending a command using an instance of InteractiveShell the output
>> is meant to filter out the leading shell prompt. The filtering logic is
>> present but the line is appended anyways.
>>
>
> I don't think that's what's happening here. The output collecting
> logic is "stop when we encounter a prompt, but not the prompt with the
> command we sent". We could change the comment though.
Yeah, I actually identified the behaviour better after writing this.
Will update the commit body accordingly. And I mixed-up leading with
trailing! This is meant to say "trailing shell prompt".
>> @@ -132,11 +132,11 @@ def send_command(self, command: str, prompt: str | None = None) -> str:
>> self._stdin.flush()
>> out: str = ""
>> for line in self._stdout:
>> - out += line
>> if prompt in line and not line.rstrip().endswith(
>> command.rstrip()
>> ): # ignore line that sent command
>> break
>> + out += line
>
> If we do this, we'll only filter out the last prompt, which may not be
> desirable, since the last prompt is there only because all of our
> interactive shells force an extra prompt with _command_extra_chars.
Could you please expand more on this?
> One thing we could improve though is removing the distribution welcome
> message from logs, or at least separate it from the first command sent
> with the interactive shell. The second option will allow us to see
> clearly that an interactive session has been established, although we
> could just emit a shorter log (something like "Started a testpmd
> session" and then flush the welcome screen output).
I am not sure what you are referring to exactly, could you also expand
more on this please?
Given it's not particularly explained, I thought having two command
prompts (especially a trailing one) was an error. The main reason behind
this is that when we go to parse the port info, the last entry which is
"device private info" appears to be open ended, and I couldn't gather
much information from the testpmd source code. So I opted to parse
everything until the end. With a trailing command prompt this meant
that: device_private_info="....testpmd> ".
next prev parent reply other threads:[~2024-04-16 12:12 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 [this message]
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
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=e39b5bad-dcf7-424b-a74a-742e15235d19@arm.com \
--to=luca.vizzarro@arm.com \
--cc=dev@dpdk.org \
--cc=jack.bond-preston@arm.com \
--cc=jspewock@iol.unh.edu \
--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).