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

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

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 3, 2012, 21:06 UTC
Message-ID
<7v7gvoyuk4.fsf@alter.siamese.dyndns.org>
In-Reply-To
<1338748217-16440-1-git-send-email-Lucien.Kong@ensimag.imag.fr>
Kong Lucien <Lucien.Kong@ensimag.imag.fr> writes:
Show 12 quoted lines
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 915cb5a..670945d 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -162,6 +162,10 @@ advice.*::
>  		Directions on how to stage/unstage/add shown in the
>  		output of linkgit:git-status[1] and the template shown
>  		when writing commit messages.
> +		Show directions on how to proceed from the current
> +		state in the output of linkgit:git-status[1] and in
> +		the template shown when writing commit messages in
> +		linkgit:git-commit[1].
I meant these four lines to _replace_, not _add_.

Reading the three lines we can see in the context before this hunk, don't you agree that they are now unnecessary because what they say is merely a subset of the four lines you added say?

Show 10 quoted lines
> diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh
> index b8cb490..61d1f38 100755
> --- a/t/t7060-wtstatus.sh
> +++ b/t/t7060-wtstatus.sh
> @@ -118,4 +121,68 @@ test_expect_success 'git diff-index --cached -C shows 2 copies + 1 unmerged' '
> ...
> +test_expect_success 'status when conflicts with add and rm advice (both deleted)' '
> +	git init git &&
> +	cd git &&
> +	test_commit init main.txt init &&
Please do not chdir around outside a subshell.

"This is the last test in this script" is not an excuse, as that will forbid others from improving the script by adding new ones later.

Show 18 quoted lines
> diff --git a/wt-status.c b/wt-status.c
> index dd6d8c4..2460e20 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -728,6 +729,169 @@ static void wt_status_print_tracking(struct wt_status *s)
> ...
> +static void show_merge_in_progress(struct wt_status *s,
> +				struct wt_status_state *state,
> +				const char *color)
> +{
> +	if (has_unmerged(s)) {
> +		status_printf_ln(s, color, _("You have unmerged paths."));
> +		if (advice_status_hints)
> +			status_printf_ln(s, color,
> +				_("  (fix conflicts and run \"git commit\")"));
> +	} else {
> +		status_printf_ln(s, color,
> +			_("All conflicts fixed but you are still merging."));
Thanks for rephrasing; this reads much easier.
Show 9 quoted lines
> +static void show_am_in_progress(struct wt_status *s,
> +				struct wt_status_state *state,
> +				const char *color)
> +{
> +	status_printf_ln(s, color,
> +		_("You are in the middle of an am session."));
> +	if (state->am_empty_patch) {
> +		status_printf_ln(s, color,
> +			_("The current patch is empty; run \"git am --skip\" to skip it."));
Isn't everything after "; " an advice?
Show 11 quoted lines
> +		if (advice_status_hints)
> +			status_printf_ln(s, color,
> +				_("  (use \"git am --abort\" to restore the original branch)"));
> +	} else if (advice_status_hints) {
> +		status_printf_ln(s, color,
> +			_("  (when you have fixed this problem run \"git am --resolved\")"));
> +		status_printf_ln(s, color,
> +			_("  (use \"git am --skip\" to skip this patch)"));
> +		status_printf_ln(s, color,
> +			_("  (use \"git am --abort\" to restore the original branch)"));
> +	}
I think the structure can simply be:
	if (am_empty_patch)
		"The current patch is empty.";
	if (advice_status_hints) {
		"  (use --abort to restore)";
		"  (use --skip to skip)";
                if (!am_empty_patch)
			"  (use --resolved after you are done)";
	}

Note that I am not suggesting to change the wording of the message in the above; just showing the flow-structure.

Also when you are in the middle of "git am -3", there may be unmerged entries in the index; do we want to suggest "add/rm" to resolve to fill in the missing detail of "fix" in your "when you have fixed this problem" message?

We may want to decide how detailed we want to make the help texts; the same fuzziness exists in "fix conflicts and then" at the beginning of show_rebase_in_progress().

Show 9 quoted lines
> +static void show_rebase_in_progress(struct wt_status *s,
> +				struct wt_status_state *state,
> +				const char *color)
> +{
> +	if (has_unmerged(s)) {
> +		status_printf_ln(s, color, _("You are currently rebasing."));
> +		if (advice_status_hints) {
> +			status_printf_ln(s, color,
> +				_("  (fix conflicts and then run \"git rebase --continue\")"));
Show 18 quoted lines
> +			status_printf_ln(s, color,
> +				_("  (use \"git rebase --skip\" to skip this patch)"));
> +			status_printf_ln(s, color,
> +				_("  (use \"git rebase --abort\" to check out the original branch)"));
> +		}
> +	} else if (state->rebase_in_progress) {
> +		status_printf_ln(s, color, _("You are currently rebasing."));
> +		if (advice_status_hints)
> +			status_printf_ln(s, color,
> +				_("  (all conflicts fixed: run \"git rebase --continue\")"));
> +	} else {
> +		status_printf_ln(s, color, _("You are currently editing a commit during a rebase."));
> +		if (advice_status_hints && !s->amend) {
> +			status_printf_ln(s, color,
> +				_("  (use \"git commit --amend\" to amend the current commit)"));
> +			status_printf_ln(s, color,
> +				_("  (use \"git rebase --continue\" once you are satisfied with your changes)"));
> +		}

This last "else" block is taken when running "rebase -i" and there is no longer any unmerged index entry. I wonder if the message from the first printf_ln needs to be clarified further depending on the context.

Specifically, in this sequence:
	- the user marked a commit as "pick";
        - replaying of that commit resulted in conflicts;
        - the user edited files and used add/rm to resolve conflicts;
        - the user did one of these:
	  1. "git status"
          2. "git commit" without "--amend"
          3. "git commit --amend"
can this message come up?

In such a case, "You are currently editing a commit" is actively wrong. The user has finished resolving the conflict and are about to record the result. Also, "git status" and "git commit" without "--amend" are both sensible commands in this situation, but running "git commit --amend" is likely to be a mistake, no?

	Side note: I am not absolutely sure if "--amend" is always a
	mistake in this situation; I'd very much appreciate users
	who creatively use "rebase -i" in real life to offer valid
	uses of "commit --amend" in this scenario.
Show 15 quoted lines
> +static void show_cherry_pick_in_progress(struct wt_status *s,
> +					struct wt_status_state *state,
> +					const char *color)
> +{
> +	if (has_unmerged(s)) {
> +		status_printf_ln(s, color, _("You are currently cherry-picking."));
> +		if (advice_status_hints)
> +			status_printf_ln(s, color,
> +				_("  (fix conflicts and run \"git commit\")"));
> +	} else {
> +		status_printf_ln(s, color, _("You are currently cherry-picking."));
> +		if (advice_status_hints)
> +			status_printf_ln(s, color,
> +				_("  (all conflicts fixed: run \"git commit\")"));
> +	}

The current status is the same in either arm of if/else; shouldn't they be lifted outside, i.e.

	"You are currently cherry-picking";
        if (advice_status_hints) {
        	if (has_unmerged(s))
			"  (fix conflicts ...)";
		else
                	"  (all fixed, run ...)";
	}

The rest of this patch I did not quote looked all very much sensible.

Thanks.
Previous: konglu@minatec.inpg.frNext: konglu@minatec.inpg.fr
Message 66 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.