I have a question about the interface. It uses the shell now, it seems. (I
could be wrong). The guile system call has a (system cmd ) which uses the shell
and a system* call which takes (system* cmd arg ...) So that it does not rely
on the shell. Maybe a similar interface could be useful (and more secure) for
the pipeline as well.
Thank you for this patch.
Linus Björnstam
On Tue, 10 Mar 2020, at 08:35, Rutger van Beusekom wrote:
>
> Hi Ludo,
>
> I have processed your feedback in this version of the patch.
>
> Ludovic Courtès writes:
>
> > Hi Rutger!
> >
> >> ...
> > Nice! That’s definitely very useful to have. We’ll need to check what
> > Andy thinks, but I think it can be added in the 3.0 series.
> >
> >
> >> ...
> > Could you mention functions renamed/removed here? The ChangeLog format
> > is about boringly listing all the language-entity-level changes. :-)
> >
> Done.
> >
> >> ...
> > I guess you can remove the commented-out bits…
> >
> Yep.
> >
> >> ...
> > … and this hunk, to minimize change.
> >
> Check.
> >
> >> ...
> > I would not export ‘pipe->fdes’. I’m not sure about exporting
> > ‘piped-process’: it’s a bit low-level and we might want to reserve
> > ourselves the possibility to change it, like this patch does actually.
> >
> > WDYT?
> >
> I agree.
> >> ...
> >
> > Please wrap lines to 80 chars.
> >
> Taken care of.
> >
> >> ...
> >
> > I suggest using ‘string=?’ above instead of ‘equal?’. Also, could you
> > add a docstring?
> >
> Yes and yes.
> >
> >> ...
> >
> > Perhaps s/procs/commands/ would be clearer? Also, @var{commands}
> > instead of @code.
> >
> Yep.
> >
> > Could you also add an entry in doc/ref/*.texi, in the “Pipes” node,
> > perhaps with one of the examples you gave?
> >
> Wrote a new example. WDYT?
> >
> >> ...
> >
> > Please move these to the top-level ‘define-module’ form.
> >
> Done.
> >
> > One last thing: we’d need you to assign copyright to the FSF for this.
> > We can discuss it off-line if you want.
> >
> Can you help me there? I already have a verbal commitment from the
> company, we just need to formalize it.
> >
> > Thank you for this great and long overdue addition!
> >
> Happy to add it.
> >
> > Ludo’.
> >
> Rutger
>
>
> Attachments:
> * 0001-Add-pipeline-procedure.patch
