From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: Re: Patch file for colorize module Date: Sat, 26 May 2018 16:16:02 +0200 Message-ID: <878t864i59.fsf@elephly.net> References: <8ea5d026-fab9-7b12-198e-610ad7743cb2@swecha.net> <87r2nvjte6.fsf@elephly.net> <5ab51417-b635-9725-9f48-3bc3f9b61fdf@swecha.net> <87tvsko2wd.fsf@elephly.net> <7290013c-990d-3f7d-d8db-38e090ed766a@swecha.net> <87zi28kt82.fsf@elephly.net> <8573e97d-d107-cde6-cd17-35f4ef6d2de3@swecha.net> <87k1takumm.fsf@elephly.net> <87o9hycwl6.fsf@elephly.net> <87r2mhdeap.fsf@elephly.net> <618c131c-6ba6-e525-aefc-72acca1c910f@swecha.net> <87a7suwtp7.fsf@elephly.net> <149bfb8c-22b5-797d-e88a-ca4077b0a4cc@swecha.net> <87d0xmok8e.fsf@elephly.net> <87k1rsb9ex.fsf@elephly.net> <3e099b0b-e3ec-2bbb-6d10-5b7e48c4dff6@swecha.net> <300fd917-6742-3ef1-044d-4b0f38a44250@swecha.net> <87a7sm4v5j.fsf@elephly.net> <3d5eca09-7730-bd38-265b-7942d0ea16ed@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]:58666) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fMa0a-0006OE-Pg for guix-devel@gnu.org; Sat, 26 May 2018 10:17:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fMa0W-0003l1-Pk for guix-devel@gnu.org; Sat, 26 May 2018 10:17:24 -0400 Received: from sender-of-o51.zoho.com ([135.84.80.216]:21028) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fMa0W-0003kG-He for guix-devel@gnu.org; Sat, 26 May 2018 10:17:20 -0400 In-reply-to: <3d5eca09-7730-bd38-265b-7942d0ea16ed@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: Sahitihi Cc: guix-devel@gnu.org Hi Sahithi, > Please find the attachment. Hope I made it correct this time. :) Thanks, this looks a lot better. Some comments below: > From 1bd9eaca576e7d958062197b9931d64c0882484e Mon Sep 17 00:00:00 2001 > From: root This indicates that you=E2=80=99re doing development as the root user. It= =E2=80=99s better to use a regular user and only switch to the root user when absolutely needed. For work on Guix you will not need to do anything as the root user. Also, you can tell git to use your name and email address by running these commands: git config --global user.name "Sahithi Yarlagadda" git config --global user.email "sahi@swecha.net" Any commits you make after that will be properly attributed to you. > Date: Sat, 26 May 2018 17:34:23 +0530 > Subject: [PATCH] Added Colorize module and Copyrights line to ui As you can see in the git history of the repository we use a peculiar convention for describing the changes. For this patch it would be something like this: --8<---------------cut here---------------start------------->8--- ui: Add support for colorization. * guix/ui.scm (ansi-color-tables): New variable. (color, colorize-string): New procedures. --8<---------------cut here---------------end--------------->8--- > diff --git a/guix/ui.scm b/guix/ui.scm > index 8d351607d..20fbf761f 100755 > --- a/guix/ui.scm > +++ b/guix/ui.scm > @@ -9,7 +9,9 @@ > ;;; Copyright =C2=A9 2015, 2016 Mathieu Lirzin > ;;; Copyright =C2=A9 2016 Roel Janssen > ;;; Copyright =C2=A9 2016 Benz Schenk > -;;; > +;;; Copyright =C2=A9 2013,2014 Free Software Foundation, Inc. Please add a space after the comma. > @@ -106,7 +108,9 @@ > guix-warning-port > warning > info > - guix-main)) > + guix-main > + color > + colorize-string)) Do we really need to export the =E2=80=9Ccolor=E2=80=9D procedure or is it = enough to export =E2=80=9Ccolorize-string=E2=80=9D? Do we even need to export =E2=80= =9Ccolorize-string=E2=80=9D at all or should we only export the soft port that applies colours to some strings? > @@ -1578,4 +1582,49 @@ and signal handling has already been set up." > (initialize-guix) > (apply run-guix args)) > > +(define ansi-color-tables I think that =E2=80=9Cansi-color-tables=E2=80=9D could be renamed to =E2=80= =9Ccolor-table=E2=80=9D. > +(define (color . lst) For every procedure we need to have a short docstring that describes the desired behaviour in a complete sentence or two. Could you provide a short description of the =E2=80=9Ccolor=E2=80=9D procedure? What does it d= o to its variable argument list =E2=80=9CLST=E2=80=9D? > +(define (colorize-string str . color-list) The same applies here. I=E2=80=99d be happy if you could make these changes quickly and send an up= dated patch. Once I receive it I=E2=80=99ll push it to a branch =E2=80=9Cwip-sah= ithi=E2=80=9D in the repository. Thanks in advance! - - - - - - - We=E2=80=99re still lacking code to actually use =E2=80=9Ccolorize-string= =E2=80=9D anywhere. Could you please prepare a patch or a series of patches that achieves the following: * add a soft port to (guix ui) that colorizes strings that match an internal list of regular expressions. The initial list of regular expressions could be something like this: '("^starting phase.*" "^phase .* succeeded.*" "^phase .* failed.*") See the documentation for =E2=80=9Cstring-match=E2=80=9D for details. Initially, the port should print any matching string as just green. All other strings should not be modified at all. This will become a little fancier in the future to allow use to apply different colours or formatting to different parts of the strings just like it is done by =E2=80=9Cguix-build-log-minor-mode=E2=80=9D of the Emacs interface for= Guix. (If you haven=E2=80=99t played with that interface yet, please do so.) * change =E2=80=9Cguix-package=E2=80=9D in =E2=80=9Cguix/scripts/package.sc= m=E2=80=9D such that it will use the new soft port. Note that =E2=80=9Ccurrent-build-output-port=E2= =80=9D in (guix store) is a so-called parameter, which can be modified dynamically using =E2=80=9Cparameterize=E2=80=9D. Take a look at =E2=80=9Cguix/scrip= ts/build.scm=E2=80=9D where =E2=80=9Ccurrent-build-output-port=E2=80=9D is parameterized to the void = port when =E2=80=9C--quiet=E2=80=9D was passed (this causes =E2=80=9Cguix build=E2= =80=9D to print no build log). The change to =E2=80=9Cguix-package=E2=80=9D should be very similar. Does this sound like a good idea to you? By using a soft port I think we can limit future work (e.g. hiding of parts of the build output) to just the inner procedures and data structures of the soft port. Please try to give us short updates every two days or so; this way we can ensure that any questions you may have can be addressed quickly. Please also ask right away when anything is unclear or if the suggested approach seems weird. -- Ricardo