Re: [PATCH 1/1] commit: display advice hints when commit fails

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Heba Waly <heba.waly@xxxxxxxxx>
> 
> Display hints to the user when trying to commit without staging the modified
> files first (when advice.statusHints is set to true). Change the output of the
> unsuccessful commit from e.g:

Wrap your commit messages at 72 characters.

>   # [...]
>   # Changes not staged for commit:
>   #   modified:   builtin/commit.c
>   #
>   # no changes added to commit
> 
> to:
> 
>   # [...]
>   # Changes not staged for commit:
>   #   (use "git add <file>..." to update what will be committed)
>   #   (use "git checkout -- <file>..." to discard changes in working directory)
>   #
>   #   modified:   ../builtin/commit.c

For tidiness, can this line also be "builtin/commit.c" (that is, without
the "../" at the beginning) to match what's before "to:"?

> In ea9882bfc4 (commit: disable status hints when writing to COMMIT_EDITMSG,
> 2013-09-12) the intent was to disable status hints when writing to
> COMMIT_EDITMSG, but in fact the implementation disabled status messages in
> more locations, e.g in case the commit wasn't successful, status hints
> will still be disabled and no hints will be displayed to the user although
> advice.statusHints is set to true.
> 
> Signed-off-by: Heba Waly <heba.waly@xxxxxxxxx>
> ---
>  builtin/commit.c                          | 1 +
>  t/t7500-commit-template-squash-signoff.sh | 9 +++++++++

I wondered if there was a better place to put the test, but I couldn't
find one, so this is fine.

> @@ -961,6 +961,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  	 */
>  	if (!committable && whence != FROM_MERGE && !allow_empty &&
>  	    !(amend && is_a_merge(current_head))) {
> +		s->hints = advice_status_hints;
>  		s->display_comment_prefix = old_display_comment_prefix;
>  		run_status(stdout, index_file, prefix, 0, s);
>  		if (amend)

I checked that this undoing of "s->hints = 0" is safe, because s is no
longer used in this function nor in the calling function cmd_commit()
(which is the one that declared s locally).

Still probably worth a comment, though. For example:

  This status is to be printed to stdout, so hints will be useful to the
  user. Reset s->hints to what the user configured.

The corresponding comment on "s->hints = 0" might need to be tweaked,
too, but I can't think of anything at the moment.

> +test_expect_success 'commit without staging files fails and displays hints' '
> +	echo "initial" >>file &&
> +	git add file &&
> +	git commit -m initial &&
> +	echo "changes" >>file &&
> +	test_must_fail git commit -m initial >actual &&

Use another commit message for this, since this is no longer "initial".
(Maybe "after initial" or something like that.)



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux