* guile --listen fix
@ 2012-09-19 16:51 Ian Price
2012-09-19 17:18 ` Mark H Weaver
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Ian Price @ 2012-09-19 16:51 UTC (permalink / raw)
To: guile-devel
[-- Attachment #1: Type: text/plain, Size: 787 bytes --]
Hi guys,
It was brought to my attention on #guile that the listen option no
longer works in guile.
$ guile --listen
ice-9/psyntax.scm:1201:48: In procedure syntax-type:
ice-9/psyntax.scm:1201:48: Syntax error:
unknown location: source expression failed to match any pattern in form (@@ (system repl server) (spawn-server))
I think the relevant commit is a March 8th commit by Mark Weaver
(8210c85), which restricts @@ to ids only. Since this change is
sensible, and in line with what we've documented, I fixed
command-line.scm.
If you are happy with it I'll push to stable-2.0. I also have a patch to
fix a typo.
--
Ian Price -- shift-reset.com
"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: --listen patch --]
[-- Type: text/x-patch, Size: 2181 bytes --]
From a764dcb9735c2478392006287a97dd07541e55ee Mon Sep 17 00:00:00 2001
From: Ian Price <ianprice90@googlemail.com>
Date: Wed, 19 Sep 2012 17:33:29 +0100
Subject: [PATCH 1/2] Fix @@ usage in --listen option
* module/ice-9/command-line.scm (compile-shell-switches): @@ no longer
supports arbitrary expressions, only identifiers.
---
module/ice-9/command-line.scm | 12 +++++-------
1 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/module/ice-9/command-line.scm b/module/ice-9/command-line.scm
index 62a2c9e..d409360 100644
--- a/module/ice-9/command-line.scm
+++ b/module/ice-9/command-line.scm
@@ -325,7 +325,7 @@ If FILE begins with `-' the -s switch is mandatory.
((string=? arg "--listen") ; start a repl server
(parse args
- (cons '(@@ (system repl server) (spawn-server)) out)))
+ (cons '((@@ (system repl server) spawn-server)) out)))
((string-prefix? "--listen=" arg) ; start a repl server
(parse
@@ -336,14 +336,12 @@ If FILE begins with `-' the -s switch is mandatory.
((string->number where) ; --listen=PORT
=> (lambda (port)
(if (and (integer? port) (exact? port) (>= port 0))
- `(@@ (system repl server)
- (spawn-server
- (make-tcp-server-socket #:port ,port)))
+ `((@@ (system repl server) spawn-server)
+ ((@@ (system repl server) make-tcp-server-socket) #:port ,port))
(error "invalid port for --listen"))))
((string-prefix? "/" where) ; --listen=/PATH/TO/SOCKET
- `(@@ (system repl server)
- (spawn-server
- (make-unix-domain-server-socket #:path ,where))))
+ `((@@ (system repl server) spawn-server)
+ ((@@ (system repl server) make-unix-domain-server-socket) #:path ,where)))
(else
(error "unknown argument to --listen"))))
out)))
--
1.7.7.6
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: typo patch --]
[-- Type: text/x-patch, Size: 954 bytes --]
From 9186a57acc30b99ddb93cac0168910222a30a366 Mon Sep 17 00:00:00 2001
From: Ian Price <ianprice90@googlemail.com>
Date: Wed, 19 Sep 2012 17:40:17 +0100
Subject: [PATCH 2/2] Fix typo in scheme-using.texi
* doc/ref/scheme-using.texi (System Commands): Fix typo.
---
doc/ref/scheme-using.texi | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/doc/ref/scheme-using.texi b/doc/ref/scheme-using.texi
index 3d43913..7eb84de 100644
--- a/doc/ref/scheme-using.texi
+++ b/doc/ref/scheme-using.texi
@@ -457,7 +457,7 @@ show a short error printout.
Default values for REPL options may be set using
@code{repl-default-option-set!} from @code{(system repl common)}:
-@deffn {Scheme Procedure} repl-set-default-option! key value
+@deffn {Scheme Procedure} repl-default-option-set! key value
Set the default value of a REPL option. This function is particularly
useful in a user's init file. @xref{Init File}.
@end deffn
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: guile --listen fix
2012-09-19 16:51 guile --listen fix Ian Price
@ 2012-09-19 17:18 ` Mark H Weaver
2012-09-19 19:59 ` Ian Price
2012-10-01 5:48 ` Mark H Weaver
2012-10-18 4:14 ` Mark H Weaver
2 siblings, 1 reply; 6+ messages in thread
From: Mark H Weaver @ 2012-09-19 17:18 UTC (permalink / raw)
To: Ian Price; +Cc: guile-devel
On 09/19/2012 12:51 PM, Ian Price wrote:
>
> Hi guys,
>
> It was brought to my attention on #guile that the listen option no
> longer works in guile.
Ouch. That's a serious regression in guile 2.0.6.
> I think the relevant commit is a March 8th commit by Mark Weaver
> (8210c85), which restricts @@ to ids only. Since this change is
> sensible, and in line with what we've documented, I fixed
> command-line.scm.
>
> If you are happy with it I'll push to stable-2.0. I also have a patch to
> fix a typo.
Your fixes look good to me. It would be good to add tests for the
listen option also.
Thanks!
Mark
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: guile --listen fix
2012-09-19 17:18 ` Mark H Weaver
@ 2012-09-19 19:59 ` Ian Price
2012-10-01 5:31 ` Mark H Weaver
0 siblings, 1 reply; 6+ messages in thread
From: Ian Price @ 2012-09-19 19:59 UTC (permalink / raw)
To: Mark H Weaver; +Cc: guile-devel
Mark H Weaver <mhw@netris.org> writes:
> Your fixes look good to me. It would be good to add tests for the
> listen option also.
(system repl server) does need tests, no question. But, I'm not sure how
I would test the command-line stuff. I suppose we could run a guile
instance with various arguments, have it output relevant variables, and
check the output. Hmm. This seems like it would be problematic with
--listen though, since we'd need to run two instances, and make some
visible changes to the instance. Maybe I just haven't thought it through
enough.
--
Ian Price -- shift-reset.com
"Programming is like pinball. The reward for doing it well is
the opportunity to do it again" - from "The Wizardy Compiled"
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: guile --listen fix
2012-09-19 19:59 ` Ian Price
@ 2012-10-01 5:31 ` Mark H Weaver
0 siblings, 0 replies; 6+ messages in thread
From: Mark H Weaver @ 2012-10-01 5:31 UTC (permalink / raw)
To: Ian Price; +Cc: guile-devel
On 09/19/2012 03:59 PM, Ian Price wrote:
> Mark H Weaver<mhw@netris.org> writes:
>
>> Your fixes look good to me. It would be good to add tests for the
>> listen option also.
>
> (system repl server) does need tests, no question. But, I'm not sure how
> I would test the command-line stuff. I suppose we could run a guile
> instance with various arguments, have it output relevant variables, and
> check the output. Hmm. This seems like it would be problematic with
> --listen though, since we'd need to run two instances, and make some
> visible changes to the instance. Maybe I just haven't thought it through
> enough.
I agree that it's not trivial to implement these tests. We shouldn't
let this get in the way of committing your fix, which is clearly
important. Since there have been no objections, I think you should go
ahead and commit it to the stable-2.0 branch.
Mark
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: guile --listen fix
2012-09-19 16:51 guile --listen fix Ian Price
2012-09-19 17:18 ` Mark H Weaver
@ 2012-10-01 5:48 ` Mark H Weaver
2012-10-18 4:14 ` Mark H Weaver
2 siblings, 0 replies; 6+ messages in thread
From: Mark H Weaver @ 2012-10-01 5:48 UTC (permalink / raw)
To: Ian Price; +Cc: guile-devel
Hi Ian,
Sorry for sending two emails in quick succession, but I just realized
that there is one minor problem with your proposed commit which should
ideally be fixed: the commit log message.
> * module/ice-9/command-line.scm (compile-shell-switches): @@ no longer
> supports arbitrary expressions, only identifiers.
The commit log should briefly describe the changes made within the
associated patch. The rationale for the changes should be secondary,
and arguably do not belong in the commit log at all.
In this case, your commit log does not describe your changes at all. It
contains only the rationale.
The result is quite misleading, because it suggests that your commit
changed the way '@@' behaves. It would perhaps have been a suitable
commit log for my earlier commit (8210c85), but it is quite
inappropriate for your commit.
Mark
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: guile --listen fix
2012-09-19 16:51 guile --listen fix Ian Price
2012-09-19 17:18 ` Mark H Weaver
2012-10-01 5:48 ` Mark H Weaver
@ 2012-10-18 4:14 ` Mark H Weaver
2 siblings, 0 replies; 6+ messages in thread
From: Mark H Weaver @ 2012-10-18 4:14 UTC (permalink / raw)
To: Ian Price; +Cc: guile-devel
Ian Price <ianprice90@googlemail.com> writes:
> It was brought to my attention on #guile that the listen option no
> longer works in guile.
>
> $ guile --listen
> ice-9/psyntax.scm:1201:48: In procedure syntax-type:
> ice-9/psyntax.scm:1201:48: Syntax error:
> unknown location: source expression failed to match any pattern in form (@@ (system repl server) (spawn-server))
>
> I think the relevant commit is a March 8th commit by Mark Weaver
> (8210c85), which restricts @@ to ids only. Since this change is
> sensible, and in line with what we've documented, I fixed
> command-line.scm.
>
> If you are happy with it I'll push to stable-2.0. I also have a patch to
> fix a typo.
I pushed these patches to stable-2.0.
Thanks!
Mark
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-10-18 4:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-19 16:51 guile --listen fix Ian Price
2012-09-19 17:18 ` Mark H Weaver
2012-09-19 19:59 ` Ian Price
2012-10-01 5:31 ` Mark H Weaver
2012-10-01 5:48 ` Mark H Weaver
2012-10-18 4:14 ` Mark H Weaver
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).