Re: [PATCH v4 2/8] status: cleanup API to wt_status_print

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 3, 2016, 21:36 UTC
Message-ID: <xmqqr3a57cxm.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1470147137-17498-3-git-send-email-git@jeffhostetler.com>

Jeff Hostetler <git@jeffhostetler.com> writes:

Show 12 quoted lines

> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> Refactor the API between builtin/commit.c and wt-status.[ch].
> Hide details of the various wt_*status_print() routines inside
> wt-status.c behind a single (new) wt_status_print() routine
> and eliminate the switch statements from builtin/commit.c
>
> This will allow us to more easily add new status formats
> and isolate that within wt-status.c
>
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> Signed-off-by: Jeff Hostetler <git@jeffhostetler.com>

Again, are these the same person? I won't repeat this for the remainder of the series.

Show 16 quoted lines

> diff --git a/wt-status.h b/wt-status.h
> index 2023a3c..a859a12 100644
> --- a/wt-status.h
> +++ b/wt-status.h
> @@ -43,6 +43,15 @@ struct wt_status_change_data {
>  	unsigned new_submodule_commits : 1;
>  };
>  
> + enum wt_status_format {
> +	STATUS_FORMAT_NONE = 0,
> +	STATUS_FORMAT_LONG,
> +	STATUS_FORMAT_SHORT,
> +	STATUS_FORMAT_PORCELAIN,
> +
> +	STATUS_FORMAT_UNSPECIFIED
> + };

Is it your editor, or is it your MUA? This definition is indented by one SP, which is funny.

Also throughout the series, I saw a handful of blank lines that should be empty but are not (e.g. three tabs and nothing else on a new line). I've fixed them up all but I won't be sending an interdiff just for them, so please make sure they won't resurface when/if you reroll.

Show 28 quoted lines

>  struct wt_status {
>  	int is_initial;
>  	char *branch;
> @@ -66,6 +75,8 @@ struct wt_status {
>  	int show_branch;
>  	int hints;
>  
> +	enum wt_status_format status_format;
> +
>  	/* These are computed during processing of the individual sections */
>  	int commitable;
>  	int workdir_dirty;
> @@ -99,6 +110,7 @@ struct wt_status_state {
>  void wt_status_truncate_message_at_cut_line(struct strbuf *);
>  void wt_status_add_cut_line(FILE *fp);
>  void wt_status_prepare(struct wt_status *s);
> +void wt_status_print(struct wt_status *s);
>  void wt_status_collect(struct wt_status *s);
>  void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
>  int wt_status_check_rebase(const struct worktree *wt,
> @@ -106,10 +118,6 @@ int wt_status_check_rebase(const struct worktree *wt,
>  int wt_status_check_bisect(const struct worktree *wt,
>  			   struct wt_status_state *state);
>  
> -void wt_longstatus_print(struct wt_status *s);
> -void wt_shortstatus_print(struct wt_status *s);
> -void wt_porcelain_print(struct wt_status *s);
> -

I think I agreed during the previous review round that shifting the division of labor boundary between the command and wt-status.[ch] this way, to have the latter do more printing, is a good idea, and I still do.

Thanks.