I didn’t hear back from anybody, so let me elaborate this change a bit.
It is clear that current behaviour is broken, as some CLI handlers which work
well in interactive mode simply eat rest of the content of exec file.
As result of that I can see people submitting patches to fix CLI handlers they
are interested in only, which i believe is wrong. This needs to be fixed inside
infra.
Patch I submitted doesn’t change behaviour of CLIs which are written in single
line, but requires change in multiline CLIs which uses data wrapped with {}. I
am aware of 2 commands which do that:
- ‘comment' with multiline comments
- ‘packet-generator new’
Funny thing about ‘packet-generator new’ is that typically needs to be followed
by “packet-generator enable” which also eats rest of the input which makes
usage of packet generator from exec script limited and constrained. i.e. when i
use packet generator typically i want to do some show commands after and that
is not possible today.
I know very few people who use packet-genrator from exec script, they are
developers and I’m quite sure they should be able to address this issue quickly.
I fully disagree that we wait for next release with this change for following
reasons:
- we don’t guarantee CLI consistency like we do for APIs
- If this is right thing to do (and I didn’t hear anybody disagreeing), it
needs to happen sooner or later, so it is better to have this issue fixed
sooner that later
- reason for not merging patch late in development cycles is high risk of bugs
and that is not the case here. This patch is low risk from bugs perspective, it
just changes behaviour for very limited number of CLIs, and there is more than
enough time to document that before release is out.
—
Damjan
> On 09.05.2022., at 13:56, Damjan Marion via lists.fd.io
> <dmarion=me....@lists.fd.io> wrote:
>
>
> Hmm, not sure I understand concern about blast radius. I also replied to your
> comment in gerrit.
>
> —
> Damjan
>
>
>
>> On 09.05.2022., at 07:31, Andrew 👽 Yourtchenko <ayour...@gmail.com> wrote:
>>
>> Damjan,
>>
>> I have left the comment on the change itself - in short, given its blast
>> radius, it needs to wait at least until 22.06 RC1 is done.
>>
>> --a
>>
>>> On 8 May 2022, at 19:39, Damjan Marion via lists.fd.io
>>> <dmarion=me....@lists.fd.io> wrote:
>>>
>>> Guys,
>>>
>>> I just submitted following patch which fixes long standing issue in how cli
>>> scripts are executed.
>>>
>>> https://gerrit.fd.io/r/c/vpp/+/36101
>>>
>>> Problem was that there was no way to execute CLIs which have optional
>>> arguments. I.e. “show version” and “show version verbose”.
>>> CLI parser was passing whole contents up to the EOF to each CLI handler,
>>> and because he eats all whitespaces there was no way to know if current
>>> unformat input points to the rest of the line or to the beginning of the
>>> new line.
>>>
>>> In this patch i changed that behaviour so CLI gets only one line of input.
>>>
>>> Also I changed unformat_input function so it recognises backslash before
>>> newline as way to pass multiline data to cli handler.
>>>
>>> As a result, there is no need for calling unformat_line in each cli
>>> handler, and still there is a way to specify multiline CLIs.
>>>
>>> i.e.
>>>
>>> show version \
>>> verbose
>>>
>>> or:
>>>
>>> packet-generator new { \
>>> name x \
>>> limit 5 \
>>> size 128-128 \
>>> interface local0 \
>>> node null-node \
>>> data { \
>>> incrementing 30 \
>>> } \
>>> }
>>>
>>> Hope nobody have issues with this change, but let me know if I’m wrong…
>>>
>>> —
>>> Damjan
>>>
>>>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#21397): https://lists.fd.io/g/vpp-dev/message/21397
Mute This Topic: https://lists.fd.io/mt/90974441/21656
Group Owner: vpp-dev+ow...@lists.fd.io
Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
