git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCHv3 1/2] wt-status.*: better advices for git status added

From
Junio C Hamano <gitster@pobox.com>
Date
May 29, 2012, 19:22 UTC
Message-ID
<7v4nqy4wrz.fsf@alter.siamese.dyndns.org>
In-Reply-To
<1338226598-16056-1-git-send-email-Lucien.Kong@ensimag.imag.fr>
Kong Lucien <Lucien.Kong@ensimag.imag.fr> writes:
Show 24 quoted lines
> This patch provides new warning messages in the display of
> 'git status' (at the top) during conflicts, rebase, am
> and bisect process. These messages can be shortened by setting
> the new advice.* config key called advice.statushelp to false.
>
> Thus, information about the new advice.* key are added in
> Documentation/config.txt
>
> Also, the test t7060-wt-status.sh is now working with the
> new warning messages.
>
> Signed-off-by: Kong Lucien <Lucien.Kong@ensimag.imag.fr>
> Signed-off-by: Duperray Valentin <Valentin.Duperray@ensimag.imag.fr>
> Signed-off-by: Jonas Franck <Franck.Jonas@ensimag.imag.fr>
> Signed-off-by: Nguy Thomas <Thomas.Nguy@ensimag.imag.fr>
> Signed-off-by: Nguyen Huynh Khoi Nguyen Lucien <Huynh-Khoi-Nguyen.Nguyen@ensimag.imag.fr>
> ---
> The new messages are not shown when using options such as
> -s or --porcelain.It figures the current state by finding 
> the files generated in .git in each case (during an am, 
> a rebase, a bisect etc.). The messages about the current
> situation of the user are always displayed but the advices 
> on what the user needs to do in order to resume a rebase/bisect
> /am/ commit after resolving conflicts can be hidden by setting advice.statushelp to 'false' in the config file.

Don't some of the above explanatory sentences deserve to be in the commit log message proper, not hidden behind the three-dash lines?

Show 21 quoted lines
>  Documentation/config.txt |    4 ++
>  advice.c                 |    2 +
>  advice.h                 |    1 +
>  t/t7060-wtstatus.sh      |    2 +
>  wt-status.c              |  106 ++++++++++++++++++++++++++++++++++++++++++++++
>  wt-status.h              |    1 +
>  6 files changed, 116 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 915cb5a..6504371 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -176,6 +176,10 @@ advice.*::
>  		Advice shown when you used linkgit:git-checkout[1] to
>  		move to the detach HEAD state, to instruct how to create
>  		a local branch after the fact.
> +	statusHelp::
> +		Directions on how to end the current process shown
> +		in the output of linkgit:git-status[1].
> +
>  --

Have you read the existing entries in the same section you are touching to see if the patch result as a whole makes sense and the new entry fits well in the context?

It strikes me odd that this is not listed next to statusHints, and it also makes me wonder if we even need to invent a new one, or it is better to just make the output more verbose when statusHints is not being declined.

