Re: Status of Submitted Patches

[Ricardo Wurmus]

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

[Ricardo Wurmus]

Hi Sahithi,

>> When you’re done with these changes, please make a local commit and send
>> the output of “git format-patch -1”.
> the following is the output of “git format-patch -1”
>
> */0001-Added-Colorize-module-to-Guix-ui.scm.patch/*
>
> Can you please check and guide to proceed further.

Please attach the file of that same name in an email.

-- 
Ricardo

Re: Status of Submitted Patches

[Sahitihi]

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

*/0001-Added-Colorize-module-to-Guix-ui.scm.patch/*

Can you please check and guide to proceed further.

Thanks!

--
Sahithi

Re: Status of Submitted Patches

[Ricardo Wurmus]

Hi Sahithi,

>> 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.
>
> I have made all necessary modifications. Can please review it once.

Thanks, this looks fine.
You don’t need (srfi srfi-13) for “string-join”.

>> 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”.
>
>
> I will proceed further once it is reviewed. :)

Please proceed with the patch.  I’d like us to make progress a little
more quickly going forward.

Thanks!

--
Ricardo

Re: Status of Submitted Patches

[Sahitihi]

Hi Ricardo,
> 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.

I have made all necessary modifications. Can please review it once.
> 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”.


I will proceed further once it is reviewed. :)

Thanks!!
---
Sahithi
(define-module (term ansi-color)
 #:export  (color
colorize-string)
 #:use-module (srfi srfi-1)   ; for 'remove'
 #:use-module (srfi srfi-13)) ; for 'string-join' 

(define ansi-color-tables
  `((CLEAR   .   "0")
(RESET   .   "0")
(BOLD.   "1")
(DARK.   "2")
(UNDERLINE   .   "4")
(UNDERSCORE  .   "4")
(BLINK   .   "5")
(REVERSE .   "6")
(CONCEALED   .   "8")
(BLACK   .  "30")
(RED .  "31")
(GREEN   .  "32")
(YELLOW  .  "33")
(BLUE.  "34")
(MAGENTA .  "35")
(CYAN.  "36")
(WHITE   .  "37")
(ON-BLACK.  "40")
(ON-RED  .  "41")
(ON-GREEN.  "42")
(ON-YELLOW   .  "43")
(ON-BLUE .  "44")
(ON-MAGENTA  .  "45")
(ON-CYAN .  "46")
(ON-WHITE.  "47")))

(define (color . lst)
  (let ((color-list 
 (remove not 
 (map (lambda (color) (assq-ref ansi-color-tables color))
  lst
(if (null? color-list)
""
(string-append 
 (string #\esc #\[)
 (string-join color-list ";" 'infix)
 "m"
  
(define (colorize-string str . color-list)
  (string-append
   (apply color color-list)
   str
   (color 'RESET)))

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

(for-each display
  (list (color 'RED 'BOLD 'ON-BLUE)
"Hello!"
 (color 'RESET)))

Re: Status of Submitted Patches

[Sahitihi]

Hi Ricardo,
> 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.

--Sahithi

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

(for-each display
  (list (color 'RED 'BOLD 'ON-BLUE)
"Hello!"
 (color 'RESET)))
(define-module (term ansi-color)
 #:export  (color
colorize-string)
 #:use-module (srfi srfi-1)   ; for 'remove'
 #:use-module (srfi srfi-13)) ; for 'string-join' 

(define ansi-color-tables
  (let ((table (make-hash-table 23)))
(hashq-set! table 'CLEAR "0")
(hashq-set! table 'RESET "0")
(hashq-set! table 'BOLD  "1")
(hashq-set! table 'DARK  "2")
(hashq-set! table 'UNDERLINE "4")
(hashq-set! table 'UNDERSCORE "4")
(hashq-set! table 'BLINK "5")
(hashq-set! table 'REVERSE "6")
(hashq-set! table 'CONCEALED "8")
(hashq-set! table 'BLACK "30")
(hashq-set! table 'RED "31")
(hashq-set! table 'GREEN "32")
(hashq-set! table 'YELLOW "33")
(hashq-set! table 'BLUE "34")
(hashq-set! table 'MAGENTA "35")
(hashq-set! table 'CYAN "36")
(hashq-set! table 'WHITE "37")
(hashq-set! table 'ON-BLACK "40")
(hashq-set! table 'ON-RED "41")
(hashq-set! table 'ON-GREEN "42")
(hashq-set! table 'ON-YELLOW "43")
(hashq-set! table 'ON-BLUE "44")
(hashq-set! table 'ON-MAGENTA "45")
(hashq-set! table 'ON-CYAN "46")
(hashq-set! table 'ON-WHITE "47")
table))

(define (color . lst)
  (let ((color-list 
 (remove not 
 (map (lambda (color) (hashq-ref ansi-color-tables color))
  lst
(if (null? color-list)
""
(string-append 
 (string #\esc #\[)
 (string-join color-list ";" 'infix)
 "m"
  
(define (colorize-string str . color-list)
  (string-append
   (apply color color-list)
   str
   (color 'RESET)))

Re: Status of Submitted Patches

[Ricardo Wurmus]

Gábor Boskovits  writes:

> 2018-05-20 11:40 GMT+02:00 Ricardo Wurmus :
>
>>
>> Hi Sahithi,
>>
>> >> While this achieves the goal for a single character it does not
>> >> constitute a custom port.  Have you read the documentation for
>> >> “make-custom-port” in the Guile manual?
>> >
>> > I have tried with the following code for, Gábor helped me in process […]
>>
>> Oh, I haven’t seen those emails on the mailing list.  Please keep the
>> discussion on the mailing list so that all mentors and the community can
>> comment.
>>
>>
> The discussion was on IRC in a 1:1 conversation. The task Sahitihi wanted
> to achieve was to create a soft-port capitalizing all text sent to it. I
> helped her
> to achieve that. I was thinking about mailing you the details, but it was
> only a
> few lines of code. I will also make sure to keep you in the circuit in the
> future.

Ah, okay.  No worries.  I wasn’t on IRC most of last week because I was
travelling.

So one way of doing this is to use a soft port and use it as the current
output port (using parameterize) when colorization is enabled; another
way is to do this where the code is supposed to be displayed.  In the
first method the decision to colorize what and how is done by the port
procedures; in the second approach it’s done right before the string is
passed to “format” or “display”.

--
Ricardo

Re: Status of Submitted Patches

[Gábor Boskovits]

2018-05-20 11:40 GMT+02:00 Ricardo Wurmus :

>
> Hi Sahithi,
>
> >> While this achieves the goal for a single character it does not
> >> constitute a custom port.  Have you read the documentation for
> >> “make-custom-port” in the Guile manual?
> >
> > I have tried with the following code for, Gábor helped me in process […]
>
> Oh, I haven’t seen those emails on the mailing list.  Please keep the
> discussion on the mailing list so that all mentors and the community can
> comment.
>
>
The discussion was on IRC in a 1:1 conversation. The task Sahitihi wanted
to achieve was to create a soft-port capitalizing all text sent to it. I
helped her
to achieve that. I was thinking about mailing you the details, but it was
only a
few lines of code. I will also make sure to keep you in the circuit in the
future.


> > the description for that goes this way
> >
> > The input taken from input port is read and stored in variable "s".
> > This variable is passed to make-soft-port. The variable s is
> > capitalized by locale conversion then binded with color. the result is
> > displayed when called.
>
> Please try to be a little more precise with the descriptions.  You
> defined a new port with “make-soft-port”.  The new port has two
> important procedures: one that takes a single character, and another
> that takes a string.  In the case of a single character you just pass it
> through to the current output port.  In the case of a string, you
> colorize it first and then write it to the output port.
>
> The result is a port that will print a coloured string whenever you pass
> it a string.
>
> How would you use this in “(guix store)”?  Note that we don’t want to
> apply colour to any and all strings there.  We want only certain
> messages to be coloured.  Can you please prepare a patch to “(guix
> store)” that shows us how you would use this new port?
>
> 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 have tried the other process using escape codes however failed with
> > the result i will come with this implementation and procedure in my next
> > mail
>
> I have not received this email.  Have you worked on this yet?
>
> When you have problems with code please always provide the relevant
> parts of your code with the error messages, so that we can help you.
>
> I think that we need to start moving forward at a slighter higher
> speed.  Thanks!
>
> --
> Ricardo
>
>
>

Re: Status of Submitted Patches

[Ricardo Wurmus]

Hi Sahithi,

>> While this achieves the goal for a single character it does not
>> constitute a custom port.  Have you read the documentation for
>> “make-custom-port” in the Guile manual?
>
> I have tried with the following code for, Gábor helped me in process […]

Oh, I haven’t seen those emails on the mailing list.  Please keep the
discussion on the mailing list so that all mentors and the community can
comment.

> the description for that goes this way
>
> The input taken from input port is read and stored in variable "s".
> This variable is passed to make-soft-port. The variable s is
> capitalized by locale conversion then binded with color. the result is
> displayed when called.

Please try to be a little more precise with the descriptions.  You
defined a new port with “make-soft-port”.  The new port has two
important procedures: one that takes a single character, and another
that takes a string.  In the case of a single character you just pass it
through to the current output port.  In the case of a string, you
colorize it first and then write it to the output port.

The result is a port that will print a coloured string whenever you pass
it a string.

How would you use this in “(guix store)”?  Note that we don’t want to
apply colour to any and all strings there.  We want only certain
messages to be coloured.  Can you please prepare a patch to “(guix
store)” that shows us how you would use this new port?

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 have tried the other process using escape codes however failed with
> the result i will come with this implementation and procedure in my next
> mail

I have not received this email.  Have you worked on this yet?

When you have problems with code please always provide the relevant
parts of your code with the error messages, so that we can help you.

I think that we need to start moving forward at a slighter higher
speed.  Thanks!

--
Ricardo

Re: Status of Submitted Patches

[Sahitihi]

Hi Ricardo,

> Here you are reading a character from the current input port.  The
> result is fed to “char-upcase”, which turns it into an upper-case
> variant, and then you write that character to the current default output
> port.
>
> While this achieves the goal for a single character it does not
> constitute a custom port.  Have you read the documentation for
> “make-custom-port” in the Guile manual?

I have tried with the following code for, Gábor helped me in process

(use-modules (ice-9 binary-ports))
(use-modules (ice-9 i18n))

(define stdout (current-output-port))
(define s (read(current-input-port)))
(define p (make-soft-port
   (vector
    (lambda (c) (write c stdout))
    (lambda (s) (display (string-upcase! s) stdout))
    (lambda () (display "." stdout))
    (lambda () (char-upcase (read-char)))
    (lambda () (display "@" stdout)))
   "rw"))
(write s p)

The above resulted me with a capitalized output
> What type is the value in the variable with name “a”?  “read-char” takes
> a port and returns a single character from it, “char-upcase” takes a
> character and returns a different character, so “a” holds a character.
> As you know, the implementation of “colorize-string” internally glues a
> bunch of strings together: a terminal escape sequence to set the colour,
> the string to be coloured, and a terminal escape sequence to disable the
> colour.  You gave a character to the procedure, but it expects a string.
>
(use-modules (ice-9 binary-ports)) ;Though name is binary, All ports in
Guile are both binary and textual ports.
(use-modules (ice-9 i18n))  ; The |(ice-9 i18n)| module provides
procedures to manipulate text and other data
(use-modules (ice-9 colorized)) ;Colorizing module
(activate-colorized)

(define stdout (current-output-port)) ;stdout variable act as an output port
(define s (read(current-input-port))) ; s variable reads the input from
input port
;soft ports are used for customization on how output port works
(define p (make-soft-port
   (vector
    (lambda (c) (write c stdout)) ;accepting one character for
output
    (lambda (s) (display (colorized-display (string-upcase! s)
'(GREEN)) stdout)) ;accepting a string, Capitalizing it and then
colorizing with for output
    (lambda () (display "." stdout))
    (lambda () (char-upcase (read-char)))
    (lambda () (display "@" stdout)))
   "rw"))
(write s p)



This results out with a capitalized, colorized output

the description for that goes this way

The input taken from input port is read and stored in variable "s". 
This variable is passed to make-soft-port.  The variable s is
capitalized by locale conversion then binded with color. the result is
displayed when called.


I have tried the other process using escape codes however failed with
the result i will come with this implementation and procedure in my next
mail

Re: Status of Submitted Patches

[Ricardo Wurmus]

Ricardo Wurmus  writes:

> While this achieves the goal for a single character it does not
> constitute a custom port.  Have you read the documentation for
> “make-custom-port” in the Guile manual?

Correction: I meant “make-soft-port”, not “make-custom-port”.

-- 
Ricardo

Re: Status of Submitted Patches

[Ricardo Wurmus]

Hi Sahithi,

>>> According to the Guile manual
>>>
>>>Ports are the way that Guile performs input and output.  Guile can
>>>read in characters or bytes from an “input port”, or write them out
>>>to an “output port”.
>>>
>>> Ports allow the programmer to ignore the details of input/output sources
>>> and sinks and concentrate on shuffling characters or bytes around.  We
>>> don’t need to know if the port is connected to a terminal or a file or a
>>> network socket.
>>>
>>> A programmer can implement custom ports with “make-soft-port” and a
>>> vector of up to five procedures that provide implementations of certain
>>> operations (such as writing a string to the port).  A silly example
>>> would be a custom port that reads characters from standard input and
>>> turns them to upper-case characters before passing them to whoever reads
>>> from the custom port.

> (use-modules (ice-9 binary-ports))
> (define stdout (current-output-port))
> (define stdin (current-input-port))
> (display stdin)
> (newline)
> (display stdout)
> (newline)

These definitions have no effect on the code that follows.  “stdout” and
“stdin” have no special meaning.  You are just defining variables with
these names, but you could give them any names.  Giving them names has
no meaningful effect, because you are not using the names later anyway.

> (write (char-upcase(read-char(current-input-port

Here you are reading a character from the current input port.  The
result is fed to “char-upcase”, which turns it into an upper-case
variant, and then you write that character to the current default output
port.

While this achieves the goal for a single character it does not
constitute a custom port.  Have you read the documentation for
“make-custom-port” in the Guile manual?

>>> With what you know about terminal escape codes and the port facility
>>> that is used in “(guix store)”, can you describe an approach to add
>>> colours to some text fragments before they are printed?  There are at
>>> least two ways this can be done; one of the two options is more
>>> complicated but might result in a clearer and more modular
>>> implementation.
>
> For this i tried using […]

Actually, I was looking for a description of the two options, not an
implementation :)

Can you think of two possible ways to add colours to text fragments, and
describe them in a few sentences?  If you can only think of one
approach, please describe only that approach.

I guess that you wanted the value of “a” to be printed in colours, but
the code says otherwise.  Here you are just displaying a string that
contains the character “a”, not the value of the variable with the name
“a”.

> (define a (char-upcase(read-char (current-input-port
[…]
> (define b (colorize-string a '(GREEN)))
> (display b)
[…]
> 4058: 2 [#]
> In /home/sripathroy/Documents/GNUGuix/experiments/sam.scm:
>  17: 1 [#]
> In unknown file:
>  ?: 0 [string-append "\x1b[32m" #\S "\x1b[0m"]
>
> ERROR: In procedure string-append:
> ERROR: In procedure string-append: Wrong type (expecting string): #\S

What type is the value in the variable with name “a”?  “read-char” takes
a port and returns a single character from it, “char-upcase” takes a
character and returns a different character, so “a” holds a character.
As you know, the implementation of “colorize-string” internally glues a
bunch of strings together: a terminal escape sequence to set the colour,
the string to be coloured, and a terminal escape sequence to disable the
colour.  You gave a character to the procedure, but it expects a string.

You can convert strings to characters, but this is not what should be
done here.  If you did that for the string “hello” you would enable
colours, print an “h”, disable colours, enable colours, print an “e”,
disable colours, etc.  That’s rather wasteful because you really want to
only enable colours once, print the string, and then disable colours
again.

But we’re getting ahead of ourselves.  Getting back to the original
problem: can you think of and shortly describe two possible ways to
alter, filter or augment the output that is sent to the
“current-build-output-port” in “(guix store)”?  Try to describe this at
a higher level, ignoring a concrete implementation.  (Bits are for
computers, ideas are for humans.)

--
Ricardo

Re: Status of Submitted Patches

[Sahithi Yarlagadda]

Hi


On Wednesday 02 May 2018 01:34 PM, Ricardo Wurmus wrote:
> Hi Sahithi,
>
> I’ve previously sent the message below.  Have you been able to take a
> look at this yet?
>
> --
> Ricardo
>
>>> I have gone through the code under guix/store/ and the outputs are sent
>>> to a specified scheme variable "current-build-output-port"
>> Yes, the “(guix store)” module implements this.
>> “current-build-output-port” is a parameter, which is a dynamically bound
>> variable (i.e. its value can be changed by the caller) and it defaults
>> to the “current-error-port”.
I Understood this.

>> According to the Guile manual
>>
>>Ports are the way that Guile performs input and output.  Guile can
>>read in characters or bytes from an “input port”, or write them out
>>to an “output port”.
>>
>> Ports allow the programmer to ignore the details of input/output sources
>> and sinks and concentrate on shuffling characters or bytes around.  We
>> don’t need to know if the port is connected to a terminal or a file or a
>> network socket.
>>
>> A programmer can implement custom ports with “make-soft-port” and a
>> vector of up to five procedures that provide implementations of certain
>> operations (such as writing a string to the port).  A silly example
>> would be a custom port that reads characters from standard input and
>> turns them to upper-case characters before passing them to whoever reads
>> from the custom port.
(use-modules (ice-9 binary-ports))
(define stdout (current-output-port))
(define stdin (current-input-port))
(display stdin)
(newline)
(display stdout)
(newline)
(write (char-upcase(read-char(current-input-port


>> With what you know about terminal escape codes and the port facility
>> that is used in “(guix store)”, can you describe an approach to add
>> colours to some text fragments before they are printed?  There are at
>> least two ways this can be done; one of the two options is more
>> complicated but might result in a clearer and more modular
>> implementation.

For this i tried using

(use-modules (ice-9 binary-ports))
(use-modules (ice-9 colorized)) 
(activate-colorized)
(define stdout (current-output-port))
(define stdin (current-input-port))
(display stdin)
(newline)
(display stdout)
(newline)

(define a (char-upcase(read-char (current-input-port
(display a)   
(newline)

(display "\x1b[31ma\x1b[0m")
(newline)

(define b (colorize-string a '(GREEN)))
(display b)


Resulted:

4058: 2 [#]
In /home/sripathroy/Documents/GNUGuix/experiments/sam.scm:
  17: 1 [#]
In unknown file:
   ?: 0 [string-append "\x1b[32m" #\S "\x1b[0m"]

ERROR: In procedure string-append:
ERROR: In procedure string-append: Wrong type (expecting string): #\S


I found that the output is stored as #\. How should i convert to
valid format.
>
>

-- 
Regards
Sahithi