From patchwork Thu May 30 13:43:49 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13680430 Received: from mail-wm1-f50.google.com (mail-wm1-f50.google.com [209.85.128.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BF62D17C7CE for ; Thu, 30 May 2024 13:43:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.50 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717076637; cv=none; b=OdIljfWyvmrXxo71alRe7pblz1umILVmWz2FaC6HjF+wPldssMc9UaLjnqxglgpkE2Lakt2JrJsfDpGEeFeknKJEdChnT4edeS7XiBItnnNE2GdOhfTlqUcbuzFp3TVpeu0T7eP0NIeVGDWWQa2Wkla/HOjLVUbhUw64QhNI10w= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717076637; c=relaxed/simple; bh=KCjic7s/AG/mE9szmLmxEu3WvZgn3u332WudbOZy7Gc=; h=Message-Id:In-Reply-To:References:From:Date:Subject:Content-Type: MIME-Version:To:Cc; b=R+FeDDLr0+XvwMwn5YFQ4BiUejC0M9JRkHGT1Exzhg3pZbcLkBwSlf3asTyiIRsWQvCiUDhNP3vr/34538aZQ9F4fBX3XugNGLQBLbNiHl52gtyigbmk4/qizGNIGB+QTpxvY2q4vDOf3rHgHBVawhAwX05OxWrCmo+KTc/wEU4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=O8DZKCK6; arc=none smtp.client-ip=209.85.128.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="O8DZKCK6" Received: by mail-wm1-f50.google.com with SMTP id 5b1f17b1804b1-421124a0b37so5679155e9.1 for ; Thu, 30 May 2024 06:43:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1717076633; x=1717681433; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=i4GKCP92169KMUszhC1hGw6XJz77a76xJIAVsemElz0=; b=O8DZKCK6BeoDDmeDx26YBBPOC06eywzAUc5qKIXj6i0jXN/Xlxq/QvmvuuCJV3271T bDUTegZGp2uhy4247I3v4EWkyEqDFXWisGMJRLfv4p1nT2+kv/mC+3xotIHOfMGp2Ank bM/+O6fI7TrjNhz/0rpRaxx3PK3RK7p5fZ9rY6KAKcY76SWl8bRG8hNMQ1eSgLfPTrTg +llcJJTIqoO2HvUNCNTc/gZAk1ajuJ/dD7bS6CK0Ls0tI0NudF1Dc5SQVHsKbAaBuhEW OKIS9o/G3xqtrZVfgL4n+RwpdjQuiLlN+pGnB7nJ2ddJryEY6Sm2cJymrSbx81ZIwdzi QssA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717076633; x=1717681433; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=i4GKCP92169KMUszhC1hGw6XJz77a76xJIAVsemElz0=; b=t6pa2eXwywwK1eJIQkjY9dLGjk6/ODZzFwWhFn03nBhRUBfrazi2jQi0x973tN0g52 PI12oavKVVgGOHk+xQ8CfPeW1tZpDINP8Sq96F/uK1psKIq/Dwy7g6CiU0Ufq2PbeNOb I6fldxIW4+VCku5qNIf6M3VtKLK2wGXJ+STMC+vCZlnKKexD4W3+YUpiU/0ot9njaYXt ekR0Ec2qCLoKs+YGwOF5Q/kfHknuSnN3NLH+CpJHUnuCXZlQPpw17IzDqXw/8PhX8WjY 6E0J4p1d4sVHge3KtKj92HM6Pl6RMZCvm2DqZcfWv4Jc1TOFZIIBj7Zk7UToMtFbSKNy LQ4Q== X-Gm-Message-State: AOJu0YwcZlveYwV26oAhuCvjXqlZaZMYAnJ8uMn1uHbHtIzSRWpFkJU0 91CkrrGavGxkZbInmmQGqwsfOmAnomx/Eh/2NL67I0zJ0wWYdsqt+1/afQ== X-Google-Smtp-Source: AGHT+IEWmrqNwOXHG36jvFjubxUHRegZAV3AMZsMMIimH8I94sbeGdyELwXsS3Tet7BP3FVVryrweg== X-Received: by 2002:a05:600c:5487:b0:421:20d:e3a4 with SMTP id 5b1f17b1804b1-4212781a881mr28668565e9.17.1717076633489; Thu, 30 May 2024 06:43:53 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-42127061df9sm25631365e9.18.2024.05.30.06.43.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 May 2024 06:43:53 -0700 (PDT) Message-Id: <91c6f2f1b458fca8db19483cd43229b737214d3d.1717076630.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 30 May 2024 13:43:49 +0000 Subject: [PATCH v3 1/2] rebase -i: pass struct replay_opts to parse_insn_line() Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Stefan Haller , Johannes Schindelin , Patrick Steinhardt , =?utf-8?b?UnViw6lu?= Justo , Phillip Wood , Phillip Wood , Phillip Wood From: Phillip Wood From: Phillip Wood This new parameter will be used in the next commit. As adding the parameter requires quite a few changes to plumb it through the call chain these are separated into their own commit to avoid cluttering up the next commit with incidental changes. Signed-off-by: Phillip Wood --- builtin/rebase.c | 17 +++++++++++------ rebase-interactive.c | 21 +++++++++++++-------- rebase-interactive.h | 9 ++++++--- sequencer.c | 22 ++++++++++++---------- sequencer.h | 4 ++-- 5 files changed, 44 insertions(+), 29 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 0466d9414af..9fb0a6888d6 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -193,7 +193,7 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) return replay; } -static int edit_todo_file(unsigned flags) +static int edit_todo_file(unsigned flags, struct replay_opts *opts) { const char *todo_file = rebase_path_todo(); struct todo_list todo_list = TODO_LIST_INIT, @@ -204,7 +204,8 @@ static int edit_todo_file(unsigned flags) return error_errno(_("could not read '%s'."), todo_file); strbuf_stripspace(&todo_list.buf, comment_line_str); - res = edit_todo_list(the_repository, &todo_list, &new_todo, NULL, NULL, flags); + res = edit_todo_list(the_repository, opts, &todo_list, &new_todo, + NULL, NULL, flags); if (!res && todo_list_write_to_file(the_repository, &new_todo, todo_file, NULL, NULL, -1, flags & ~(TODO_LIST_SHORTEN_IDS))) res = error_errno(_("could not write '%s'"), todo_file); @@ -295,8 +296,8 @@ static int do_interactive_rebase(struct rebase_options *opts, unsigned flags) error(_("could not generate todo list")); else { discard_index(the_repository->index); - if (todo_list_parse_insn_buffer(the_repository, todo_list.buf.buf, - &todo_list)) + if (todo_list_parse_insn_buffer(the_repository, &replay, + todo_list.buf.buf, &todo_list)) BUG("unusable todo list"); ret = complete_action(the_repository, &replay, flags, @@ -351,9 +352,13 @@ static int run_sequencer_rebase(struct rebase_options *opts) replay_opts_release(&replay_opts); break; } - case ACTION_EDIT_TODO: - ret = edit_todo_file(flags); + case ACTION_EDIT_TODO: { + struct replay_opts replay_opts = get_replay_opts(opts); + + ret = edit_todo_file(flags, &replay_opts); + replay_opts_release(&replay_opts); break; + } case ACTION_SHOW_CURRENT_PATCH: { struct child_process cmd = CHILD_PROCESS_INIT; diff --git a/rebase-interactive.c b/rebase-interactive.c index c343e16fcdd..56fd7206a95 100644 --- a/rebase-interactive.c +++ b/rebase-interactive.c @@ -101,9 +101,10 @@ void append_todo_help(int command_count, strbuf_add_commented_lines(buf, msg, strlen(msg), comment_line_str); } -int edit_todo_list(struct repository *r, struct todo_list *todo_list, - struct todo_list *new_todo, const char *shortrevisions, - const char *shortonto, unsigned flags) +int edit_todo_list(struct repository *r, struct replay_opts *opts, + struct todo_list *todo_list, struct todo_list *new_todo, + const char *shortrevisions, const char *shortonto, + unsigned flags) { const char *todo_file = rebase_path_todo(), *todo_backup = rebase_path_todo_backup(); @@ -114,7 +115,9 @@ int edit_todo_list(struct repository *r, struct todo_list *todo_list, * it. If there is an error, we do not return, because the user * might want to fix it in the first place. */ if (!initial) - incorrect = todo_list_parse_insn_buffer(r, todo_list->buf.buf, todo_list) | + incorrect = todo_list_parse_insn_buffer(r, opts, + todo_list->buf.buf, + todo_list) | file_exists(rebase_path_dropped()); if (todo_list_write_to_file(r, todo_list, todo_file, shortrevisions, shortonto, @@ -134,13 +137,13 @@ int edit_todo_list(struct repository *r, struct todo_list *todo_list, if (initial && new_todo->buf.len == 0) return -3; - if (todo_list_parse_insn_buffer(r, new_todo->buf.buf, new_todo)) { + if (todo_list_parse_insn_buffer(r, opts, new_todo->buf.buf, new_todo)) { fprintf(stderr, _(edit_todo_list_advice)); return -4; } if (incorrect) { - if (todo_list_check_against_backup(r, new_todo)) { + if (todo_list_check_against_backup(r, opts, new_todo)) { write_file(rebase_path_dropped(), "%s", ""); return -4; } @@ -228,13 +231,15 @@ int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo) return res; } -int todo_list_check_against_backup(struct repository *r, struct todo_list *todo_list) +int todo_list_check_against_backup(struct repository *r, + struct replay_opts *opts, + struct todo_list *todo_list) { struct todo_list backup = TODO_LIST_INIT; int res = 0; if (strbuf_read_file(&backup.buf, rebase_path_todo_backup(), 0) > 0) { - todo_list_parse_insn_buffer(r, backup.buf.buf, &backup); + todo_list_parse_insn_buffer(r, opts, backup.buf.buf, &backup); res = todo_list_check(&backup, todo_list); } diff --git a/rebase-interactive.h b/rebase-interactive.h index 7239c60f791..8e5b181b334 100644 --- a/rebase-interactive.h +++ b/rebase-interactive.h @@ -3,17 +3,20 @@ struct strbuf; struct repository; +struct replay_opts; struct todo_list; void append_todo_help(int command_count, const char *shortrevisions, const char *shortonto, struct strbuf *buf); -int edit_todo_list(struct repository *r, struct todo_list *todo_list, - struct todo_list *new_todo, const char *shortrevisions, - const char *shortonto, unsigned flags); +int edit_todo_list(struct repository *r, struct replay_opts *opts, + struct todo_list *todo_list, struct todo_list *new_todo, + const char *shortrevisions, const char *shortonto, + unsigned flags); int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo); int todo_list_check_against_backup(struct repository *r, + struct replay_opts *opts, struct todo_list *todo_list); #endif diff --git a/sequencer.c b/sequencer.c index aa2a2398357..7ab465da14a 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2626,8 +2626,9 @@ static int check_label_or_ref_arg(enum todo_command command, const char *arg) return 0; } -static int parse_insn_line(struct repository *r, struct todo_item *item, - const char *buf, const char *bol, char *eol) +static int parse_insn_line(struct repository *r, struct replay_opts *opts UNUSED, + struct todo_item *item, const char *buf, + const char *bol, char *eol) { struct object_id commit_oid; char *end_of_object_name; @@ -2761,8 +2762,8 @@ int sequencer_get_last_command(struct repository *r UNUSED, enum replay_action * return ret; } -int todo_list_parse_insn_buffer(struct repository *r, char *buf, - struct todo_list *todo_list) +int todo_list_parse_insn_buffer(struct repository *r, struct replay_opts *opts, + char *buf, struct todo_list *todo_list) { struct todo_item *item; char *p = buf, *next_p; @@ -2780,7 +2781,7 @@ int todo_list_parse_insn_buffer(struct repository *r, char *buf, item = append_new_todo(todo_list); item->offset_in_buf = p - todo_list->buf.buf; - if (parse_insn_line(r, item, buf, p, eol)) { + if (parse_insn_line(r, opts, item, buf, p, eol)) { res = error(_("invalid line %d: %.*s"), i, (int)(eol - p), p); item->command = TODO_COMMENT + 1; @@ -2930,7 +2931,7 @@ static int read_populate_todo(struct repository *r, if (strbuf_read_file_or_whine(&todo_list->buf, todo_file) < 0) return -1; - res = todo_list_parse_insn_buffer(r, todo_list->buf.buf, todo_list); + res = todo_list_parse_insn_buffer(r, opts, todo_list->buf.buf, todo_list); if (res) { if (is_rebase_i(opts)) return error(_("please fix this using " @@ -2960,7 +2961,7 @@ static int read_populate_todo(struct repository *r, struct todo_list done = TODO_LIST_INIT; if (strbuf_read_file(&done.buf, rebase_path_done(), 0) > 0 && - !todo_list_parse_insn_buffer(r, done.buf.buf, &done)) + !todo_list_parse_insn_buffer(r, opts, done.buf.buf, &done)) todo_list->done_nr = count_commands(&done); else todo_list->done_nr = 0; @@ -5319,7 +5320,8 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts) goto release_todo_list; if (file_exists(rebase_path_dropped())) { - if ((res = todo_list_check_against_backup(r, &todo_list))) + if ((res = todo_list_check_against_backup(r, opts, + &todo_list))) goto release_todo_list; unlink(rebase_path_dropped()); @@ -6363,7 +6365,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla return error(_("nothing to do")); } - res = edit_todo_list(r, todo_list, &new_todo, shortrevisions, + res = edit_todo_list(r, opts, todo_list, &new_todo, shortrevisions, shortonto, flags); if (res == -1) return -1; @@ -6391,7 +6393,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla strbuf_release(&buf2); /* Nothing is done yet, and we're reparsing, so let's reset the count */ new_todo.total_nr = 0; - if (todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo) < 0) + if (todo_list_parse_insn_buffer(r, opts, new_todo.buf.buf, &new_todo) < 0) BUG("invalid todo list after expanding IDs:\n%s", new_todo.buf.buf); diff --git a/sequencer.h b/sequencer.h index a309ddd712b..304ba4b4d35 100644 --- a/sequencer.h +++ b/sequencer.h @@ -136,8 +136,8 @@ struct todo_list { .buf = STRBUF_INIT, \ } -int todo_list_parse_insn_buffer(struct repository *r, char *buf, - struct todo_list *todo_list); +int todo_list_parse_insn_buffer(struct repository *r, struct replay_opts *opts, + char *buf, struct todo_list *todo_list); int todo_list_write_to_file(struct repository *r, struct todo_list *todo_list, const char *file, const char *shortrevisions, const char *shortonto, int num, unsigned flags); From patchwork Thu May 30 13:43:50 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Phillip Wood X-Patchwork-Id: 13680431 Received: from mail-wr1-f46.google.com (mail-wr1-f46.google.com [209.85.221.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 596B217C9F7 for ; Thu, 30 May 2024 13:43:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.46 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717076638; cv=none; b=b4era7dLXTOqQw2ZKCSXnTjTfKERgBpjJoOsk+eIc2Vbh9uvgloiwNGKHM1foigoUneb9d4x03sGzI7O8PwfpNTTwzem/mIg9NqK8HFAJbCusiN+8wCoq4YOsNRZxOGxO/RIYlTG2ZUkoWVkGPXE8PQTzs5RhrvG82RaE9u+nJw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717076638; c=relaxed/simple; bh=ZNZLd+skaTak7Qg1XsfYufC7C9JhpIlUsvlPHXVnXTw=; h=Message-Id:In-Reply-To:References:From:Date:Subject:Content-Type: MIME-Version:To:Cc; b=gWKk2OdTXnnboZ5QW/85s3hH1o04qvNOTrjr+mtxNpToFLDxnIlzhvTzz7d7SlHj6gA/ZDrYnai/hBgqZ9xuZxKI1KRUeKXL3SIHTBDIq5GG498QXUcAmp3go6KXUQfJd1xBInl17t/XNczkRWLJ9BS6lXzR9KcNoy3iHzZ89Eg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=QvaEKp5Z; arc=none smtp.client-ip=209.85.221.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="QvaEKp5Z" Received: by mail-wr1-f46.google.com with SMTP id ffacd0b85a97d-354f14bd80cso596322f8f.1 for ; Thu, 30 May 2024 06:43:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1717076634; x=1717681434; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=GiegT6MHJWRZf/ZHUjF7tc4JVjQVHslRBVlUDHzwZQU=; b=QvaEKp5ZbHWZFbojJX/dpKrd0xqRdfaCIYGyumRw4r34W+2wpVJ24uxMjYW5Dm6iy8 dZkowFuf74g6viTOCEe2P1WLhOMdqUE9enQIh9iJAYyJzJ1L9xQ9MtE0VDSogrhRdOdw yE7V6APbr9KBKtJjL1fP6DHVwwPHxTb+e3pG/ViS/wa1AUbtYrefCTl6ISn0DjIJrfRN 94LvX0XPVeehrN4lzj76vy8xJ5z8PEmwG4VKuDch17YwvJsrRy7Y4HpDQv5XfLx2mEXv xFAnkRSzs+9mgbOAV2XNp7E7E7r+2DNqjpRiWVRo8PylB6l7C0O65sQptqjfS2qPwkqa rNKQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717076634; x=1717681434; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=GiegT6MHJWRZf/ZHUjF7tc4JVjQVHslRBVlUDHzwZQU=; b=p0S20cEBR/oaXNGdwQzQobaMr6rCKylb8O/iBky+zv3+w/Qj5e7rEKcN3R0yI8SRL+ +288u0M0xW/05cbXwFZhuH1z/DpIbKjHX4KeaNgy5/MOlts+FEHNKev0BcGY3huHvAdz QlwQGbgevoWnvDX+mYkkwv7JfxQZrzJhNNlKpcrHTrFqaNTHv9fSGNHNnrhwQTAzH6nL 3o1XzRp/Nyv9S802qeiQMUt0U8XiK2mole2XJUFHDYZfist8sOm3KO/gdZG+y2zeEUow +uUm/zY4uuqxH37uDGS2r38iBR03GoG2TddW4W8Jhmo+1Ccboxt5tbj64ycycQLgB/w/ es9w== X-Gm-Message-State: AOJu0Yxe63QV9ru+o8OB4TU0oq0KWIhjXUk2sMcZrwcZBn3V3LEo1HLp uJj94G4TaVgIEdYJ/VjGDCOBvMFZprvLv+Ovcr/NAtIUhCbWJU/JKLlOUw== X-Google-Smtp-Source: AGHT+IFdxS/Hiy2L15VJqGwIaWv4kMCuzQA6TJ6jdNVjlBFuuniSJ6ipzzERl49Ue04FwFZcbOvxiQ== X-Received: by 2002:adf:a309:0:b0:354:be7c:954 with SMTP id ffacd0b85a97d-35dc00919fbmr1644619f8f.15.1717076634178; Thu, 30 May 2024 06:43:54 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-358c0acf7d5sm12110579f8f.7.2024.05.30.06.43.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 May 2024 06:43:53 -0700 (PDT) Message-Id: <86052416b22143a5c71ad011ca8d6e4cf80bcc50.1717076630.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Thu, 30 May 2024 13:43:50 +0000 Subject: [PATCH v3 2/2] rebase -i: improve error message when picking merge Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Stefan Haller , Johannes Schindelin , Patrick Steinhardt , =?utf-8?b?UnViw6lu?= Justo , Phillip Wood , Phillip Wood , Phillip Wood From: Phillip Wood From: Phillip Wood The only todo commands that accept a merge commit are "merge" and "reset". All the other commands like "pick" or "reword" fail when they try to pick a a merge commit and print the message error: commit abc123 is a merge but no -m option was given. followed by a hint about the command being rescheduled. This message is designed to help the user when they cherry-pick a merge and forget to pass "-m". For users who are rebasing the message is confusing as there is no way for rebase to cherry-pick the merge. Improve the user experience by detecting the error and printing some advice on how to fix it when the todo list is parsed rather than waiting for the "pick" command to fail. The advice recommends "merge" rather than "exec git cherry-pick -m ..." on the assumption that cherry-picking merges is relatively rare and it is more likely that the user chose "pick" by a mistake. It would be possible to support cherry-picking merges by allowing the user to pass "-m" to "pick" commands but that adds complexity to do something that can already be achieved with exec git cherry-pick -m1 abc123 Reported-by: Stefan Haller Signed-off-by: Phillip Wood --- Documentation/config/advice.txt | 2 ++ advice.c | 1 + advice.h | 1 + sequencer.c | 63 +++++++++++++++++++++++++++++++-- t/t3404-rebase-interactive.sh | 45 +++++++++++++++++++++++ 5 files changed, 110 insertions(+), 2 deletions(-) diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt index 0e35ae5240f..fa612417568 100644 --- a/Documentation/config/advice.txt +++ b/Documentation/config/advice.txt @@ -96,6 +96,8 @@ advice.*:: `pushNonFFCurrent`, `pushNonFFMatching`, `pushAlreadyExists`, `pushFetchFirst`, `pushNeedsForce`, and `pushRefNeedsUpdate` simultaneously. + rebaseTodoError:: + Shown when there is an error after editing the rebase todo list. refSyntax:: Shown when the user provides an illegal ref name, to tell the user about the ref syntax documentation. diff --git a/advice.c b/advice.c index 0a122c2020b..558a46fc0b3 100644 --- a/advice.c +++ b/advice.c @@ -70,6 +70,7 @@ static struct { [ADVICE_PUSH_UNQUALIFIED_REF_NAME] = { "pushUnqualifiedRefName" }, [ADVICE_PUSH_UPDATE_REJECTED] = { "pushUpdateRejected" }, [ADVICE_PUSH_UPDATE_REJECTED_ALIAS] = { "pushNonFastForward" }, /* backwards compatibility */ + [ADVICE_REBASE_TODO_ERROR] = { "rebaseTodoError" }, [ADVICE_REF_SYNTAX] = { "refSyntax" }, [ADVICE_RESET_NO_REFRESH_WARNING] = { "resetNoRefresh" }, [ADVICE_RESOLVE_CONFLICT] = { "resolveConflict" }, diff --git a/advice.h b/advice.h index c8d29f97f39..5105d90129d 100644 --- a/advice.h +++ b/advice.h @@ -37,6 +37,7 @@ enum advice_type { ADVICE_PUSH_UNQUALIFIED_REF_NAME, ADVICE_PUSH_UPDATE_REJECTED, ADVICE_PUSH_UPDATE_REJECTED_ALIAS, + ADVICE_REBASE_TODO_ERROR, ADVICE_REF_SYNTAX, ADVICE_RESET_NO_REFRESH_WARNING, ADVICE_RESOLVE_CONFLICT, diff --git a/sequencer.c b/sequencer.c index 7ab465da14a..ee209c46417 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2626,7 +2626,61 @@ static int check_label_or_ref_arg(enum todo_command command, const char *arg) return 0; } -static int parse_insn_line(struct repository *r, struct replay_opts *opts UNUSED, +static int check_merge_commit_insn(enum todo_command command) +{ + switch(command) { + case TODO_PICK: + error(_("'%s' does not accept merge commits"), + todo_command_info[command].str); + advise_if_enabled(ADVICE_REBASE_TODO_ERROR, _( + /* + * TRANSLATORS: 'pick' and 'merge -C' should not be + * translated. + */ + "'pick' does not take a merge commit. If you wanted to\n" + "replay the merge, use 'merge -C' on the commit.")); + return -1; + + case TODO_REWORD: + error(_("'%s' does not accept merge commits"), + todo_command_info[command].str); + advise_if_enabled(ADVICE_REBASE_TODO_ERROR, _( + /* + * TRANSLATORS: 'reword' and 'merge -c' should not be + * translated. + */ + "'reword' does not take a merge commit. If you wanted to\n" + "replay the merge and reword the commit message, use\n" + "'merge -c' on the commit")); + return -1; + + case TODO_EDIT: + error(_("'%s' does not accept merge commits"), + todo_command_info[command].str); + advise_if_enabled(ADVICE_REBASE_TODO_ERROR, _( + /* + * TRANSLATORS: 'edit', 'merge -C' and 'break' should + * not be translated. + */ + "'edit' does not take a merge commit. If you wanted to\n" + "replay the merge, use 'merge -C' on the commit, and then\n" + "'break' to give the control back to you so that you can\n" + "do 'git commit --amend && git rebase --continue'.")); + return -1; + + case TODO_FIXUP: + case TODO_SQUASH: + return error(_("cannot squash merge commit into another commit")); + + case TODO_MERGE: + return 0; + + default: + BUG("unexpected todo_command"); + } +} + +static int parse_insn_line(struct repository *r, struct replay_opts *opts, struct todo_item *item, const char *buf, const char *bol, char *eol) { @@ -2732,7 +2786,12 @@ static int parse_insn_line(struct repository *r, struct replay_opts *opts UNUSED return status; item->commit = lookup_commit_reference(r, &commit_oid); - return item->commit ? 0 : -1; + if (!item->commit) + return -1; + if (is_rebase_i(opts) && + item->commit->parents && item->commit->parents->next) + return check_merge_commit_insn(item->command); + return 0; } int sequencer_get_last_command(struct repository *r UNUSED, enum replay_action *action) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index d1bead61fad..f92baad1381 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -2215,6 +2215,51 @@ test_expect_success 'bad labels and refs rejected when parsing todo list' ' test_path_is_missing execed ' +test_expect_success 'non-merge commands reject merge commits' ' + test_when_finished "test_might_fail git rebase --abort" && + git checkout E && + git merge I && + oid=$(git rev-parse HEAD) && + cat >todo <<-EOF && + pick $oid + reword $oid + edit $oid + fixup $oid + squash $oid + EOF + ( + set_replace_editor todo && + test_must_fail git rebase -i HEAD 2>actual + ) && + cat >expect <<-EOF && + error: ${SQ}pick${SQ} does not accept merge commits + hint: ${SQ}pick${SQ} does not take a merge commit. If you wanted to + hint: replay the merge, use ${SQ}merge -C${SQ} on the commit. + hint: Disable this message with "git config advice.rebaseTodoError false" + error: invalid line 1: pick $oid + error: ${SQ}reword${SQ} does not accept merge commits + hint: ${SQ}reword${SQ} does not take a merge commit. If you wanted to + hint: replay the merge and reword the commit message, use + hint: ${SQ}merge -c${SQ} on the commit + hint: Disable this message with "git config advice.rebaseTodoError false" + error: invalid line 2: reword $oid + error: ${SQ}edit${SQ} does not accept merge commits + hint: ${SQ}edit${SQ} does not take a merge commit. If you wanted to + hint: replay the merge, use ${SQ}merge -C${SQ} on the commit, and then + hint: ${SQ}break${SQ} to give the control back to you so that you can + hint: do ${SQ}git commit --amend && git rebase --continue${SQ}. + hint: Disable this message with "git config advice.rebaseTodoError false" + error: invalid line 3: edit $oid + error: cannot squash merge commit into another commit + error: invalid line 4: fixup $oid + error: cannot squash merge commit into another commit + error: invalid line 5: squash $oid + You can fix this with ${SQ}git rebase --edit-todo${SQ} and then run ${SQ}git rebase --continue${SQ}. + Or you can abort the rebase with ${SQ}git rebase --abort${SQ}. + EOF + test_cmp expect actual +' + # This must be the last test in this file test_expect_success '$EDITOR and friends are unchanged' ' test_editor_unchanged