Re: [PATCH 1/1] commit: display advice hints when commit fails
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Dec 18, 2019, 03:13 UTC
- Message-ID
- <20191218031338.203382-1-jonathantanmy@google.com>
- In-Reply-To
- <f23477c5a32e5d638310024194040146026972b8.1576574242.git.gitgitgadget@gmail.com>
Show 5 quoted lines
> From: Heba Waly <heba.waly@gmail.com> > > 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.
Show 14 quoted lines
> # [...] > # 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:"?
Show 11 quoted lines
> 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@gmail.com> > --- > 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.
Show 8 quoted lines
> @@ -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.
Show 6 quoted lines
> +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.)