Re: [PATCH v2 4/8] status: per-file data collection for --porcelain=v2
On 07/25/2016 04:14 PM, Junio C Hamano wrote:
Jeff Hostetler writes:
+static void aux_updated_entry_porcelain_v2(
+ struct wt_status *s,
+ struct wt_status_change_data *d,
+ struct diff_filepair *p)
+{
+ switch (p->status) {
+ case DIFF_STATUS_ADDED:
+ /* {mode,sha1}_head are zero for an add. */
+ d->aux.porcelain_v2.mode_index = p->two->mode;
I doubt that it makes sense in the longer term to have a new "aux"
field. Why isn't it part of the wt_status_change_data structure?
For that matter, why should these new functions have both "aux" and
"v2" in their names?
Imagine what should happen when somebody wants to add --porcelain=v3
format in 6 months. Why must "v3" be treated differently from "v1"
and in a way close to "v2"? Why shouldn't all the three be treated
in a similar way that "v1" has already?
I wasn't sure if we wanted the v2 fields to be isolated
and only filled in for v2 requests or whether we wanted
them to be common going forward. In the case of the former,
I could see the "aux" struct becoming a union and the various
aux_*() routines only populating one member in that union.
And then the various per-format print routines would know
which aux member to access. That may be more complicated
that necessary though -- if we assume that any subsequent
formats (and possibly any JSON formats) would always want
to keep these fields and add more.
I'll flatten the fields into the main structure.
+ oidcpy(&d->aux.porcelain_v2.oid_index, &p->two->oid);
+ break;
+
+ case DIFF_STATUS_DELETED:
+ d->aux.porcelain_v2.mode_head = p->one->mode;
+ oidcpy(&d->aux.porcelain_v2.oid_head, &p->one->oid);
+ /* {mode,oid}_index are zero for a delete. */
+ break;
+
+ case DIFF_STATUS_RENAMED:
+ d->aux.porcelain_v2.rename_score = p->score * 100 / MAX_SCORE;
I have a slight aversion against losing the precision in a helper
function like this that does not do the actual output, but it
probably is OK.
Don't we have copy detection score that is computed exactly the same
way for DIFF_STATUS_COPIED case, too?
Yes I believe so. I'll see about adding that. Or rather make
the field apply to both.
For readability, unless a case arm is completely empty, we should
have
/* fallthru */
comment where "break;" would go for a normal case arm.
will do. thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/8] status: per-file data collection for --porcelain=v2
Jeff Hostetler writes:
> +static void aux_updated_entry_porcelain_v2(
> + struct wt_status *s,
> + struct wt_status_change_data *d,
> + struct diff_filepair *p)
> +{
> + switch (p->status) {
> + case DIFF_STATUS_ADDED:
> + /* {mode,sha1}_head are zero for an add. */
> + d->aux.porcelain_v2.mode_index = p->two->mode;
I doubt that it makes sense in the longer term to have a new "aux"
field. Why isn't it part of the wt_status_change_data structure?
For that matter, why should these new functions have both "aux" and
"v2" in their names?
Imagine what should happen when somebody wants to add --porcelain=v3
format in 6 months. Why must "v3" be treated differently from "v1"
and in a way close to "v2"? Why shouldn't all the three be treated
in a similar way that "v1" has already?
> + oidcpy(&d->aux.porcelain_v2.oid_index, &p->two->oid);
> + break;
> +
> + case DIFF_STATUS_DELETED:
> + d->aux.porcelain_v2.mode_head = p->one->mode;
> + oidcpy(&d->aux.porcelain_v2.oid_head, &p->one->oid);
> + /* {mode,oid}_index are zero for a delete. */
> + break;
> +
> + case DIFF_STATUS_RENAMED:
> + d->aux.porcelain_v2.rename_score = p->score * 100 / MAX_SCORE;
I have a slight aversion against losing the precision in a helper
function like this that does not do the actual output, but it
probably is OK.
Don't we have copy detection score that is computed exactly the same
way for DIFF_STATUS_COPIED case, too?
For readability, unless a case arm is completely empty, we should
have
/* fallthru */
comment where "break;" would go for a normal case arm.
> + case DIFF_STATUS_COPIED:
> + case DIFF_STATUS_MODIFIED:
> + case DIFF_STATUS_TYPE_CHANGED:
> + case DIFF_STATUS_UNMERGED:
> + d->aux.porcelain_v2.mode_head = p->one->mode;
> + d->aux.porcelain_v2.mode_index = p->two->mode;
> + oidcpy(&d->aux.porcelain_v2.oid_head, &p->one->oid);
> + oidcpy(&d->aux.porcelain_v2.oid_index, &p->two->oid);
> + break;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/8] status: per-file data collection for --porcelain=v2
The output of `git status --porcelain` leaves out many details
about the current status that clients might like to have. This
can force them to be less efficient as they may need to launch
secondary commands (and try to match the logic within git) to
accumulate this extra information. For example, a GUI IDE might
need the file mode to display the correct icon for a changed item.
Signed-off-by: Jeff Hostetler
---
builtin/commit.c | 3 ++
wt-status.c | 114 ++-
wt-status.h | 17 +
3 files changed, 133 insertions(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index e6bbb12..5b9efd2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -153,6 +153,8 @@ static int opt_parse_porcelain(const struct option *opt,
const char *arg, int un
*value = STATUS_FORMAT_PORCELAIN;
else if (!strcmp(arg, "v1"))
*value = STATUS_FORMAT_PORCELAIN;
+ else if (!strcmp(arg, "v2"))
+ *value = STATUS_FORMAT_PORCELAIN_V2;
else
die("unsupported porcelain version");
@@ -1104,6 +1106,7 @@ static struct status_deferred_config {
static void finalize_deferred_config(struct wt_status *s)
{
int use_deferred_config = (status_format != STATUS_FORMAT_PORCELAIN &&
+ status_format != STATUS_FORMAT_PORCELAIN_V2
&&
!s->null_termination);
if (s->null_termination) {
diff --git a/wt-status.c b/wt-status.c
index a9031e4..54aedc1 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -406,6 +406,110 @@ static void wt_longstatus_print_change_data(struct
wt_status *s,
strbuf_release(&twobuf);
}
+static void aux_updated_entry_porcelain_v2(
+ struct wt_status *s,
+ struct wt_status_change_data *d,
+ struct diff_filepair *p)
+{
+ switch (p->status) {
+ case DIFF_STATUS_ADDED:
+ /* {mode,sha1}_head are zero for an add. */
+ d->aux.porcelain_v2.mode_index = p->two->mode;
+ oidcpy(&d->aux.porcelain_v2.oid_index, &p->two->oid);
+ break;
+
+ case DIFF_STATUS_DELETED:
+ d->aux.porcelain_v2.mode_head = p->one->mode;
+ oidcpy(&d->aux.porcelain_v2.oid_head, &p->one->oid);
+ /* {mode,oid}_index are zero for a delete. */
+ break;
+
+ case DIFF_STATUS_RENAMED:
+ d->aux.porcelain_v2.rename_score = p->score * 100 / MAX_SCORE;
+ case DIFF_STATUS_COPIED:
+ case DIFF_STATUS_MODIFIED:
+ case DIFF_STATUS_TYPE_CHANGED:
+ case DIFF_STATUS_UNMERGED:
+ d->aux.porcelain_v2.mode_head = p->one->mode;
+ d->aux.porcelain_v2.mode_index = p->two->mode;
+ oidcpy(&d->aux.porcelain_v2.oid_head, &p->one->oid);
+ oidcpy(&d->aux.porcelain_v2.oid_index, &p->two->oid);
+ break;
+
+ case DIFF_STATUS_UNKNOWN:
+ die("BUG: index status unknown");
+ break;
+ }
+}
+
+/* Save aux info for a head-vs-index change. */
+static void aux_updated_entry(
+ struct wt_status *s,
+ struct wt_status_change_data *d,
+ struct diff_filepair *p)
+{
+ if (s->status_format == STATUS_FORMAT_PORCELAIN_V2)
+ aux_updated_entry_porcelain_v2(s, d, p);
+}
+
+static void aux_changed_entry_porcelain_v2(
+ struct wt_status *s,
+ struct wt_status_change_data *d,
+ const struct diff_filepair *p)
+{
+ switch (p->status) {
+ case DIFF_STATUS_ADDED:
+ die("BUG: worktree status add???");
+ break;
+
+ case DIFF_STATUS_DELETED:
+ d->aux.porcelain_v2.mode_index = p->one->mode;
+ oidcpy(&d->aux.porcelain_v2.oid_index, &p->one->oid);
+ /* mode_worktree is zero for a delete. */
+ break;
+
+ case DIFF_STATUS_MODIFIED:
+ case DIFF_STATUS_TYPE_CHANGED:
+ case DIFF_STATUS_UNMERGED:
+ d->aux.porcelain_v2.mode_index = p->one->mode;
+ d->aux.porcelain_v2.mode_worktree = p->two->mode;
+ oidcpy(&d->aux.porcelain_v2.oid_index, &p->one->oid);
+ break;
+
+ case DIFF_STATUS_UNKNOWN:
+ die("BUG: worktree status unknown???");
+ break;
+ }
+}
+
+/* Save aux info for an index-vs-worktree change. */
+static void aux_changed_entry(
+ struct wt_status *s,
+ struct wt_status_change_data *d,
+ struct diff_filepair *p)
+{
+ if (s->status_format == STATUS_FORMAT_PORCELAIN_V2)
+ aux_changed_entry_porcelain_v2(s, d, p);
+}
+
+static void aux_initial_entry_porcelain_v2(
+ struct wt_status *s,
+ struct wt_status_change_data *d,
+ const struct cache_entry *ce)
+{
+ d->aux.porcelain_v2.mode_index = ce->ce_mode;
+ hashcpy(d->aux.porcelain_v2.oid_index.hash, ce->sha1);
+}