Show 34 quoted lines
> diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh
> index b8cb490..d9a1e18 100755
> --- a/t/t7060-wtstatus.sh
> +++ b/t/t7060-wtstatus.sh
> @@ -30,6 +30,8 @@ test_expect_success 'Report new path with conflict' '
>  
>  cat >expect <<EOF
>  # On branch side
> +# You have unmerged paths: fix conflicts and then commit the result.
> +#
>  # Unmerged paths:
>  #   (use "git add/rm <file>..." as appropriate to mark resolution)
>  #
> diff --git a/wt-status.c b/wt-status.c
> index dd6d8c4..093a352 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -23,6 +23,7 @@ static char default_wt_status_colors[][COLOR_MAXLEN] = {
>  	GIT_COLOR_GREEN,  /* WT_STATUS_LOCAL_BRANCH */
>  	GIT_COLOR_RED,    /* WT_STATUS_REMOTE_BRANCH */
>  	GIT_COLOR_NIL,    /* WT_STATUS_ONBRANCH */
> +	GIT_COLOR_NORMAL, /* WT_STATUS_IN_PROGRESS */
>  };
>  
>  static const char *color(int slot, struct wt_status *s)
> @@ -728,6 +729,109 @@ static void wt_status_print_tracking(struct wt_status *s)
>  	color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
>  }
>  
> +static void wt_status_print_in_progress(struct wt_status *s)
> +{
> +	int i;
> +	const char *c = color(WT_STATUS_IN_PROGRESS, s);
> +	struct stat st;
Show 6 quoted lines
> +	int merge_in_progress = 0;
> +	int rebase_state = 0;
> +	int rebase_interactive_state = 0;
> +	int am_state = 0;
> +	int am_wrong_format_state = 0;
> +	int bisect_state = 0;

merge_in_progress is very descriptive, but compared to that, none of the foo_state is.

> +	int unmerged_present = 0;
Show 18 quoted lines
> +
> +	for (i = 0; i < s->change.nr; i++) {
> +		struct wt_status_change_data *d;
> +		d = s->change.items[i].util;
> +		if (d->stagemask) {
> +			unmerged_present = 1;
> +			break;
> +		}
> +	}
> +
> +	if (!stat(git_path("MERGE_HEAD"), &st))
> +		merge_in_progress = 1;
> +	else {
> +		if (!stat(git_path("rebase-apply"), &st)) {
> +			if (!stat(git_path("rebase-apply/applying"), &st))
> +				am_state = 1;
> +				if (!stat(git_path("rebase-apply/patch"), &st) && !(st.st_size))
> +					am_wrong_format_state = 1;

Isn't nesting screwed-up around here? Your indentation suggests you need opening "{" after "if (applying)" and closing "}" before "else" on the next line.

Show 12 quoted lines
> +			else
> +				rebase_state = 1;
> +		}
> +		else {
> +			if (!stat(git_path("rebase-merge"), &st)) {
> +				if (!stat(git_path("rebase-merge/interactive"), &st))
> +					rebase_interactive_state = 1;
> +				else
> +					rebase_state = 1;
> +			}
> +		}
> +	}

Whenever you write "else {" that is immediately followed by "if", re-read what you wrote and think if you can make it "else if () {" to reduce the indentation level. If you still need five levels of indentation, the function you are writing is too complex and is a sign that you can use a helper function.

In this case, I think you could structure it like this:
	if (merge_head) {
        	merge_in_progress++;
	} else if (rebase-apply) {
		if (applying) {
			am_in_progress++;
                        if (patch is empty)
                                ...
		} else {			
			rebase_in_progress++;
		}
	} else if (rebase-merge) {
		if (interactive)
			rebase_i_in_progress++;
		else
			rebase_in_progress++;
	}
> +	if (!stat(git_path("BISECT_LOG"), &st))
> +		bisect_state = 1;
> +
> +	if(merge_in_progress) {
s/if/if /;  Didn't I say this already in my previous message?
Show 8 quoted lines
> +		if (unmerged_present)
> +			status_printf_ln(s, c, _("You have unmerged paths: fix conflicts and then commit the result."));
> +		else
> +			status_printf_ln(s, c, _("You are still merging, run \"git commit\" to conclude merge."));
> +		wt_status_print_trailer(s);
> +	}
> +	else {
> +		if(am_state) {

The same comments on the "else if" cascading and on the missing SP after keyword apply here.

> +			status_printf_ln(s, c, _("You are currently in am progress:"));
-ECANTPARSE.
> +			if(am_wrong_format_state)
> +				status_printf_ln(s, c, _("One of the patches is empty or corrupted !"));
You never checked corrupted; you only detected an empty file.
Show 13 quoted lines
> +			if (advice_status_help) {
> +				status_printf_ln(s, c, _("When you have resolved this problem run \"git am --resolved\"."));
> +				status_printf_ln(s, c, _("If you would prefer to skip this patch, instead run \"git am --skip\"."));
> +				status_printf_ln(s, c, _("To restore the original branch and stop patching run \"git am --abort\"."));
> +			}
> +			wt_status_print_trailer(s);
> +		}
> +
> +		else if (rebase_state || rebase_interactive_state) {
> +			if (unmerged_present) {
> +				status_printf_ln(s, c, _("You are currently rebasing%s"),
> +				advice_status_help
> +				? _(": fix conflicts and then run \"git rebase -- continue\".") : ".");
s/-- /--/;
Show 55 quoted lines
> +				if (advice_status_help) {
> +					status_printf_ln(s, c, _("If you would prefer to skip this patch, instead run \"git rebase --skip\"."));
> +					status_printf_ln(s, c, _("To check out  the original branch and stop rebasing run \"git rebase --abort\"."));
> +				}
> +			}
> +			else {
> +				if (rebase_state)
> +					status_printf_ln(s, c, _("You are currently rebasing: all conflicts fixed%s"),
> +					advice_status_help
> +					? _(": run \"git rebase --continue\".") : ".");
> +				else {
> +					status_printf_ln(s, c, _("You are currently editing a commit during a rebase."));
> +					if (advice_status_help) {
> +						status_printf_ln(s, c, _("You can amend the commit with"));
> +						status_printf_ln(s, c, _("	git commit --amend"));
> +						status_printf_ln(s, c, _("Once you are satisfied with your changes, run"));
> +						status_printf_ln(s, c, _("	git rebase --continue"));
> +					}
> +				}
> +			}
> +			wt_status_print_trailer(s);
> +		}
> +	}
> +
> +	if(bisect_state) {
> +		status_printf_ln(s, c, _("You are currently bisecting."));
> +		if (advice_status_help)
> +		status_printf_ln(s, c, _("To get back to the original branch run \"git bisect reset\""));
> +		wt_status_print_trailer(s);
> +	}
> +}
> +
>  void wt_status_print(struct wt_status *s)
>  {
>  	const char *branch_color = color(WT_STATUS_ONBRANCH, s);
> @@ -750,6 +854,8 @@ void wt_status_print(struct wt_status *s)
>  			wt_status_print_tracking(s);
>  	}
>  
> +	wt_status_print_in_progress(s);
> +
>  	if (s->is_initial) {
>  		status_printf_ln(s, color(WT_STATUS_HEADER, s), "");
>  		status_printf_ln(s, color(WT_STATUS_HEADER, s), _("Initial commit"));
> diff --git a/wt-status.h b/wt-status.h
> index 14aa9f7..0556669 100644
> --- a/wt-status.h
> +++ b/wt-status.h
> @@ -15,6 +15,7 @@ enum color_wt_status {
>  	WT_STATUS_LOCAL_BRANCH,
>  	WT_STATUS_REMOTE_BRANCH,
>  	WT_STATUS_ONBRANCH,
> +	WT_STATUS_IN_PROGRESS,
>  	WT_STATUS_MAXSLOT
>  };
Previous: Matthieu MoyNext: konglu@minatec.inpg.fr
Message 19 of 97 in “t7512-status-warnings.sh : better advices for git status”
  1. t7512-status-warnings.sh : better advices for git statusKong Lucien, May 24, 2012
  2. Matthieu MoyMay 24, 2012
  3. 1/2 wt-status: better advices for git statusKong Lucien, May 26, 2012
  4. 2/2 t7512-status-warnings.sh: better advices for git statusKong Lucien, May 26, 2012
  5. Matthieu MoyMay 27, 2012
  6. Matthieu MoyMay 28, 2012
  7. Nguyen Thai Ngoc DuyMay 26, 2012
  8. NGUY ThomasMay 26, 2012
  9. Matthieu MoyMay 27, 2012
  10. Matthieu MoyMay 27, 2012
  11. Junio C HamanoMay 28, 2012
  12. Matthieu MoyMay 28, 2012
  13. Junio C HamanoMay 30, 2012
  14. konglu@minatec.inpg.frMay 28, 2012
  15. 1/2 wt-status.*: better advices for git status addedKong Lucien, May 28, 2012
  16. 2/2 t7512-status-warnings.sh: better advices for git statusKong Lucien, May 28, 2012
  17. Matthieu MoyMay 28, 2012
  18. Matthieu MoyMay 28, 2012
  19. Junio C HamanoMay 29, 2012
  20. konglu@minatec.inpg.frMay 30, 2012
  21. Junio C HamanoMay 30, 2012
  22. Matthieu MoyMay 31, 2012
  23. 1/3 wt-status.*: better advices for git status addedKong Lucien, May 30, 2012
  24. 2/3 t7512-status-warnings.sh: better advices for git statusKong Lucien, May 30, 2012
  25. 3/3 Advices about 'git rm' during conflicts (unmerged paths) more relevantKong Lucien, May 30, 2012
  26. Junio C HamanoMay 30, 2012
  27. konglu@minatec.inpg.frMay 30, 2012
  28. Matthieu MoyMay 31, 2012
  29. konglu@minatec.inpg.frMay 31, 2012
  30. Junio C HamanoMay 30, 2012
  31. Junio C HamanoMay 30, 2012
  32. Matthieu MoyMay 31, 2012
  33. Matthieu MoyMay 31, 2012
  34. Matthieu MoyMay 31, 2012
  35. Andrew ArdillMay 31, 2012
  36. 1/3 wt-status.*: better advices for git status addedKong Lucien, May 31, 2012
  37. 2/3 t7512-status-help.sh: better advices for git statusKong Lucien, May 31, 2012
  38. 3/3 status: don't suggest "git rm" or "git add" if not appropriateKong Lucien, May 31, 2012
  39. Matthieu MoyJun 1, 2012
  40. Phil HordJun 1, 2012
  41. konglu@minatec.inpg.frJun 1, 2012
  42. Junio C HamanoJun 1, 2012
  43. Junio C HamanoMay 31, 2012
  44. konglu@minatec.inpg.frJun 1, 2012
  45. Matthieu MoyJun 1, 2012
  46. Junio C HamanoJun 1, 2012
  47. Junio C HamanoJun 1, 2012
  48. konglu@minatec.inpg.frJun 1, 2012
  49. Matthieu MoyJun 1, 2012
  50. konglu@minatec.inpg.frJun 1, 2012
  51. Phil HordJun 1, 2012
  52. Junio C HamanoJun 1, 2012
  53. Phil HordJun 4, 2012
  54. 1/4 wt-status.*: better advices for git status addedKong Lucien, Jun 3, 2012
  55. 2/4 t7512-status-help.sh: better advices for git statusKong Lucien, Jun 3, 2012
  56. Junio C HamanoJun 3, 2012
  57. konglu@minatec.inpg.frJun 4, 2012
  58. Junio C HamanoJun 4, 2012
  59. Junio C HamanoJun 4, 2012
  60. 3/4 status: don't suggest "git rm" or "git add" if not appropriateKong Lucien, Jun 3, 2012
  61. Matthieu MoyJun 3, 2012
  62. 4/4 status: better advices when splitting a commit (during rebase -i)Kong Lucien, Jun 3, 2012
  63. Junio C HamanoJun 3, 2012
  64. Phil HordJun 4, 2012
  65. konglu@minatec.inpg.frJun 5, 2012
  66. Junio C HamanoJun 3, 2012
  67. konglu@minatec.inpg.frJun 4, 2012
  68. Junio C HamanoJun 4, 2012
  69. 1/4 wt-status.*: better advices for git status addedKong Lucien, Jun 4, 2012
  70. 2/4 t7512-status-help.sh: better advices for git statusKong Lucien, Jun 4, 2012
  71. Matthieu MoyJun 4, 2012
  72. Junio C HamanoJun 4, 2012
  73. 3/4 status: don't suggest "git rm" or "git add" if not appropriateKong Lucien, Jun 4, 2012
  74. 4/4 status: better advices when splitting a commit (during rebase -i)Kong Lucien, Jun 4, 2012
  75. Junio C HamanoJun 4, 2012
  76. konglu@minatec.inpg.frJun 5, 2012
  77. Matthieu MoyJun 5, 2012
  78. 1/4 wt-status.*: better advices for git status addedLucien Kong, Jun 5, 2012
  79. 2/4 t7512-status-help.sh: better advices for git statusLucien Kong, Jun 5, 2012
  80. 3/4 status: don't suggest "git rm" or "git add" if not appropriateLucien Kong, Jun 5, 2012
  81. 4/4 status: better advices when splitting a commit (during rebase -i)Lucien Kong, Jun 5, 2012
  82. Junio C HamanoJun 5, 2012
  83. konglu@minatec.inpg.frJun 8, 2012
  84. Junio C HamanoJun 8, 2012
  85. 1/4 wt-status.*: better advices for git status addedLucien Kong, Jun 7, 2012
  86. 2/4 t7512-status-help.sh: better advices for git statusLucien Kong, Jun 7, 2012
  87. 3/4 status: don't suggest "git rm" or "git add" if not appropriateLucien Kong, Jun 7, 2012
  88. 4/4 status: better advices when splitting a commit (during rebase -i)Lucien Kong, Jun 7, 2012
  89. Junio C HamanoJun 7, 2012
  90. Junio C HamanoJun 7, 2012
  91. konglu@minatec.inpg.frJun 7, 2012
  92. 1/4 wt-status.*: better advices for git status addedLucien Kong, Jun 10, 2012
  93. 2/4 t7512-status-help.sh: better advices for git statusLucien Kong, Jun 10, 2012
  94. 3/4 status: don't suggest "git rm" or "git add" if not appropriateLucien Kong, Jun 10, 2012
  95. 4/4 status: better advices when splitting a commit (during rebase -i)Lucien Kong, Jun 10, 2012
  96. Junio C HamanoJun 11, 2012
  97. konglu@minatec.inpg.frJun 12, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.