From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: Re: Fwd: Re: Patch file for colorize module Date: Wed, 06 Jun 2018 22:06:48 +0200 Message-ID: <87fu1zznl3.fsf@elephly.net> References: <8ea5d026-fab9-7b12-198e-610ad7743cb2@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> <878t864i59.fsf@elephly.net> <87o9gwpcmx.fsf@elephly.net> <2f71be8d-c672-c66a-0b16-bc3abc748754@swecha.net> <877enjpquf.fsf@elephly.net> <557b30de-5aa2-113f-7e90-a4a23e86bb07@swecha.net> <87602zbrcc.fsf@elephly.net> <328742c6-f96f-74b8-68ef-3b165a6199aa@swecha.net> <87zi0a3m3t.fsf@elephly.net> <07c93c07-2172-6bf1-6188-88830c1d8b4c@swecha.net> <87o9gpyq4t.fsf@elephly.net> <91773c8b-3072-2a82-dca3-cbeb5adf826d@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]:34981) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fQtU7-0001Ht-Rj for guix-devel@gnu.org; Thu, 07 Jun 2018 07:53:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fQtU4-0003FQ-Vz for guix-devel@gnu.org; Thu, 07 Jun 2018 07:53:43 -0400 Received: from sender-of-o51.zoho.com ([135.84.80.216]:21135) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fQtU4-0003EF-Nj for guix-devel@gnu.org; Thu, 07 Jun 2018 07:53:40 -0400 In-reply-to: <91773c8b-3072-2a82-dca3-cbeb5adf826d@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 Hi Sahithi, > Please find my changes in the patch file attached. Thanks. Could you please squash these changes and the previous commit? This is easier for me to review. Thanks for fixing the commit message! >> You have probably noticed that this looks rather repetitive at this >> point. Maybe we can think of a better way to express what colours >> should be applied. The match group numbers are monotonically >> increasing, so maybe we can avoid repeated statements of this kind and >> simply iterate over a list of colours=E2=80=A6 I have an idea already; = how >> about you? :) > > I have an idea about making a using filter-string and lists. Not sure > about functionality but that seems fine. :-P Could you share some more details? It doesn=E2=80=99t have to be a ready implementation in Scheme, but I=E2=80=99d love to hear a few more details. >> Another thing that=E2=80=99s worth thinking about now is the next step: >> how can we *optionally* hide all lines between these build system >> notices about started and completed build phases? > I din't think of it yet. Will do it in mean process. That=E2=80=99s okay. Let=E2=80=99s concentrate on the matter at hand first. >> One more thing: the fact that handle-string didn=E2=80=99t do the right = thing >> indicates that you didn=E2=80=99t test the changes well enough. To test= them, >> please locally modify =E2=80=9Cguix/scripts/build.scm=E2=80=9D and/or >> =E2=80=9Cguix/scripts/package.scm=E2=80=9D to make it use your colourful= port instead of >> the default, as discussed on IRC and in previous emails. > I made changes to /guix/scripts/build.scm /but that din/t workout. That > resulted me colourless outputs as before. :-( That=E2=80=99s because you must have overlooked a comment I made about your= code in =E2=80=9Chandle-string=E2=80=9D :) Let=E2=80=99s look at it: > (define (handle-string str) > (or (and=3D> (string-match "^(starting phase)(.*)" str) [=E2=80=A6] > ;; Didn=E2=80=99t match, so return unmodified string. > - str) > - (display str target-port)) > + (display str current-error-port))) This suffers from the same problem as before. The result of the expression starting with =E2=80=9Cor=E2=80=9D is ignored. You don=E2=80=99= t do anything with it. After that expression you just run =E2=80=9C(display str current-error-port)=E2=80=9D, which prints the original =E2=80=9Cstr=E2=80= =9D to the (undefined) =E2=80=9Ccurrent-error-port=E2=80=9D. You should do something with the result of the previous expression, such as storing it in a variable. Then you can use that variable in the final line. Something like this: --8<---------------cut here---------------start------------->8--- (define (handle-string str) (let ((message (or (and=3D> =E2=80=A6) =E2=80=A6 ;; Nothing matched, use unmodified string. str))) (display message (current-error-port)))) --8<---------------cut here---------------end--------------->8--- Also note that you must use =E2=80=9C(current-error-port)=E2=80=9D, not =E2=80=9Ccurrent-error-port=E2=80=9D. Could you please send an updated patch? -- Ricardo