guile-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: guile pipeline do-over


From: Linus Björnstam
Subject: Re: guile pipeline do-over
Date: Tue, 10 Mar 2020 09:54:25 +0100
User-agent: Cyrus-JMAP/3.1.7-991-g5a577d3-fmstable-20200305v3

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]