git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v5] commit: restore --edit when combined with --fixup

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Aug 17, 2021, 10:06 UTC
Message-ID
<24356b20-45e2-5d60-434c-4f04d1b2055c@gmail.com>
In-Reply-To
<pull.1014.v5.git.1628977230247.gitgitgadget@gmail.com>
Hi Joel
On 14/08/2021 22:40, Joel Klinghed via GitGitGadget wrote:
Show 8 quoted lines
> From: Joel Klinghed <the_jk@spawned.biz>
> 
> Recent changes to --fixup, adding amend suboption, caused the
> --edit flag to be ignored as use_editor was always set to zero.
> 
> Restore edit_flag having higher priority than fixup_message when
> deciding the value of use_editor by moving the edit flag condition
> later in the method.
This version looks good, thanks for revising it
Best Wishes
Phillip
Show 142 quoted lines
> 
> Signed-off-by: Joel Klinghed <the_jk@spawned.biz>
> ---
>      commit: restore --edit when combined with --fixup
>      
>      Recent changes to --fixup, adding amend suboption, caused the --edit
>      flag to be ignored as use_editor was always set to zero.
>      
>      Restore edit_flag having higher priority than fixup_message when
>      deciding the value of use_editor by only changing the default if
>      edit_flag is not set.
>      
>      Changes since v1: Added test verifying that --fixup --edit brings up
>      editor.
>      
>      Changes since v2: Clarify if condition and use write_script helper in
>      test.
>      
>      Changes since v3: Simplify test.
>      
>      Changes since v4: Using cleaner fix by Phillip Wood instead and added an
>      explicit verification to a test for --fixup without --edit.
>      
>      Signed-off-by: Joel Klinghed the_jk@spawned.biz
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1014%2Fthejk%2Ffixup_edit-v5
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1014/thejk/fixup_edit-v5
> Pull-Request: https://github.com/gitgitgadget/git/pull/1014
> 
> Range-diff vs v4:
> 
>   1:  0c0cb647e03 ! 1:  1c608daf0cd commit: restore --edit when combined with --fixup
>       @@ Commit message
>            --edit flag to be ignored as use_editor was always set to zero.
>        
>            Restore edit_flag having higher priority than fixup_message when
>       -    deciding the value of use_editor by only changing the default
>       -    if edit_flag is not set.
>       +    deciding the value of use_editor by moving the edit flag condition
>       +    later in the method.
>        
>            Signed-off-by: Joel Klinghed <the_jk@spawned.biz>
>        
>         ## builtin/commit.c ##
>        @@ builtin/commit.c: static int parse_and_validate_options(int argc, const char *argv[],
>       - 		} else {
>       - 			fixup_commit = fixup_message;
>       - 			fixup_prefix = "fixup";
>       --			use_editor = 0;
>       -+			if (edit_flag < 0)
>       -+				use_editor = 0;
>       +
>       + 	if (logfile || have_option_m || use_message)
>       + 		use_editor = 0;
>       +-	if (0 <= edit_flag)
>       +-		use_editor = edit_flag;
>       +
>       + 	/* Sanity check options */
>       + 	if (amend && !current_head)
>       +@@ builtin/commit.c: static int parse_and_validate_options(int argc, const char *argv[],
>         		}
>         	}
>         
>       ++	if (0 <= edit_flag)
>       ++		use_editor = edit_flag;
>       ++
>       + 	cleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);
>       +
>       + 	handle_untracked_files_arg(s);
>        
>         ## t/t7500-commit-template-squash-signoff.sh ##
>       +@@ t/t7500-commit-template-squash-signoff.sh: EOF
>       +
>       + test_expect_success 'commit --fixup provides correct one-line commit message' '
>       + 	commit_for_rebase_autosquash_setup &&
>       +-	git commit --fixup HEAD~1 &&
>       ++	EDITOR="echo ignored >>" git commit --fixup HEAD~1 &&
>       + 	commit_msg_is "fixup! target message subject line"
>       + '
>       +
>        @@ t/t7500-commit-template-squash-signoff.sh: test_expect_success 'commit --fixup -m"something" -m"extra"' '
>         
>         extra"
> 
> 
>   builtin/commit.c                          | 5 +++--
>   t/t7500-commit-template-squash-signoff.sh | 9 ++++++++-
>   2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 190d215d43b..854903ad113 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1246,8 +1246,6 @@ static int parse_and_validate_options(int argc, const char *argv[],
>   
>   	if (logfile || have_option_m || use_message)
>   		use_editor = 0;
> -	if (0 <= edit_flag)
> -		use_editor = edit_flag;
>   
>   	/* Sanity check options */
>   	if (amend && !current_head)
> @@ -1337,6 +1335,9 @@ static int parse_and_validate_options(int argc, const char *argv[],
>   		}
>   	}
>   
> +	if (0 <= edit_flag)
> +		use_editor = edit_flag;
> +
>   	cleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);
>   
>   	handle_untracked_files_arg(s);
> diff --git a/t/t7500-commit-template-squash-signoff.sh b/t/t7500-commit-template-squash-signoff.sh
> index 7d02f79c0de..8515736003a 100755
> --- a/t/t7500-commit-template-squash-signoff.sh
> +++ b/t/t7500-commit-template-squash-signoff.sh
> @@ -270,7 +270,7 @@ EOF
>   
>   test_expect_success 'commit --fixup provides correct one-line commit message' '
>   	commit_for_rebase_autosquash_setup &&
> -	git commit --fixup HEAD~1 &&
> +	EDITOR="echo ignored >>" git commit --fixup HEAD~1 &&
>   	commit_msg_is "fixup! target message subject line"
>   '
>   
> @@ -281,6 +281,13 @@ test_expect_success 'commit --fixup -m"something" -m"extra"' '
>   
>   extra"
>   '
> +test_expect_success 'commit --fixup --edit' '
> +	commit_for_rebase_autosquash_setup &&
> +	EDITOR="printf \"something\nextra\" >>" git commit --fixup HEAD~1 --edit &&
> +	commit_msg_is "fixup! target message subject linesomething
> +extra"
> +'
> +
>   get_commit_msg () {
>   	rev="$1" &&
>   	git log -1 --pretty=format:"%B" "$rev"
> 
> base-commit: ebf3c04b262aa27fbb97f8a0156c2347fecafafb
> 
Previous: Joel Klinghed via GitGitGadget
Message 19 of 19 in “commit: restore --edit when combined with --fixup”
  1. commit: restore --edit when combined with --fixupJoel Klinghed via GitGitGadget, Aug 11, 2021
  2. Jeff KingAug 11, 2021
  3. Joel KlinghedAug 11, 2021
  4. Jeff KingAug 11, 2021
  5. brian m. carlsonAug 11, 2021
  6. commit: restore --edit when combined with --fixupJoel Klinghed via GitGitGadget, Aug 11, 2021
  7. Junio C HamanoAug 12, 2021
  8. Joel KlinghedAug 12, 2021
  9. Junio C HamanoAug 12, 2021
  10. commit: restore --edit when combined with --fixupJoel Klinghed via GitGitGadget, Aug 12, 2021
  11. Phillip WoodAug 12, 2021
  12. Joel KlinghedAug 12, 2021
  13. Phillip WoodAug 13, 2021
  14. Joel KlinghedAug 13, 2021
  15. Phillip WoodAug 14, 2021
  16. Joel KlinghedAug 14, 2021
  17. commit: restore --edit when combined with --fixupJoel Klinghed via GitGitGadget, Aug 12, 2021
  18. commit: restore --edit when combined with --fixupJoel Klinghed via GitGitGadget, Aug 14, 2021
  19. Phillip WoodAug 17, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.