On Thu, Jul 14, 2016 at 12:01:10PM +0200, Andy Wingo wrote:
> On Wed 13 Jul 2016 06:08, Tobin Harding <m...@tobin.cc> writes:
>
> > While working on this I discovered that compile messages are output from two
> > separate places (load.c and boot-9.scm). Each file contains identical
> > strings
> > for the messages. This goes against the rule of SPOT.
>
> Sadly it has to be like this for now. I do not have suggestions to fix
> it; it's gnarly.
Ok, Will forget about this. I would love to know why though, solely out of
interest.
>
> > - scm_puts (";;; note: source file ", scm_current_warning_port ());
> > - scm_display (full_filename, scm_current_warning_port ());
> > - scm_puts ("\n;;; newer than compiled ",
> > scm_current_warning_port ());
> > - scm_display (compiled_filename, scm_current_warning_port ());
> > - scm_puts ("\n", scm_current_warning_port ());
> > + if (scm_is_false (*scm_loc_quiet)) {
> > + scm_puts (";;; note: source file ", scm_current_warning_port ());
> > + scm_display (full_filename, scm_current_warning_port ());
> > + scm_puts ("\n;;; newer than compiled ", scm_current_warning_port
> > ());
> > + scm_display (compiled_filename, scm_current_warning_port ());
> > + scm_puts ("\n", scm_current_warning_port ());
> > + }
> > }
>
> Please resend without tabs, please. Thanks :)
I'm not sure about this one. I manually put spaces in front of each line as
directed but it does not show up as a change in 'git diff'. Is the rest of the
patch correct in regards to Guile project tab/space guidelines. (I couldn't find
any such guideline, is there some specific Emacs setup I should be using for
Guile development?)
>
> >
> >
> > +;;; Don't know where to put these:
> > +(define (cond_warn_compiling file)
> > + (unless %quiet
> > + (format (current-warning-port) ";;; compiling ~a\n" file)))
> > +
> > +(define (cond_warn_compiled file)
> > + (unless %quiet
> > + (format (current-warning-port) ";;; compiled ~a\n" file)))
> > +
> > +(define (cond_warn_newer new old)
> > + (unless %quiet
> > + (format (current-warning-port)
> > + ";;; note: source file ~a\n;;; newer than compiled ~a\n"
> > + name go-file-name)))
>
> These definitions are inappropriate for being at the top-level of
> boot-9.scm -- they would become automatically a part of all
> environments. I would leave out this change.
Fixed. I have put these definitions at the top of the procedure that calls them,
does that fit in with the rest of the code ok?
> Note also that the Guile convention is hyphen-case not snake_case.
My bad, amateur mistake.
>
> > diff --git a/module/ice-9/command-line.scm b/module/ice-9/command-line.scm
> > index 98d3855..9a3f7e1 100644
> > --- a/module/ice-9/command-line.scm
> > +++ b/module/ice-9/command-line.scm
> > @@ -136,6 +136,7 @@ If FILE begins with `-' the -s switch is mandatory.
> > --listen[=P] listen on a local port or a path for REPL clients;
> > if P is not given, the default is local port 37146
> > -q inhibit loading of user init file
> > + --quiet inhibit compile and load messages
> > --use-srfi=LS load SRFI modules for the SRFIs in LS,
> > which is a list of numbers like \"2,13,14\"
> > -h, --help display this help and exit
>
> What do people think about the name of this command line option?
>
> To me "inhibit compile and load messages" does not quite capture what it
> does. But maybe it is good enough.
I've changed it to;
--quiet inhibit informational compiler output, e.g name of
files being compiled
Open to suggestions still. Unlike most programmers I find documenting my work
harder
than writing the code :)
> Incidentally this change will need a documentation update as well. I
> often find that adding documentation makes it more clear how things
> should be named -- that the process of explaining things makes it
> apparent what things should be changed :)
>
> > + ((string=? arg "--quiet")
> > + (set! %quiet #t)
> > + (parse args out))
> > +
>
> Probably %quiet should be a parameter, and unless we plan on using it
> for other purposes it should have a more specific name
> (%verbose-auto-compilation-messages or so; no idea).
Could you clarify please, a parameter to what?
>
> Or, you could always just (current-warning-port (%make-void-port "w"));
> then the patch is super simple, no conditions needed. That is what I
> was originally thinking, but it has the disadvantage that you lose other
> uses of the current warning port, but maybe that's an advantage too.
Won't we loose compiler warnings then? I did not want to inhibit those.
thanks,
Tobin.
reworked patch follows :)
---
libguile/load.c | 34 ++++++++++++++++++++++------------
module/ice-9/boot-9.scm | 22 +++++++++++++++++-----
module/ice-9/command-line.scm | 6 ++++++
3 files changed, 45 insertions(+), 17 deletions(-)
diff --git a/libguile/load.c b/libguile/load.c
index 7ad9a75..e2d6c4a 100644
--- a/libguile/load.c
+++ b/libguile/load.c
@@ -226,6 +226,9 @@ static SCM *scm_loc_fresh_auto_compile;
/* The fallback path for auto-compilation */
static SCM *scm_loc_compile_fallback_path;
+/* Whether we should output compiler informational messages */
+static SCM *scm_loc_verbose_compiler_messages;
+
/* Ellipsis: "..." */
static SCM scm_ellipsis;
@@ -561,11 +564,13 @@ compiled_is_fresh (SCM full_filename, SCM
compiled_filename,
else
{
compiled_is_newer = 0;
- scm_puts (";;; note: source file ", scm_current_warning_port ());
- scm_display (full_filename, scm_current_warning_port ());
- scm_puts ("\n;;; newer than compiled ", scm_current_warning_port
());
- scm_display (compiled_filename, scm_current_warning_port ());
- scm_puts ("\n", scm_current_warning_port ());
+ if (scm_is_true (*scm_loc_verbose_compiler_messages)) {
+ scm_puts (";;; note: source file ", scm_current_warning_port ());
+ scm_display (full_filename, scm_current_warning_port ());
+ scm_puts ("\n;;; newer than compiled ", scm_current_warning_port
());
+ scm_display (compiled_filename, scm_current_warning_port ());
+ scm_puts ("\n", scm_current_warning_port ());
+ }
}
return compiled_is_newer;
@@ -1006,10 +1011,11 @@ do_try_auto_compile (void *data)
{
SCM source = SCM_PACK_POINTER (data);
SCM comp_mod, compile_file;
-
- scm_puts (";;; compiling ", scm_current_warning_port ());
- scm_display (source, scm_current_warning_port ());
- scm_newline (scm_current_warning_port ());
+ if (scm_is_true (*scm_loc_verbose_compiler_messages)) {
+ scm_puts (";;; compiling ", scm_current_warning_port ());
+ scm_display (source, scm_current_warning_port ());
+ scm_newline (scm_current_warning_port ());
+ }
comp_mod = scm_c_resolve_module ("system base compile");
compile_file = scm_module_variable (comp_mod, sym_compile_file);
@@ -1036,9 +1042,11 @@ do_try_auto_compile (void *data)
/* Assume `*current-warning-prefix*' has an appropriate value. */
res = scm_call_n (scm_variable_ref (compile_file), args, 5);
- scm_puts (";;; compiled ", scm_current_warning_port ());
- scm_display (res, scm_current_warning_port ());
- scm_newline (scm_current_warning_port ());
+ if (scm_is_true (*scm_loc_verbose_compiler_messages)) {
+ scm_puts (";;; compiled ", scm_current_warning_port ());
+ scm_display (res, scm_current_warning_port ());
+ scm_newline (scm_current_warning_port ());
+ }
return res;
}
else
@@ -1349,6 +1357,8 @@ scm_init_load ()
= SCM_VARIABLE_LOC (scm_c_define ("%load-should-auto-compile",
SCM_BOOL_F));
scm_loc_fresh_auto_compile
= SCM_VARIABLE_LOC (scm_c_define ("%fresh-auto-compile", SCM_BOOL_F));
+ scm_loc_verbose_compiler_messages
+ = SCM_VARIABLE_LOC (scm_c_define ("%verbose-compiler-messages",
SCM_BOOL_T));
scm_ellipsis = scm_from_latin1_string ("...");
diff --git a/module/ice-9/boot-9.scm b/module/ice-9/boot-9.scm
index 99543e7..018ca6a 100644
--- a/module/ice-9/boot-9.scm
+++ b/module/ice-9/boot-9.scm
@@ -3762,6 +3762,20 @@ when none is available, reading FILE-NAME with READER."
;; Return GO-FILE-NAME after making sure that it contains a freshly
;; compiled version of source file NAME with stat SCMSTAT; return #f
;; on failure.
+ (define (cond-warn-compiling file)
+ (if %verbose-compiler-messages
+ (format (current-warning-port) ";;; compiling ~a\n" file)))
+
+ (define (cond-warn-compiled file)
+ (if %verbose-compiler-messages
+ (format (current-warning-port) ";;; compiled ~a\n" file)))
+
+ (define (cond-warn-newer new old)
+ (if %verbose-compiler-messages
+ (format (current-warning-port)
+ ";;; note: source file ~a\n;;; newer than compiled ~a\n"
+ name go-file-name)))
+
(false-if-exception
(let ((gostat (and (not %fresh-auto-compile)
(stat go-file-name #f))))
@@ -3769,15 +3783,13 @@ when none is available, reading FILE-NAME with READER."
(load-thunk-from-file go-file-name)
(begin
(when gostat
- (format (current-warning-port)
- ";;; note: source file ~a\n;;; newer than
compiled ~a\n"
- name go-file-name))
+ (cond-warn-newer name go-file-name)
(cond
(%load-should-auto-compile
(%warn-auto-compilation-enabled)
- (format (current-warning-port) ";;; compiling ~a\n" name)
+ (cond-warn-compiling name)
(let ((cfn (compile name)))
- (format (current-warning-port) ";;; compiled ~a\n" cfn)
+ (cond-warn-compiled cfn)
(load-thunk-from-file cfn)))
(else #f)))))
#:warning "WARNING: compilation of ~a failed:\n" name))
diff --git a/module/ice-9/command-line.scm b/module/ice-9/command-line.scm
index 98d3855..fe639bb 100644
--- a/module/ice-9/command-line.scm
+++ b/module/ice-9/command-line.scm
@@ -136,6 +136,8 @@ If FILE begins with `-' the -s switch is mandatory.
--listen[=P] listen on a local port or a path for REPL clients;
if P is not given, the default is local port 37146
-q inhibit loading of user init file
+ --quiet inhibit informational compiler output, e.g name of
+ files being compiled
--use-srfi=LS load SRFI modules for the SRFIs in LS,
which is a list of numbers like \"2,13,14\"
-h, --help display this help and exit
@@ -358,6 +360,10 @@ If FILE begins with `-' the -s switch is mandatory.
(set! inhibit-user-init? #t)
(parse args out))
+ ((string=? arg "--quiet")
+ (set! %verbose-compiler-messages #f)
+ (parse args out))
+
((string-prefix? "--use-srfi=" arg)
(let ((srfis (map (lambda (x)
(let ((n (string->number x)))
--
2.8.3
