guix-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Status of Submitted Patches


From: Ricardo Wurmus
Subject: Re: Status of Submitted Patches
Date: Wed, 23 May 2018 10:21:37 +0200
User-agent: mu4e 1.0; emacs 25.3.1

Hi Sahithi,

>> As a first change, could you please add the relevant parts of “(ice-9
>> colorized)” to a module in Guix?  We probably don’t want to depend on
>> having users install this module separately.  We also don’t need all of
>> it.  Please prepare a patch that adds only the relevant parts to “(guix
>> ui)” and update the copyright headers.
>
> I think small changes to the attached file will serve the purpose.
> But when I tried executing the file, that resulted with a error unbound 
> variable : colorize string
>
> I am not sure where I went wrong, can you please explain.

Procedures need to be defined before they are used.  On the first line

    (display (colorize-string "Hello!\n" 'RED 'BOLD 'ON-BLUE))

you’re refering to “colorize-string”, which is only defined at the very
bottom.

Another problem is that you’re defining a module, but you aren’t using
it correctly.  A module has a name — in this case that’s “(term
ansi-color)” – and that name must match the path of the file containing
it.  So for “(term ansi-color)” we’d expect the module to be in a file
“term/ansi-color.scm” in a directory in which Guile looks for modules.

For your change to Guix itself I’d suggest adding the needed definitions
to the existing module “(guix ui)”.

Another note about style: I think it would be better to use
“alist->hash-table” instead of “make-hash-table” followed by repeated
modifications to the hash table with “hashq-set!”.  We prefer to avoid
mutation of values when possible.

Regarding copyright headers: please make sure to also add a copyright
line for yourself and a copyright line from the file of guile-colorize
to “(guix ui)”.

When you’re done with these changes, please make a local commit and send
the output of “git format-patch -1”.

Thanks!

--
Ricardo





reply via email to

[Prev in Thread] Current Thread [Next in Thread]