Subject: Re: [PATCH 3/3] wt-status.c: Set the commitable flag in the collect phase.

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 31 Aug 2018 09:23:47 +0200

On Fri, Aug 31 2018, Stephen P. Smith wrote:

> In an update to fix a bug with "commit --dry-run" it was found that
> the commitable flag was broken.  The update was, at the time,
> accepted as it was better than the previous version.

What update is this? I.e. git.git commit id? See the "or this invocation
of `git show`" part of SubmittingPatches for how to quote it in the
commit message.

> Since the set of the flag had been done in wt_longstatus_print_updated,
> set the flag in wt_status_collect_updated_cb.
>
> Set the commitable flag in wt_status_collect_changes_initial to keep
> from introducing a rebase regression.
>
> Leave the setting of the commitable flag in show_merge_in_progress. If
> a check for merged commits is moved to the collect phase then other
> --dry-run tests fail.
>
> Signed-off-by: Stephen P. Smith <[email-masked]>
> ---
>  wt-status.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/wt-status.c b/wt-status.c
> index 5ffab6101..d50798425 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -540,10 +540,12 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,
>  			/* Leave {mode,oid}_head zero for an add. */
>  			d->mode_index = p->two->mode;
>  			oidcpy(&d->oid_index, &p->two->oid);
> +			s->commitable = 1;
>  			break;
>  		case DIFF_STATUS_DELETED:
>  			d->mode_head = p->one->mode;
>  			oidcpy(&d->oid_head, &p->one->oid);
> +			s->commitable = 1;
>  			/* Leave {mode,oid}_index zero for a delete. */
>  			break;
>
> @@ -561,6 +563,7 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,
>  			d->mode_index = p->two->mode;
>  			oidcpy(&d->oid_head, &p->one->oid);
>  			oidcpy(&d->oid_index, &p->two->oid);
> +			s->commitable = 1;
>  			break;
>  		case DIFF_STATUS_UNMERGED:
>  			d->stagemask = unmerged_mask(p->two->path);
> @@ -665,11 +668,13 @@ static void wt_status_collect_changes_initial(struct wt_status *s)
>  			 * code will output the stage values directly and not use the
>  			 * values in these fields.
>  			 */
> +			s->commitable = 1;
>  		} else {
>  			d->index_status = DIFF_STATUS_ADDED;
>  			/* Leave {mode,oid}_head zero for adds. */
>  			d->mode_index = ce->ce_mode;
>  			oidcpy(&d->oid_index, &ce->oid);
> +			s->commitable = 1;
>  		}
>  	}
>  }
> @@ -773,7 +778,6 @@ static void wt_longstatus_print_updated(struct wt_status *s)
>  			continue;
>  		if (!shown_header) {
>  			wt_longstatus_print_cached_header(s);
> -			s->commitable = 1;
>  			shown_header = 1;
>  		}
>  		wt_longstatus_print_change_data(s, WT_STATUS_UPDATED, it);

This looks sensible, but I'm not familiar with the status code.

Structurally, re: my comment on 1/3 and 2/3, it would make sense to make
this a two-part series. In 1/2 you add the test you're adding in 2/3 as
a test_expect_failure test, and in 2/2 (this commit) you tweak all the
"test_expect_failure" that now pass to "test_expect_success".