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
next prev parent 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).