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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 25, 2019, 09:21 UTC
Message-ID: <nycvar.QRO.7.76.6.1906251116020.44@tvgsbejvaqbjf.bet>
In-Reply-To: <755a4cfe-fd6b-044b-dca2-05eebfa518b1@jeffhostetler.com>

Hi Jeff,

On Mon, 24 Jun 2019, Jeff Hostetler wrote:

Show 13 quoted lines

> 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.

Show 40 quoted lines

> [...]
> > 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