All of lore.kernel.org
 help / color / mirror / Atom feed
From: Denton Liu <liu.denton@gmail.com>
To: git@vger.kernel.org
Cc: Phillip Wood <phillip.wood123@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 0/3] cherry-pick/revert cleanup scissors fix
Date: Wed, 6 Mar 2019 22:44:21 -0800	[thread overview]
Message-ID: <cover.1551940635.git.liu.denton@gmail.com> (raw)
In-Reply-To: <cover.1551867827.git.liu.denton@gmail.com>

As I stated earlier[1], I discovered today that git revert (and thus,
git cherry-pick as well) do not place the scissors line properly as
well. This patchset adds a scissors line when conflicts arise in both
cherry-pick and revert, which fixes the same bug present in git-merge
earlier.

This patchset should apply on top of dl/merge-cleanup-scissors-fix.

Changes since v1:

* Address Phillip's concern of calling get_cleanup_mode with use_editor = 1;
  only set to 1 if we are calling for revert or cherry-pick, else 0 to maintain
  original behaviour (for rebasing).

* Do not die if provided an invalid cleanup option, instead just warn and
  fallback to default option.

[1]: https://public-inbox.org/git/20190306014143.GA2580@dev-l/


Denton Liu (3):
  t3507: cleanup space after redirection operators
  cherry-pick/revert: add scissors line on merge conflict
  sequencer.c: don't die on invalid cleanup_arg

 Documentation/git-cherry-pick.txt |   7 ++
 Documentation/git-revert.txt      |   7 ++
 builtin/merge.c                   |  13 +---
 builtin/rebase--helper.c          |   2 +-
 builtin/revert.c                  |   5 ++
 sequencer.c                       |  39 +++++-----
 sequencer.h                       |   3 +-
 t/t3507-cherry-pick-conflict.sh   | 122 +++++++++++++++++++++++++-----
 8 files changed, 149 insertions(+), 49 deletions(-)

Range-diff against v1:
1:  70a508ca0b ! 1:  8fdc5bfb15 cherry-pick/revert: add scissors line on merge conflict
    @@ -8,11 +8,11 @@
     
         Note that the removal of the if-else tower in git_sequencer_config may
         appear to be a no-op refactor but it actually isn't. First of all, we
    -    now accept "default" as a configuration option and also we die on an
    -    invalid cleanup mode. Most importantly, though, if
    +    now accept "default". More importantly, though, if
         commit.cleanup = scissors, the cleanup enum will be set to
    -    COMMIT_MSG_CLEANUP_SCISSORS instead of COMMIT_MSG_CLEANUP_SPACE. This
    -    allows us to append scissors to MERGE_MSG in the case of a conflict.
    +    COMMIT_MSG_CLEANUP_SCISSORS instead of COMMIT_MSG_CLEANUP_SPACE when we
    +    are reverting or cherry-picking. This allows us to append scissors to
    +    MERGE_MSG in the case of a conflict.
     
         Signed-off-by: Denton Liu <liu.denton@gmail.com>
     
    @@ -77,6 +77,22 @@
      	strbuf_release(&msgbuf);
      	fclose(fp);
     
    + diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
    + --- a/builtin/rebase--helper.c
    + +++ b/builtin/rebase--helper.c
    +@@
    + 		OPT_END()
    + 	};
    + 
    ++	opts.action = REPLAY_INTERACTIVE_REBASE;
    + 	sequencer_init_config(&opts);
    + 	git_config_get_bool("rebase.abbreviatecommands", &abbreviate_commands);
    + 
    +-	opts.action = REPLAY_INTERACTIVE_REBASE;
    + 	opts.allow_ff = 1;
    + 	opts.allow_empty = 1;
    + 
    +
      diff --git a/builtin/revert.c b/builtin/revert.c
      --- a/builtin/revert.c
      +++ b/builtin/revert.c
    @@ -108,6 +124,18 @@
      diff --git a/sequencer.c b/sequencer.c
      --- a/sequencer.c
      +++ b/sequencer.c
    +@@
    + static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts")
    + static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate")
    + 
    ++static inline int is_rebase_i(const struct replay_opts *opts)
    ++{
    ++	return opts->action == REPLAY_INTERACTIVE_REBASE;
    ++}
    ++
    + static int git_sequencer_config(const char *k, const char *v, void *cb)
    + {
    + 	struct replay_opts *opts = cb;
     @@
      		if (status)
      			return status;
    @@ -123,10 +151,22 @@
     -		else
     -			warning(_("invalid commit message cleanup mode '%s'"),
     -				  s);
    -+		opts->default_msg_cleanup = get_cleanup_mode(s, 1);
    ++		opts->default_msg_cleanup = get_cleanup_mode(s, !is_rebase_i(opts));
      
      		free((char *)s);
      		return status;
    +@@
    + 	git_config(git_sequencer_config, opts);
    + }
    + 
    +-static inline int is_rebase_i(const struct replay_opts *opts)
    +-{
    +-	return opts->action == REPLAY_INTERACTIVE_REBASE;
    +-}
    +-
    + static const char *get_dir(const struct replay_opts *opts)
    + {
    + 	if (is_rebase_i(opts))
     @@
      		die(_("Invalid cleanup mode %s"), cleanup_arg);
      }
-:  ---------- > 2:  f3af8000ae sequencer.c: don't die on invalid cleanup_arg
-- 
2.21.0.368.gbf248417d7


  parent reply	other threads:[~2019-03-07  6:44 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-06 10:30 [PATCH 0/2] cherry-pick/revert cleanup scissors fix Denton Liu
2019-03-06 10:30 ` [PATCH 1/2] t3507: cleanup space after redirection operators Denton Liu
2019-03-06 10:30 ` [PATCH 2/2] cherry-pick/revert: add scissors line on merge conflict Denton Liu
2019-03-06 16:29   ` Phillip Wood
2019-03-07  0:04     ` Denton Liu
2019-03-07  0:16     ` Denton Liu
2019-03-07  6:44 ` Denton Liu [this message]
2019-03-07  6:44   ` [PATCH v2 1/3] t3507: cleanup space after redirection operators Denton Liu
2019-03-07  7:34     ` Junio C Hamano
2019-03-07  6:44   ` [PATCH v2 2/3] cherry-pick/revert: add scissors line on merge conflict Denton Liu
2019-03-07  7:52     ` Junio C Hamano
2019-03-07  9:19       ` Denton Liu
2019-03-07  6:44   ` [PATCH v2 3/3] sequencer.c: don't die on invalid cleanup_arg Denton Liu
2019-03-07  8:01     ` Junio C Hamano
2019-03-07  9:58   ` [PATCH v3 0/4] cherry-pick/revert cleanup scissors fix Denton Liu
2019-03-07  9:58     ` [PATCH v3 1/4] merge-options.txt: correct typo Denton Liu
2019-03-08  3:40       ` Junio C Hamano
2019-03-07  9:58     ` [PATCH v3 2/4] t3507: cleanup space after redirection operators Denton Liu
2019-03-07  9:58     ` [PATCH v3 3/4] cherry-pick/revert: add scissors line on merge conflict Denton Liu
2019-03-07 15:24       ` Phillip Wood
2019-03-07 17:56         ` Denton Liu
2019-03-07 18:36           ` Phillip Wood
2019-03-08  0:09             ` Junio C Hamano
2019-03-07  9:58     ` [PATCH v3 4/4] sequencer.c: don't die on invalid cleanup_arg Denton Liu
2019-03-07 15:21       ` 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=cover.1551940635.git.liu.denton@gmail.com \
    --to=liu.denton@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood123@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.