unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
From: "Linus Björnstam" <linus.bjornstam@veryfast.biz>
To: "Rutger van Beusekom" <rutger.van.beusekom@verum.com>,
	"Ludovic Courtès" <ludo@gnu.org>
Cc: Guile Devel <guile-devel@gnu.org>
Subject: Re: guile pipeline do-over
Date: Tue, 10 Mar 2020 09:54:25 +0100	[thread overview]
Message-ID: <26cad467-5204-4f2c-80c2-d8032887aa7a@www.fastmail.com> (raw)
In-Reply-To: <877dzsy84v.fsf@verum.com>

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	other threads:[~2020-03-10  8:54 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-06  9:52 guile pipeline do-over Rutger van Beusekom
2020-03-07 15:46 ` Ludovic Courtès
2020-03-10  7:35   ` Rutger van Beusekom
2020-03-10  8:54     ` Linus Björnstam [this message]
2020-03-10 10:37       ` Rutger van Beusekom
2020-03-26  9:09     ` Ludovic Courtès
2020-04-04  8:01       ` Rutger van Beusekom
2020-05-16 20:38         ` Ludovic Courtès

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

  List information: https://www.gnu.org/software/guile/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=26cad467-5204-4f2c-80c2-d8032887aa7a@www.fastmail.com \
    --to=linus.bjornstam@veryfast.biz \
    --cc=guile-devel@gnu.org \
    --cc=ludo@gnu.org \
    --cc=rutger.van.beusekom@verum.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 read-only IMAP folder(s) and NNTP newsgroup(s).