Re: PATCH: script: Introduced a streaming output command

[Rick van Rein <rick@openfortress.nl>]

Hello Karel,

Thanks for the evaluation.  I will follow your hints on all points, some
are subjective but these are your prerogative.  New patch will come in a
few days.

> Nice idea ;-)

Thanks.  I didn't want to mention a specific online service in the
"real" documentation, but the idea was inspired by "nc seashells.io 1337".

> Do we really need the default command? ;-) 

No, it's merely a luxoury.  One might say that something _around_ script
is another way of doing this.  I will remove it.

> I think the example with nc(1) in the man page is good enough. We do
> not need hardcode any path/name in the code.

OK.  This a matter of taste, I will follow your directions.

> It would be better to use one bit for this boolean,
> 
>   unsigned int
>     stream :1

Yes I agree.  I hadn't noticed that :1 was used on other flags (they're
concentrated below, I see now).  Will change move it into that habit.

>     if (ctl->stream)
>         wait_for_stream_cmd(ctl, 0);
> 
> I guess it would be more robust, than call wait_for_stream_cmd()
> in all situations.


OK, will do.

>> +	int cmdio[2];
> 
> It's detail, but stream_pipe[] wold be more readable :-)

Sure, will do.

>> +			fprintf(stderr,_("streaming command failed: %s\r\n"),strerror(errno));
>> +			exit(1);
> 
>     err(1, _("streaming command failed"));
> 
> err() seems better than fprintf() + strerror() + exit();

I missed that option.  Will do.

> Frankly, all the command-line semantic seems complicated and a little
> bit like over-engineering.

I am trying to make it simple to use, with a minimum of entry (and
manpage reading).  It's a matter of taste, and falls in the same line as
moving the default script from inner command to wrapper around script.
So I'm not going to debate this, and follow your taste.  It's your
software, after all :)


I will post a new patch in a while (and this time I'll be more accurate
about the subject and line breaks while logically occurred in the email
body).