From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: Next steps Date: Fri, 15 Jun 2018 23:47:59 +0200 Message-ID: <87r2l767sg.fsf@elephly.net> References: <8ea5d026-fab9-7b12-198e-610ad7743cb2@swecha.net> <87fu1zznl3.fsf@elephly.net> <87bmcnzjsp.fsf@elephly.net> <18408eca-ad0b-111a-0ef6-ab452c9ca89c@swecha.net> <878t7ryxvf.fsf@elephly.net> <02e1a59a-ebc1-dfce-e1d6-22e97e742214@swecha.net> <874lifypeb.fsf@elephly.net> <648f81fc-0607-2c3b-f9a3-ca3b29cd637b@swecha.net> <87bmck22uz.fsf@elephly.net> <8736xv1xvq.fsf@elephly.net> <64dfaf0c-3bcf-7b76-5831-b1c41d3f5563@swecha.net> <877en4ccut.fsf@elephly.net> <2b03fcec-c2d8-0604-156c-70fe7643a6cb@swecha.net> <87zhzzbqn6.fsf@elephly.net> <8f509759-c798-a150-bf5a-9b13cd07ac5b@swecha.net> <87lgbibir5.fsf@elephly.net> <564233bd-9511-7f84-3350-6a3a905534a8@swecha.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:49938) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fTwpF-00020F-6j for guix-devel@gnu.org; Fri, 15 Jun 2018 18:04:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fTwpB-0000hS-2z for guix-devel@gnu.org; Fri, 15 Jun 2018 18:04:09 -0400 Received: from sender-of-o51.zoho.com ([135.84.80.216]:21111) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fTwpA-0000h1-S0 for guix-devel@gnu.org; Fri, 15 Jun 2018 18:04:05 -0400 In-reply-to: <564233bd-9511-7f84-3350-6a3a905534a8@swecha.net> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Sahithi Cc: guix-devel 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=E2=80=99s talk about the next steps. 1) Enabling colours only *optionally*. We modified =E2=80=9Cguix/scripts/build.scm=E2=80=9D to test the changes, b= ut this really should be optional, because not everybody wants colours. For example, when the environment variable =E2=80=9CNO_COLOR=E2=80=9D is se= t to any value we should not use colours. Likewise, when the environment variable =E2=80=9CINSIDE_EMACS=E2=80=9D has a value (meaning that this is a= shell in Emacs where people should rather use =E2=80=9Cguix-build-log-minor-mode=E2= =80=9D) we should not print colours. You can read the values of environment variables with the =E2=80=9Cgetenv=E2=80=9D procedure (it is explained in t= he manual). This check could be in =E2=80=9Cguix/scripts/build.scm=E2=80=9D and =E2=80=9Cguix/scripts/package.scm=E2=80=9D to either use =E2=80=9Ccolorful-= build-output-port=E2=80=9D or =E2=80=9C(current-error-port)=E2=80=9D. Another option is to do this in the definition of =E2=80=9Ccolorful-build-output-port=E2=80=9D itself, which would either be = defined with =E2=80=9Chandle-string=E2=80=9D or =E2=80=9Chandle-plain-string=E2=80=9D (w= hich 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 =E2=80=9Cguix package=E2=80=9D is used).= Instead of swapping out the full port we could configure it with various options like =E2=80=9Ccolor?=E2=80=9D and =E2=80=9Cfilter?=E2=80=9D. 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 =E2=80=93 according to the manual it should force or flush the output, not print a space. Please use =E2=80=9C(lambda () (force-output (current-error-port)))=E2=80=9D instead. The fourth procedure reads a character. Currently it also turns the read character to an uppercase character. We don=E2=80=99t need that, beca= use we don=E2=80=99t read from the port at all. You can use =E2=80=9C(const #t= )=E2=80=9D instead. Time estimate: This should take less than 10 minutes to fix. 3) Filtering all lines between =E2=80=9Cstarting phase=E2=80=9D and =E2=80= =9Cphase succeeded/failed=E2=80=9D. Currently, the port applies colours and passes all other lines through unmodified. When using =E2=80=9Cguix package=E2=80=9D it may be good to *h= ide* other lines and replace them with a progress indicator. The first step to test this would be to replace *any* other line with =E2=80=9C.=E2=80=9D. Y= ou would then only see the lines that announce phases, but not the build output. (Later we would change the dots for a cute little =E2=80=9Cspinner=E2=80=9D= animation.) Can you think of a way to *only* filter lines when =E2=80=9Cguix package=E2= =80=9D is used but not when =E2=80=9Cguix build=E2=80=9D is used? Maybe we need anot= her variation of the port=E2=80=A6? 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 =E2=80=9Ccolorize-string=E2=80=9D a lot! Can we do better? How = about using =E2=80=9Cmatch:count=E2=80=9D 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 =E2=80=9Cmatch-lambda=E2=80=9D (srfi srfi-1)) ; need this for =E2=80=9Cany=E2=80=9D (define str "phase foo failed after 100 seconds") ;; Each list item in =E2=80=9Cpatterns=E2=80=9D is a regular expression fol= lowed 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 =E2=80=9Cany=E2=80=9D of the patterns matches, i.e. returns a = string and ;; not just #f. We use =E2=80=9Cmatch-lambda=E2=80=9D to bind the patte= rn to the ;; variable =E2=80=9Cpattern=E2=80=9D and the list of colours to the var= iable ;; =E2=80=9Ccolors=E2=80=9D. (or (any (match-lambda ((pattern . colors) ;; TODO: use string-match, match:count, match:substring, ;; colorize-string, and=3D>, etc to see if a pattern matches ;; and to transform the string according to =E2=80=9Ccolors= =E2=80=9D. ;; If the pattern does not match return #f to ;; automatically try the next, thanks to =E2=80=9Cany=E2=80= =9D. #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 =E2=80=9Cpk=E2=80=9D to confirm that vari= ables have the values you expect, and to ask others if you get stuck. -- Ricardo