On Sun, Apr 24, 2011 at 04:33:44PM +0200, Andy Wingo wrote:
> On Sat 23 Apr 2011 21:46, Mark Harig <idirectscm@aim.com> writes:
>
> > Here is a set of patches to add the new section "Invoking Guile" to
> > the chapter "Programming in Scheme."
>
> They look good in general, though I have some comments. I would like
> Neil to look over them as well, or at least say he's OK with them.
>
> First, your patches should be "atomic"; see
> http://www.kernel.org/pub/software/scm/git/docs/user-manual.html#patch-series
> for some commentary. In particular see point 3. The manual should be
> working after each patch.
From that web page:
"3. No patch introduces a regression: after applying any
initial part of the series, the resulting project still
compiles and works, and has no bugs that it didn’t have
before."
I have generated both a guile.info and a guile.dvi file for
these changes and confirmed that there are no errors that
prevent these files from being generated. Here are the versions of
the tools that I used:
$ makeinfo --version
makeinfo (GNU texinfo) 4.13
$ texi2dvi --version
texi2dvi (GNU Texinfo 4.13) 1.135
Patches 0004 and 0005 from my original message are atomic and may
*each* be reviewed, and rejected or committed independent of the other
patches. They corrected errors in other parts of the manual that I
found while reviewing the changes I was making.
>
> The commit logs are good.
>
> > +@item -s @var{script} @var{arg...}
> > +@cindex script mode
> > +Read and evaluate Scheme source code from the file @var{script}, as the
> > +@code{load} function would. After loading @var{script}, exit. Any
> > +command-line arguments @var{arg...} following @var{script} become the
> > +script's arguments; the @code{command-line} function returns a list of
> > +strings of the form @code{(@var{script} @var{arg...})}.
>
> The "-s" is actually optional, and only *needed* if your script starts
> with a dash. So please document "guile foo.scm" as the default, and
> just mention "-s" in case of the script starting with a dash, or if you
> are writing some other shell script.
OK. The description of "-s" was a copy of the original. I
have written an updated description based on your comments.
Please review the new description.
> Use @itemx in this case, I think.
After reading the description of @itemx in the Texinfo
manual, I have left this unchanged. @itemx is to be used
when there are more than a single table item for a given
description. If you still think that this is needed, then I
need additional clarification.
>
> > +strings, even those that are written as numerals. (Note that here we
> > +are referring to names and values that are defined in the operating
> > +system shell from which Guile is invoked. This is not the same as a
> > +Scheme environment that is defined within a running instance of guile.
>
> "Guile", I think.
Corrected.
>
> > +How to set environment variables before starting Guile depends on the
> > +operating system, and especially the shell that you are using. For
> > +example, here's how to set the environment variable @env{ORGANIZATION}
> > +to @samp{not very much} using Bash:
> > +
> > +@example
> > +export ORGANIZATION="not very much"
> > +@end example
> > +
> > +@noindent
> > +and here's how to do it in csh or tcsh:
> > +
> > +@example
> > +setenv ORGANIZATION "not very much"
> > +@end example
>
> Perhaps the tcsh example is superfluous. Perhaps we should also mention
> the way to set env vars for one invocation: "GUILE_AUTO_COMPILE=0
> guile".
>
Deleted the tcsh example, added a Bash example, and updated
the descriptive text. Please review.
> > +If you wish to retrieve or change the value of the shell environment
> > +variables that effect the run-time behavior of Guile from within a
> > +running instance of guile, see @xref{Runtime Environment}.
>
> "affect"
>
Corrected.
> > +@item GUILE_HUSH
> > +@vindex GUILE_HUSH
> > +The @code{#/} notation for lists provokes a warning message from Guile.
> > +This syntax will be removed from Guile in the near future.
> > +
> > +To disable the warning message, set the @env{GUILE_HUSH} environment
> > +variable to any non-empty value.
>
> This variable does not exist.
>
Deleted.
> > +@item GUILE_INIT_MALLOC_LIMIT
> > +@vindex GUILE_INIT_MALLOC_LIMIT
> > +@item GUILE_MIN_YIELD_MALLOC
> > +@vindex GUILE_MIN_YIELD_MALLOC
> > +@cindex garbage collecting
> > +@item GUILE_MAX_SEGMENT_SIZE
> > +@vindex GUILE_MAX_SEGMENT_SIZE
> > +@item GUILE_INIT_SEGMENT_SIZE_2
> > +@vindex GUILE_INIT_SEGMENT_SIZE_2
> > +@item GUILE_INIT_SEGMENT_SIZE_1
> > +@vindex GUILE_INIT_SEGMENT_SIZE_1
> > +@item GUILE_MIN_YIELD_2
> > +@vindex GUILE_MIN_YIELD_2
>
> Since switching to the BDW GC, these variables are not used any more.
>
> GC_FREE_SPACE_DIVISOR is parsed by Guile though. The GC itself does
> appear to do a number of getenv calls, but we can't really document
> those in Guile I don't think.
>
Deleted.
> > +@item GUILE_SYSTEM_LOAD_COMPILED_PATH
> > +@item GUILE_SYSTEM_PATH
> > +@item GUILE_SYSTEM_LTDL_PATH
>
> These are paths that Guile itself uses to look up .go, .scm, and .so
> files, respectively. They have default values, relative to $prefix, but
> can be overridden if you really know what you're doing. This is only
> really useful when building Guile itself.
>
Deleted. (From what you have written, it sounds as though someone
knowledgeable about the Guile construction and installation process
should document these elsewhere.)
> > +@item GUILE_INIT_SEGMENT_SIZE_1
> > +@item GUILE_MIN_YIELD_1
> > +@item GUILE_INIT_SEGMENT_SIZE_2
> > +@item GUILE_MIN_YIELD_2
> > +@item GUILE_MAX_SEGMENT_SIZE
> > +@item GUILE_INIT_HEAP_SIZE_1
> > +@item GUILE_INIT_HEAP_SIZE_2
>
> Not used any more.
>
Deleted.
> > +@item GUILE_WARN_DEPRECATED
> > +@vindex GUILE_WARN_DEPRECATED
> > +Please provide a description of me.
>
> @ref to Deprecation.
>
I wrote a description based on my understanding from the
'Deprecation' node. Please review.
>
> Otherwise, looking very good. Thank you for this work, and looking
> forward to the revised patchset :-)
>
Please find the three revised patches attached. After
checking in the changes, I generated the new patches with:
git format-patch origin
0001-doc-ref-guile.texi-node-Programming-in-Scheme-Added-.patch
0002-doc-ref-scheme-scripts.texi-node-Guile-Scripting-Del.patch
0003-doc-ref-guile-invoke.texi-node-Invoking-Guile-Initia.patch
Please also find attached an updated version of the plain
text generated for the new node.
--