Re: [PATCH v2 01/10] ls-files: add --json to dump the index

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 2492 bytes --]

Hi Jeff,

On Mon, 24 Jun 2019, Jeff Hostetler wrote:

> On 6/24/2019 9:02 AM, Nguyễn Thái Ngọc Duy wrote:
> [...]
> > +{
> > +	jw_object_inline_begin_object(jw, name);
> > +	jw_object_intmax(jw, "ctime_sec", sd->sd_ctime.sec);
> > +	jw_object_intmax(jw, "ctime_nsec", sd->sd_ctime.nsec);
> > +	jw_object_intmax(jw, "mtime_sec", sd->sd_mtime.sec);
> > +	jw_object_intmax(jw, "mtime_nsec", sd->sd_mtime.nsec);
>
> It'd be nice if we could also have a formatted date
> for the mtime and ctime in addition to the integer
> values.  (I'm not sure whether you'd always want them
> or make it a verbose option.)

It would be more consistent with JSON conventions to have only the
formatted date, preferably in ISO-8601 format [*1*], I would think.

And for debugging, it would also make more sense, in my opinion.

> [...]
> > diff --git a/read-cache.c b/read-cache.c
> > index 4dd22f4f6e..db5147d088 100644
> > --- a/read-cache.c
> > +++ b/read-cache.c
> > @@ -25,6 +25,7 @@
> >   #include "fsmonitor.h"
> >   #include "thread-utils.h"
> >   #include "progress.h"
> > +#include "json-writer.h"
> >
> >   /* Mask for the name length in ce_flags in the on-disk index */
> >
> > @@ -1952,6 +1953,49 @@ static void *load_index_extensions(void *_data)
> >   	return NULL;
> >   }
> >
> > +static void dump_cache_entry(struct index_state *istate,
> > +			     int index,
> > +			     unsigned long offset,
> > +			     const struct cache_entry *ce)
> > +{
> > +	struct json_writer *jw = istate->jw;
> > +
> > +	jw_array_inline_begin_object(jw);
> > +
> > +	/*
> > +	 * this is technically redundant, but it's for easier
> > +	 * navigation when there hundreds of entries
> > +	 */
> > +	jw_object_intmax(jw, "id", index);
> > +
> > +	jw_object_string(jw, "name", ce->name);
> > +
> > +	jw_object_filemode(jw, "mode", ce->ce_mode);
> > +
> > +	jw_object_intmax(jw, "flags", ce->ce_flags);
>
> It would be nice to have the flags as a hex-formatted string
> in addition to (or instead of) the decimal integer value.

Instead of, please, prefixed with "0x" to make it clear that this is hex.

And "mode" in octal, please, prefixed with "0", as that is the convention
to display file modes.

Thanks,
Dscho

Footnote *1*: It is too bad that JSON leaves the exact date format
unspecified, leading to a lot of inconsistency and putting the burden on
parsers:
https://stackoverflow.com/questions/10286204/the-right-json-date-format