Hello Eric On 2019-03-04 19:17:50 -0500 Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Mon, Mar 4, 2019 at 7:08 AM Rohit Ashiwal <rohit.ashiwal265@xxxxxxxxx> wrote: > > if ! test -s "$1" > > then > > echo "'$1' is not a non-empty file." > > Although not incorrect, the double-negative is hard to digest. I had > to read it a few times to convince myself that it matched the intent > of the new function. I wonder if a message such as > > echo "'$1' is unexpectedly empty" > > would be better. (Subjective, and not at all worth a re-roll.) I think the current message is more accurate as it implies both: 1. There is no file, and 2. If there is, it is not empty "unexpectedly empty" may imply that there is a directory which is not empty and that is not the intention of the function. > Also, it might be a good idea to add this new function as a neighbor > of test_must_be_empty() rather than defining it a couple hundred lines > earlier in the file. Alternately, perhaps a preparatory patch could > move test_must_be_empty() closer to the other similar functions > (test_path_is_missing() and cousins). I think we should relocate the function `test_must_be_empty` in a separate patch as this patch deals with a different issue. Thanks Rohit
