First off I want to say thank you for your review and your time

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Jules Tamagnan <jtamagnan@gmail.com>
>> Date: Mon, 29 Oct 2018 11:52:17 -0700
>>
>> I've found it difficult to go from an integer group id to the name of
>> that group. I've looked at the source for `user-login-name` and
>> `system-groups` and could not find a way to do what I needed.
>>
>> As such I think it would be great to add a function to do that.
>
> Thanks.  Please allow me a few comments:
>
> Please always provide a ChangeLog-style commit log message describing
> your changes.  The file CONTRIBUTE has more about that.
>
>> I'm unsure how to make an argument not be optional.
>
> You want the argument NOT to be optional?  If so, make this:
>
>> +DEFUN ("group-name", Fgroup_name, Sgroup_name, 0, 1, 0,
>
> say this instead:
>
>> +DEFUN ("group-name", Fgroup_name, Sgroup_name, 1, 1, 0,
>
> See the node "Writing Emacs Primitives" in the ELisp manual, which
> describes how to use the DEFUN macro.

Turns out there is a word for not optional... required :)

>> +DEFUN ("group-name", Fgroup_name, Sgroup_name, 0, 1, 0,
>> +       doc: /* If argument GID is an integer or a float, return the login name
>> +of the group with that GID, or nil if there is no such GID.  */)
>> +  (Lisp_Object gid)
>> +{
>> +  struct group *gr;
>> +  gid_t id;
>> +
>> +  if (NILP (gid))
>> +    return Qnil;
>
> Wouldn't it be more useful to return user's group name when the
> argument is nil or omitted?

I always thought that user could have many groups, for example if I run

    $ groups $(id -un)

I get a long list of groups. I'm not sure if I misunderstood your
statement or the desired behavior but for the moment I think a separate
function would be best for this. Or maybe the `system-groups` function
could be modified to take in a UID and then list groups for that UID,
then if no argument is passed in, it would return the full list of
system-groups as it does now.

> Also, would you please write a couple of simple tests for this new
> function?

I've tried my hand at writing a test for this but am dubious it will
pass on all machines, I believe it should work on gnu/linux

> Finally, this function should be announced in NEWS and documented in
> the ELisp manual.

I've added a short entry in NEWS under "Lisp Changes in Emacs 27.1" and
in os.texi