From: Phillip Wood <phillip.wood123@gmail.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
Junio C Hamano <gitster@pobox.com>,
Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: [PATCH v2 8/9] [RFC] rebase: fix advice when a fixup creates an empty commit
Date: Fri, 6 Dec 2019 16:06:13 +0000 [thread overview]
Message-ID: <20191206160614.631724-9-phillip.wood123@gmail.com> (raw)
In-Reply-To: <20191206160614.631724-1-phillip.wood123@gmail.com>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Add a specific message to advise the user what to do when a fixup or
squash command would create an empty commit.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
Notes:
I'm slightly nervous of moving the call to determine_whence() to
finalized_deferred_config(). I think it is ok but need to do a more
thorough audit of the code to be sure.
builtin/commit.c | 32 ++++++++++++++++++++++++++++----
sequencer.c | 31 ++++++++++++++++++++++++++++++-
sequencer.h | 3 ++-
t/t3403-rebase-skip.sh | 18 ++++++++++++------
wt-status.h | 5 +++--
5 files changed, 75 insertions(+), 14 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index c3b6b8a6bd..4ae984c470 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -52,6 +52,20 @@ N_("You asked to amend the most recent commit, but doing so would make\n"
"it empty. You can repeat your command with --allow-empty, or you can\n"
"remove the commit entirely with \"git reset HEAD^\".\n");
+static const char empty_rebase_fixup_advice[] =
+N_("The fixup would make the commit empty\n"
+"If you wish to commit it anyway use:\n"
+"\n"
+" git commit --amend --allow-empty\n"
+" git rebase --continue\n"
+"\n"
+"To remove the commit entirely use:\n"
+"\n"
+" git reset HEAD^\n"
+" git rebase --continue\n"
+"\n"
+"Otherwise, please use 'git rebase --skip' to skip it\n");
+
static const char empty_cherry_pick_advice[] =
N_("The previous cherry-pick is now empty, possibly due to conflict resolution.\n"
"If you wish to commit it anyway, use:\n"
@@ -181,8 +195,14 @@ static void determine_whence(struct wt_status *s)
{
if (file_exists(git_path_merge_head(the_repository)))
whence = FROM_MERGE;
- else if (!sequencer_determine_whence(the_repository, &whence))
- whence = FROM_COMMIT;
+ else {
+ int res = sequencer_determine_whence(the_repository, &whence,
+ amend);
+ if (res < 0)
+ die(_("could not read sequencer state"));
+ else if (!res)
+ whence = FROM_COMMIT;
+ }
if (s)
s->whence = whence;
}
@@ -192,7 +212,6 @@ static void status_init_config(struct wt_status *s, config_fn_t fn)
wt_status_prepare(the_repository, s);
init_diff_ui_defaults();
git_config(fn, s);
- determine_whence(s);
s->hints = advice_status_hints; /* must come after git_config() */
}
@@ -943,9 +962,12 @@ 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))) {
+ fprintf(stderr, "\nwhence = %d\n", whence);
s->display_comment_prefix = old_display_comment_prefix;
run_status(stdout, index_file, prefix, 0, s);
- if (amend)
+ if (whence == FROM_REBASE_FIXUP)
+ fputs(_(empty_rebase_fixup_advice), stderr);
+ else if (amend)
fputs(_(empty_amend_advice), stderr);
else if (is_from_cherry_pick(whence) ||
whence == FROM_REBASE_PICK) {
@@ -1114,6 +1136,8 @@ static void finalize_deferred_config(struct wt_status *s)
if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
s->ahead_behind_flags = AHEAD_BEHIND_FULL;
+
+ determine_whence(s);
}
static int parse_and_validate_options(int argc, const char *argv[],
diff --git a/sequencer.c b/sequencer.c
index 6e153fea76..64242f4ce7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -5270,7 +5270,8 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
return 0;
}
-int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
+int sequencer_determine_whence(struct repository *r, enum commit_whence *whence,
+ int amending)
{
if (file_exists(git_path_cherry_pick_head(r))) {
struct object_id cherry_pick_head, rebase_head;
@@ -5286,6 +5287,34 @@ int sequencer_determine_whence(struct repository *r, enum commit_whence *whence)
*whence = FROM_CHERRY_PICK_SINGLE;
return 1;
+ } else if (amending && file_exists(rebase_path_current_fixups()) &&
+ (file_exists(git_path_squash_msg(r)) ||
+ file_exists(git_path_merge_msg(r)))) {
+ /*
+ * If rebase_path_amend() exists the user is running `git
+ * commit`, if not we're committing a fixup/squash directly from
+ * the sequencer
+ */
+ if (file_exists(rebase_path_amend())) {
+ struct strbuf rev = STRBUF_INIT;
+ struct object_id to_amend, head;
+
+ if (get_oid("HEAD", &head))
+ return error(_("amending invalid head")); /* let commit deal with error */
+ if (!read_oneliner(&rev, rebase_path_amend(), 0))
+ return error(_("invalid file: '%s'"),
+ rebase_path_amend());
+ if (get_oid_hex(rev.buf, &to_amend))
+ return error(_("invalid contents: '%s'"),
+ rebase_path_amend());
+ if (oideq(&head, &to_amend)) {
+ *whence = FROM_REBASE_FIXUP;
+ return 1;
+ }
+ } else {
+ *whence = FROM_REBASE_FIXUP;
+ return 1;
+ }
}
return 0;
diff --git a/sequencer.h b/sequencer.h
index 8d451dbfcb..4e631900b4 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -202,5 +202,6 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
void sequencer_post_commit_cleanup(struct repository *r);
int sequencer_get_last_command(struct repository* r,
enum replay_action *action);
-int sequencer_determine_whence(struct repository *r, enum commit_whence *whence);
+int sequencer_determine_whence(struct repository *r, enum commit_whence *whence,
+ int amending);
#endif /* SEQUENCER_H */
diff --git a/t/t3403-rebase-skip.sh b/t/t3403-rebase-skip.sh
index a927774910..bc110b66b3 100755
--- a/t/t3403-rebase-skip.sh
+++ b/t/t3403-rebase-skip.sh
@@ -164,22 +164,28 @@ test_expect_success 'correct advice upon multi cherry-pick picking an empty comm
test_i18ngrep "git cherry-pick --skip" err
'
-test_expect_success 'fixup that empties commit fails' '
+test_expect_success 'correct advice when fixup empties commit' '
test_when_finished "git rebase --abort" &&
(
set_fake_editor &&
test_must_fail env FAKE_LINES="1 fixup 2" git rebase -i \
- goodbye^ reverted-goodbye
- )
+ goodbye^ reverted-goodbye 2>err
+ ) &&
+ test_i18ngrep "git rebase --skip" err &&
+ test_must_fail git commit --amend --no-edit 2>err &&
+ test_i18ngrep "git rebase --skip" err
'
-test_expect_success 'squash that empties commit fails' '
+test_expect_success 'correct advice when squash empties commit' '
test_when_finished "git rebase --abort" &&
(
set_fake_editor &&
test_must_fail env FAKE_LINES="1 squash 2" git rebase -i \
- goodbye^ reverted-goodbye
- )
+ goodbye^ reverted-goodbye 2>err
+ ) &&
+ test_i18ngrep "git rebase --skip" err &&
+ test_must_fail git commit --amend --no-edit 2>err &&
+ test_i18ngrep "git rebase --skip" err
'
# Must be the last test in this file
diff --git a/wt-status.h b/wt-status.h
index 5f81bf7507..a4b7fe6de9 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -41,7 +41,8 @@ enum commit_whence {
FROM_MERGE, /* commit came from merge */
FROM_CHERRY_PICK_SINGLE, /* commit came from cherry-pick */
FROM_CHERRY_PICK_MULTI, /* commit came from a sequence of cherry-picks */
- FROM_REBASE_PICK /* commit came from a pick/reword/edit */
+ FROM_REBASE_PICK, /* commit came from a pick/reword/edit */
+ FROM_REBASE_FIXUP /* commit came from fixup/squash */
};
static inline int is_from_cherry_pick(enum commit_whence whence)
@@ -52,7 +53,7 @@ static inline int is_from_cherry_pick(enum commit_whence whence)
static inline int is_from_rebase(enum commit_whence whence)
{
- return whence == FROM_REBASE_PICK;
+ return whence == FROM_REBASE_PICK || whence == FROM_REBASE_FIXUP;
}
struct wt_status_change_data {
--
2.24.0
next prev parent reply other threads:[~2019-12-06 16:08 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-22 23:30 [PATCH 0/3] commit: fix advice for empty commits during rebases Johannes Schindelin via GitGitGadget
2019-10-22 23:30 ` [PATCH 1/3] cherry-pick: add test for `--skip` advice in `git commit` Johannes Schindelin via GitGitGadget
2019-10-22 23:30 ` [PATCH 2/3] sequencer: export the function to get the path of `.git/rebase-merge/` Johannes Schindelin via GitGitGadget
2019-10-22 23:30 ` [PATCH 3/3] commit: give correct advice for empty commit during a rebase Johannes Schindelin via GitGitGadget
2019-10-23 2:45 ` Junio C Hamano
2019-10-24 10:15 ` Phillip Wood
2019-10-25 11:48 ` Johannes Schindelin
2019-10-25 14:01 ` Phillip Wood
2019-11-08 5:28 ` Junio C Hamano
2019-11-08 14:09 ` Johannes Schindelin
2019-11-11 16:13 ` Phillip Wood
2019-10-23 3:02 ` [PATCH 0/3] commit: fix advice for empty commits during rebases Junio C Hamano
2019-10-25 12:11 ` Johannes Schindelin
2019-10-29 2:05 ` Junio C Hamano
2019-10-29 13:00 ` Johannes Schindelin
2019-12-06 16:06 ` [PATCH v2 0/9] " Phillip Wood
2019-12-06 16:06 ` [PATCH v2 1/9] t3404: use test_cmp_rev Phillip Wood
2019-12-06 17:39 ` Junio C Hamano
2019-12-06 16:06 ` [PATCH v2 2/9] cherry-pick: add test for `--skip` advice in `git commit` Phillip Wood
2019-12-06 16:06 ` [PATCH v2 3/9] cherry-pick: check commit error messages Phillip Wood
2019-12-06 16:06 ` [PATCH v2 4/9] sequencer: write CHERRY_PICK_HEAD for reword and edit Phillip Wood
2019-12-06 16:06 ` [PATCH v2 5/9] commit: use enum value for multiple cherry-picks Phillip Wood
2019-12-06 18:13 ` Junio C Hamano
2019-12-06 16:06 ` [PATCH v2 6/9] commit: encapsulate determine_whence() for sequencer Phillip Wood
2019-12-06 18:24 ` Junio C Hamano
2019-12-18 14:26 ` Phillip Wood
2019-12-06 16:06 ` [PATCH v2 7/9] commit: give correct advice for empty commit during a rebase Phillip Wood
2019-12-06 16:06 ` Phillip Wood [this message]
2020-02-26 19:45 ` [PATCH v2 8/9] [RFC] rebase: fix advice when a fixup creates an empty commit Elijah Newren
2019-12-06 16:06 ` [PATCH v2 9/9] [RFC] rebase -i: leave CHERRY_PICK_HEAD when there are conflicts Phillip Wood
2019-12-18 14:35 ` Phillip Wood
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191206160614.631724-9-phillip.wood123@gmail.com \
--to=phillip.wood123@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=phillip.wood@dunelm.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).