Kamil Dudka wrote: > On Thursday 21 of January 2010 12:28:03 Jim Meyering wrote: >> Actually, more comments (in the code and log) would be welcome, too ;-) > > Sure. I also forgot the credit Piotr Gackiewicz as the original author > in the first place...
Glad you remembered that. >> Please add a little explanation before the new function, is_tty_writable > > What about "Return true if a terminal device given as PSTAT allows other users > to send messages to; false otherwise" ? > >> and list one or both of those URLs for context in the commit log. > > I think the rhbz link should be sufficient. Fine with me. >> > +** New features >> > + >> > + who --mesg now respects also group of a TTY when compiled with >> > + --with-tty-group >> >> Saying it "respects the group..." doesn't really communicate what is >> changed. How about saying that it works now in some cases (details?) >> where it used to fail? > > What about this? Good. Thanks. two of these: s/ group/ the group/ s/Now/Now,/ > "who --mesg used to ignore group of a TTY device when checking if it is > possible to send messages there. Now if coreutils is compiled > with --with-tty-group[=TTY_GROUP_NAME] configure option, it also compares > group of the TTY device with TTY_GROUP_NAME (or "tty" if no group is > specified)." > >> It would be helpful to say how to determine the appropriate group name. >> Something like "ls -lg /dev/tty" or >> $ stat --format %G /dev/tty >> tty > > Do you mean directly in the help string or somewhere else? I was thinking of a comment in the .m4 file, but now that you mention the help string, perhaps that's better. You choose. >> > diff --git a/src/who.c b/src/who.c >> >> ... >> >> There is enough duplication below that >> I'd prefer to move the #ifdef/#endif into the function. > > I am all for that. I thought it was prohibited in GNU coreutils :-) ;-) You're right that we go to extremes to avoid in-functions #ifdefs. In-function #ifdefs are evil, but so is code duplication. It's a trade-off. Since this function is so small, and the fraction of duplicated code would have been so high, the in-function #ifdef is clearly the lesser evil.
