Web lists-archives.com

Re: [GSoc][PATCH v3 1/3] test functions: add function `test_file_not_empty`




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