Re: [PATCH 4/6] fsmonitor: Add a trailing newline to test-dump-fsmonitor

[Junio C Hamano]

Alex Vandiver  writes:

> The only current uses of this tool are in tests, which only examine
> the first ("no fsmonitor" / "fsmonitor last update ...") line.  I find
> it useful as a brief summary view of the fsmonitor bits, but I suppose
> I'd also be happy with just presence/absence and a count of set/unset
> bits.
>
> Barring objections from Dscho or Ben, I'll reroll with a version that
> shows something like:
>
> fsmonitor last update 1513821151547101894 (5 seconds ago)
> 5 files valid / 10 files invalid

As I agree that this is test/debug only, I do not care too deeply
either way, as long as it does not end with an incomplete line ;-)

Re: [PATCH 4/6] fsmonitor: Add a trailing newline to test-dump-fsmonitor

[Alex Vandiver]

On Tue, 19 Dec 2017, Junio C Hamano wrote:
> That (and existing) uses of printf() all feel a bit overkill ;-)
> Perhaps putchar() would suffice.
> 
> I am not sure if the above wants to become something like
> 
>   for (i = 0; i < istate->cache_nr; i++) {
>   putchar(istate->cache[i]->ce_flags & CE_FSMONITOR_VALID ? '+' : 
> '-');
>   quote_c_style(istate->cache[i]->name, NULL, stdout, 0);
>   putchar('\n');
>   }
> 
> instead of "a single long incomplete line" in the first place.  Your
> "fix" merely turns it into "a single long complete line", which does
> not quite feel big enough an improvement, at least to me.

The more user-digestable form like you describe already exists by way
of `git ls-files -f`.  I am not sure it is worth replicating it.

The only current uses of this tool are in tests, which only examine
the first ("no fsmonitor" / "fsmonitor last update ...") line.  I find
it useful as a brief summary view of the fsmonitor bits, but I suppose
I'd also be happy with just presence/absence and a count of set/unset
bits.

Barring objections from Dscho or Ben, I'll reroll with a version that
shows something like:

fsmonitor last update 1513821151547101894 (5 seconds ago)
5 files valid / 10 files invalid

 - Alex

Re: [PATCH 4/6] fsmonitor: Add a trailing newline to test-dump-fsmonitor

[Junio C Hamano]

Alex Vandiver <ale...@dropbox.com> writes:

> Subject: Re: [PATCH 4/6] fsmonitor: Add a trailing newline to 
> test-dump-fsmonitor

"Subject: fsmonitor: complete the last line of test-dump-fsmonitor output"

perhaps.

> This makes it more readable when used for debugging from the
> commandline.
>
> Signed-off-by: Alex Vandiver <ale...@dropbox.com>
> ---
>  t/helper/test-dump-fsmonitor.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/t/helper/test-dump-fsmonitor.c b/t/helper/test-dump-fsmonitor.c
> index 53b19b39b..4e56929f7 100644
> --- a/t/helper/test-dump-fsmonitor.c
> +++ b/t/helper/test-dump-fsmonitor.c
> @@ -19,5 +19,6 @@ int cmd_main(int ac, const char **av)
>   for (i = 0; i < istate->cache_nr; i++)
>   printf((istate->cache[i]->ce_flags & CE_FSMONITOR_VALID) ? "+" 
> : "-");
>  
> + printf("\n");

That (and existing) uses of printf() all feel a bit overkill ;-)
Perhaps putchar() would suffice.

I am not sure if the above wants to become something like

for (i = 0; i < istate->cache_nr; i++) {
putchar(istate->cache[i]->ce_flags & CE_FSMONITOR_VALID ? '+' : 
'-');
quote_c_style(istate->cache[i]->name, NULL, stdout, 0);
putchar('\n');
}

instead of "a single long incomplete line" in the first place.  Your
"fix" merely turns it into "a single long complete line", which does
not quite feel big enough an improvement, at least to me.

>   return 0;
>  }