From: Karthik Nayak <karthik.188@gmail.com>
Date: Tue, 14 Oct 2025 08:49:14 GMT
Subject: Re: [PATCH v4 01/12] wt-status: provide function to expose status for trees
Message-ID: <CAOLa=ZTUDOYb1KVUEZY4KFyoaP-PdXjBvUto0vxOtYanEjoOFA@mail.gmail.com>
In-Reply-To: <20251001-b4-pks-history-builtin-v4-1-8e61ddb86317@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> The "wt-status" subsystem is responsible for printing status information
> around the current state of the working tree. This most importantly
> includes information around whether the working tree or the index have
> any changes.
>
> We're about to introduce a new command though where the changes in

Nit: s/though//

> neither of them are actually relevant to us. Instead, what we want is to
> format the changes between two different trees. While it is a little bit
> of a stretch to add this as functionality to _working tree_ status, it
> doesn't make any sense to open-code this functionality, either.
>
> Implement a new function `wt_status_collect_changes_trees()` that diffs
> two trees and formats the status accordingly. This function is not yet
> used, but will be in a subsequent commit.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  wt-status.c | 24 ++++++++++++++++++++++++
>  wt-status.h |  3 +++
>  2 files changed, 27 insertions(+)
>
> diff --git a/wt-status.c b/wt-status.c
> index 8ffe6d3988..b66edbfca6 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -612,6 +612,30 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,
>  	}
>  }
>
> +void wt_status_collect_changes_trees(struct wt_status *s,
> +				     const struct object_id *old_treeish,
> +				     const struct object_id *new_treeish)
> +{

So, my understanding here is that we want to diff two trees
`old_treeish` and `new_treeish` and then finally store the status change
in `wt_status`

> +	struct diff_options opts = { 0 };
> +
> +	repo_diff_setup(s->repo, &opts);
> +	opts.output_format = DIFF_FORMAT_CALLBACK;
> +	opts.format_callback = wt_status_collect_updated_cb;
> +	opts.format_callback_data = s;
> +	opts.detect_rename = s->detect_rename >= 0 ? s->detect_rename : opts.detect_rename;
> +	opts.rename_limit = s->rename_limit >= 0 ? s->rename_limit : opts.rename_limit;
> +	opts.rename_score = s->rename_score >= 0 ? s->rename_score : opts.rename_score;

Curious, why do we need a '>= 0' check here?

> +	opts.flags.recursive = 1;
> +	diff_setup_done(&opts);
> +
>

So first we setup the diff options, with the right callbacks so that the
relevant information is added to the `wt_status`.

> +	diff_tree_oid(old_treeish, new_treeish, "", &opts);
> +	diffcore_std(&opts);
> +	diff_flush(&opts);

This is the part which calls the callback function with the relevant
information and callback data.

> +	wt_status_get_state(s->repo, &s->state, 0);
> +

Based on the list of diff data in `s->change`, we add the status print
information. Okay makes sense.

> +	diff_free(&opts);
> +}
> +
>  static void wt_status_collect_changes_worktree(struct wt_status *s)
>  {
>  	struct rev_info rev;
> diff --git a/wt-status.h b/wt-status.h
> index e40a27214a..924d7a5fa9 100644
> --- a/wt-status.h
> +++ b/wt-status.h
> @@ -153,6 +153,9 @@ void wt_status_add_cut_line(struct wt_status *s);
>  void wt_status_prepare(struct repository *r, struct wt_status *s);
>  void wt_status_print(struct wt_status *s);
>  void wt_status_collect(struct wt_status *s);
> +void wt_status_collect_changes_trees(struct wt_status *s,
> +				     const struct object_id *old_treeish,
> +				     const struct object_id *new_treeish);
>  /*
>   * Frees the buffers allocated by wt_status_collect.
>   */
>
> --
> 2.51.0.700.g236ee7b076.dirty

So this function will be used in an upcoming patch, looks good.