guix-patches
[Top][All Lists]
Advanced

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

[bug#44460] Add copyright lines


From: John Soo
Subject: [bug#44460] Add copyright lines
Date: Wed, 11 Nov 2020 09:51:08 -0800
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)

Hello Ludo,

Ludovic Courtès <ludo@gnu.org> writes:

>> +@table @code
>> +@item --normalize
>> +Normalize the output records into record sets (@pxref{Record Sets,,,
>> +recutils, GNU recutils manual}).  Normalizing into record sets allows
>> +joins across record types.
>> +
>> +@example
>> +$ guix processes --normalize | \
>> +    recsel \
>> +    -j Session \
>> +    -t ChildProcess \
>> +    -p Session.PID,PID \
>> +    -e 'Session.ClientCommand ~ build'
>> +Session_PID: 4278
>> +PID: 4435
>> +
>> +Session_PID: 4278
>> +PID: 4554
>> +
>> +Session_PID: 4278
>> +PID: 4646
>> +@end example
>> +@end table
>
> Nice!  Right above the example, I’d suggest adding a sentence like “The
> example below lists…” (what does it list actually? :-)).
>
> In the default format, I wonder if we could already change split
> ‘ChildProcess’ into ‘ChildPID’ and ‘ChildCommand’, as you had initially
> proposed on IRC; would that work?

I think we could do that, but I had two reasons to use the normalized
format instead.

* Backwards incompatibility - I didn't want to break any existing scripts
  that may exist.

* Still not normalized - how can I search for just the child processes
  associated with a particular command?

I wouldn't be opposed to splitting ChildProcess into ChildPID and
ChildCommand.  I would like it best if that change was made in addition
to adding the normalized version, since the normalized version allows
more functionality.

>> +(define (lock->record lock port)
>> +  (format port "LockHeld: ~a~%" lock))
>
> Maybe ‘lock->recutils’ for consistency and to avoid confusion with
> Scheme “records”?

Done.

>> +(define (format-single-record port)
>
> Maybe ‘daemon-sessions->recutils’?  Should ‘sessions’ be a parameter for
> clarity?

Much better, thank you. I updated the normalized version too.

>
>> +  "Display denormalized session information to PORT."
>> +  (for-each (lambda (session)
>> +              (daemon-session->recutils session port)
>> +                (newline port))
>                   ^
> Indentation is off.

Fixed.

>> +(define (child-process->normalized-record process port)
>> +  "Display PROCESS record on PORT in normalized form"
>> +  (format port "PID: ~a" (process-id process))
>> +  (newline port)
>> +  (format port "Command:~{ ~a~}" (process-command process)))
>> +
>> +(define (format-normalized port)
>
> Please add a docstring.  Perhaps make ‘sessions’ a parameter?

Done.

>> +  (define sessions (daemon-sessions))
>> +
>> +  (format port session-rec-type)
>
> As reported by ‘-Wformat’, passing a non-literal format string is risky;
> write this instead:
>
>   (display session-rec-type port)

Done.

>> +  (newline port)
>> +  (newline port)
>> +  (for-each
>> +   (lambda (session)
>
> Preferable indent ‘for-each’ like so:
>
>   (for-each (lambda (session)

Done.

> Likewise below.
>
>> +  (display (G_ "
>> +  --normalize            display results as normalized record sets"))
>
> Should it be ‘--format=normalized’ (just like we have ‘--format’ in
> ‘guix describe’, for instance)?

That makes sense. What do you think of

Other changes:

* Updated the record descriptors to include the possible
  fields for sessions. I got some nice guidance from the recutils irc on
  that.

* Put the PID and Command first for the ChildProcess

* Add a --list-formats like guix describe has.

Thanks!

John

Attachment: 0001-processes-Optionally-normalize-recutils-output.patch
Description: Text Data


reply via email to

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