[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
- Re: Status of Submitted Patches, Sahithi Yarlagadda, 2018/05/11
- Re: Status of Submitted Patches, Ricardo Wurmus, 2018/05/11
- Re: Status of Submitted Patches, Ricardo Wurmus, 2018/05/12
- Re: Status of Submitted Patches, Sahitihi, 2018/05/15
- Re: Status of Submitted Patches, Sahitihi, 2018/05/23
- Re: Status of Submitted Patches,
Ricardo Wurmus <=
- Re: Status of Submitted Patches, Sahitihi, 2018/05/24
- Re: Status of Submitted Patches, Ricardo Wurmus, 2018/05/24
- Re: Status of Submitted Patches, Sahitihi, 2018/05/24
- Re: Status of Submitted Patches, Ricardo Wurmus, 2018/05/25
- Patch file for colorize module, Sahitihi, 2018/05/25
- Re: Patch file for colorize module, Sahitihi, 2018/05/26
- Re: Patch file for colorize module, Ricardo Wurmus, 2018/05/26
- Re: Patch file for colorize module, Sahitihi, 2018/05/26
- Re: Patch file for colorize module, Ricardo Wurmus, 2018/05/26
- Re: Patch file for colorize module, Sahitihi, 2018/05/26