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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 6/25/2019 5:52 AM, Duy Nguyen wrote:
On Tue, Jun 25, 2019 at 2:15 AM Jeff Hostetler <git@xxxxxxxxxxxxxxxxx> wrote:
@@ -202,6 +202,28 @@ void jw_object_null(struct json_writer *jw, const char *key)
       strbuf_addstr(&jw->json, "null");
   }

+void jw_object_filemode(struct json_writer *jw, const char *key, mode_t mode)
+{
+     object_common(jw, key);
+     strbuf_addf(&jw->json, "\"%06o\"", mode);
+}
+
+void jw_object_stat_data(struct json_writer *jw, const char *name,
+                      const struct stat_data *sd)

Should this be in json_writer.c or in read-cache.c ?
Currently, json_writer.c is concerned with formatting
JSON on basic/scalar types.  Do we want to start
extending it to handle arbitrary structures?  Or would
it be better for the code that defines/manipulates the
structure to define a "stat_data_dump_json()" function.

I'm torn on the jw_object_filemode() function, JSON format
limits us to decimal integers and there are places where
I'd like to have hex, or in this case octal values.

I'm thinking it'd be better to have a helper function in
read-cache.c that formats a local strbuf and calls
js_object_string(&jw, key, buf);

I can move these back to read-cache.c. Though if we have a lot more jw
helpers like this (hard to tell at the moment) then perhaps we can
have json-writer-utils.c or something to group them together. That
keep the "boring" code out of main logic code in read-cache.c and
other call sites.

yeah, in an utils file or close to the "constructors" of the
structure types.  either one works.


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

I'm not against reformatting it in hex string, but is there a value in
it? ce_flags is expanded in the code below so that you don't have to
decode it yourself when you read each entry. The "flags" field here is
for further processing in tools. I'm trying to see if looking at hex
values helps, but I'm still not seeing it...


I guess I was thinking of the in-memory bits and thinking
it'd be useful to be able to dump the index immediately
after reading it and then later after some operation or
traversal and see the intermediate states.  But I realize
now that that's not what you're building.  This is a dump
it while you're reading it feature (and that's fine).

So, as long as you have all of the on-disk bits, we should
be fine as you suggest.

Jeff


+     /*
+      * again redundant info, just so you don't have to decode
+      * flags values manually
+      */
+     if (ce->ce_flags & CE_EXTENDED)
+             jw_object_true(jw, "extended_flags");
+     if (ce->ce_flags & CE_VALID)
+             jw_object_true(jw, "assume_unchanged");
+     if (ce->ce_flags & CE_INTENT_TO_ADD)
+             jw_object_true(jw, "intent_to_add");
+     if (ce->ce_flags & CE_SKIP_WORKTREE)
+             jw_object_true(jw, "skip_worktree");
+     if (ce_stage(ce))
+             jw_object_intmax(jw, "stage", ce_stage(ce));



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux