Re: add command line option to quiet compiler messages

[Tobin Harding]

On Thu, Jul 14, 2016 at 12:01:10PM +0200, Andy Wingo wrote:
> On Wed 13 Jul 2016 06:08, Tobin Harding <address@hidden> 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