From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 21 Oct 2025 11:43:34 GMT
Subject: Re: [PATCH v4 01/12] wt-status: provide function to expose status for trees
Message-ID: <aPdx5heE7blfnX3e@pks.im>
In-Reply-To: <CAOLa=ZTUDOYb1KVUEZY4KFyoaP-PdXjBvUto0vxOtYanEjoOFA@mail.gmail.com>

On Tue, Oct 14, 2025 at 04:49:14AM -0400, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > 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`

Exactly.

> > +	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?

I'm mostly just mirroring the same behaviour that we already have in
`wt_status_collect_chanegs_worktree()`.

Patrick