Next steps

[Ricardo Wurmus <rekado@elephly.net>]

Hi Sahithi,

> I have fixed the errors mentioned bellow, Please do check and notify me
> for further modifications.

Excellent, this looks better.  Unfortunately, your patch includes a
couple of unrelated indentation changes.  Please remove those.

Let’s talk about the next steps.


1) Enabling colours only *optionally*.

We modified “guix/scripts/build.scm” to test the changes, but this
really should be optional, because not everybody wants colours.

For example, when the environment variable “NO_COLOR” is set to any
value we should not use colours.  Likewise, when the environment
variable “INSIDE_EMACS” has a value (meaning that this is a shell in
Emacs where people should rather use “guix-build-log-minor-mode”) we
should not print colours.  You can read the values of environment
variables with the “getenv” procedure (it is explained in the manual).

This check could be in “guix/scripts/build.scm” and
“guix/scripts/package.scm” to either use “colorful-build-output-port” or
“(current-error-port)”.

Another option is to do this in the definition of
“colorful-build-output-port” itself, which would either be defined with
“handle-string” or “handle-plain-string” (which uses no colours)
dependent on these environment variables.

This option would be better, because we want to extend the soft port to
also filter lines optionally (when “guix package” is used).  Instead of
swapping out the full port we could configure it with various options
like “color?” and “filter?”.

Time estimate: This should take no longer than 3 days.


2) Fixing the other soft port procedures

Currently we have this:

> +   (vector
> +    (lambda (c) (write c (current-error-port)))
> +    ;; procedure accepting one character for output
> +    handle-string
> +    ;; procedure accepting a string for handle-string procedure
> +    (lambda () (display " " (current-error-port)))
> +    (lambda () (char-upcase (read-char)))
> +    (lambda () (display "@" (current-error-port))))

Please move the comments *above* the procedures they describe.  Note
that the third procedure is still wrong – according to the manual it
should force or flush the output, not print a space.  Please use
“(lambda () (force-output (current-error-port)))” instead.

The fourth procedure reads a character.  Currently it also turns the
read character to an uppercase character.  We don’t need that, because
we don’t read from the port at all.  You can use “(const #t)” instead.

Time estimate: This should take less than 10 minutes to fix.


3) Filtering all lines between “starting phase” and “phase
succeeded/failed”.

Currently, the port applies colours and passes all other lines through
unmodified.  When using “guix package” it may be good to *hide* other
lines and replace them with a progress indicator.  The first step to
test this would be to replace *any* other line with “.”.  You would then
only see the lines that announce phases, but not the build output.

(Later we would change the dots for a cute little “spinner” animation.)

Can you think of a way to *only* filter lines when “guix package” is
used but not when “guix build” is used?  Maybe we need another variation
of the port…?

Time estimate: You should have some results and an idea how to finish
this after no more than a day of work.


4) Making the colorization prettier.

We repeat “colorize-string” a lot!  Can we do better?  How about using
“match:count” and a list of colours to be applied in order?

Start playing with something like this in the REPL:

--8<---------------cut here---------------start------------->8---


(use-modules (ice-9 match)   ; need this for “match-lambda”
             (srfi srfi-1))  ; need this for “any”

(define str "phase foo failed after 100 seconds")


;; Each list item in “patterns” is a regular expression followed by a
;; number of colours.
(let ((patterns '(("^(starting phase )(.*)"
                   BLUE GREEN)
                  ("^(phase)(.*)(succeeded after)(.*)(seconds)"
                   GREEN BLUE GREEN BLUE GREEN)
                  ("^(phase)(.*)(failed after)(.*)(seconds)"
                   RED BLUE RED BLUE RED))))

   ;; See if “any” of the patterns matches, i.e. returns a string and
   ;; not just #f.  We use “match-lambda” to bind the pattern to the
   ;; variable “pattern” and the list of colours to the variable
   ;; “colors”.

   (or (any (match-lambda
             ((pattern . colors)
              ;; TODO: use string-match, match:count, match:substring,
              ;; colorize-string, and=>, etc to see if a pattern matches
              ;; and to transform the string according to “colors”.

              ;; If the pattern does not match return #f to
              ;; automatically try the next, thanks to “any”.
              #f
             ))
             patterns)
       ;; Nothing matched, so return the string without changes.
       str))
--8<---------------cut here---------------end--------------->8---

Take your time to understand this one.  Play around with this in the
REPL and ask questions on IRC (both #guile and #guix) if the manual does
not answer your questions.  Once you understand it, it should take no
longer than 3 days to implement this.

All together, these steps should take no longer than two weeks to
implement.  What do you think?

Remember to play in the REPL, use “pk” to confirm that variables have
the values you expect, and to ask others if you get stuck.

--
Ricardo