On Fri, Dec 11, 2020 at 01:55:11AM +0100, Stefan Sperling wrote:
> On Fri, Dec 11, 2020 at 01:27:04AM +0100, Stefan Sperling wrote:
> > If the histedit log message editor exits without saving its buffer,
> > we currently throw away log messages of all commits which were folded.
> > Only the last commit message is preserved, which could be something
> > meaningless like "fixup".
> > 
> > This patch makes us preserve the initial editor buffer content as-is.
> > That's not going to be an ideal log message, but doesn't throw away
> > information and stands out visually because the newly created log
> > message will start with a comment like:
> > [[[
> > # log message of folded commit aabbcc00...
> > ]]]
> > 
> > Git's approach is to concatentate all log messages and strip comments.
> > That has other downsides such as resulting in a potentially nonsensical
> > log message.
> > I'd like to stick to a simple "what you see is what you get" approach.
> >  
> > Problem reported by jrick
> > 
> > ok?
> 
> Off-list, jrick suggested to keep the behaviour intact where histedit's
> commit operation is aborted if the saved log message is empty.
> 
> Differentiating between "empty message" and "unchanged message" should
> be a good thing to do in general.
> 
> ok?
> 
> (This is the same change as the previous patch but with the empty log
> message behaviour kept as-is.)

This patch behaves as advertised with histedit -f, but I'm hitting an
unexpected error (got: object <sha1> not found) when using histedit -m
and saving an empty commit message.  On the main branch, under this
test Got will continue through the histedit and does not modify the
original commit message at all.  I think we'd want to make it work
like histedit -f will after this patch, where the histedit is left in
an interrupted state.

> 
> diff a2c1255649ab1b8d0fc66ec6ff07646b6176e764 e559f03aa5e4b408dced5ddbbcfd102692d19a36
> blob - 7bdd9e142a5f26254c73c308684bb6d6da95ef20
> blob + c8dbc08063b2590cb230bbb89b7864078f5b1a4d
> --- got/got.c
> +++ got/got.c
> @@ -448,7 +448,7 @@ edit_logmsg(char **logmsg, const char *editor, const c
>  		return got_error_from_errno("stat");
>  
>  	if (st.st_mtime == st2.st_mtime && st.st_size == st2.st_size)
> -		return got_error_msg(GOT_ERR_COMMIT_MSG_EMPTY,
> +		return got_error_msg(GOT_ERR_COMMIT_MSG_NOCHANGE,
>  		    "no changes made to commit message, aborting");
>  
>  	/*
> @@ -521,7 +521,7 @@ edit_logmsg(char **logmsg, const char *editor, const c
>  		goto done;
>  	}
>  	if (strcmp(*logmsg, initial_content_stripped) == 0)
> -		err = got_error_msg(GOT_ERR_COMMIT_MSG_EMPTY,
> +		err = got_error_msg(GOT_ERR_COMMIT_MSG_NOCHANGE,
>  		    "no changes made to commit message, aborting");
>  done:
>  	free(initial_content_stripped);
> @@ -803,6 +803,7 @@ cmd_import(int argc, char *argv[])
>  		    path_dir, refname);
>  		if (error) {
>  			if (error->code != GOT_ERR_COMMIT_MSG_EMPTY &&
> +			    error->code != GOT_ERR_COMMIT_MSG_NOCHANGE &&
>  			    logmsg_path != NULL)
>  				preserve_logmsg = 1;
>  			goto done;
> @@ -5971,6 +5972,7 @@ add_tag(struct got_repository *repo, struct got_worktr
>  		    tag_name, got_repo_get_path(repo));
>  		if (err) {
>  			if (err->code != GOT_ERR_COMMIT_MSG_EMPTY &&
> +			    err->code != GOT_ERR_COMMIT_MSG_NOCHANGE &&
>  			    tagmsg_path != NULL)
>  				preserve_tagmsg = 1;
>  			goto done;
> @@ -6904,6 +6906,7 @@ cmd_commit(int argc, char *argv[])
>  	    print_status, NULL, repo);
>  	if (error) {
>  		if (error->code != GOT_ERR_COMMIT_MSG_EMPTY &&
> +		    error->code != GOT_ERR_COMMIT_MSG_NOCHANGE &&
>  		    cl_arg.logmsg_path != NULL)
>  			preserve_logmsg = 1;
>  		goto done;
> @@ -8016,9 +8019,12 @@ histedit_edit_logmsg(struct got_histedit_list_entry *h
>  	err = edit_logmsg(&hle->logmsg, editor, logmsg_path, logmsg,
>  	    logmsg_len);
>  	if (err) {
> -		if (err->code != GOT_ERR_COMMIT_MSG_EMPTY)
> +		if (err->code != GOT_ERR_COMMIT_MSG_NOCHANGE)
>  			goto done;
> -		err = got_object_commit_get_logmsg(&hle->logmsg, commit);
> +		err = NULL;
> +		hle->logmsg = strdup(new_msg);
> +		if (hle->logmsg == NULL)
> +			err = got_error_from_errno("strdup");
>  	}
>  done:
>  	if (logmsg_path && unlink(logmsg_path) != 0 && err == NULL)
> blob - 5fa8ffa0d2de4c2d61942eb2eed66a240149f6d0
> blob + cfef35362eff7e295b62a678e352388f674a4b43
> --- include/got_error.h
> +++ include/got_error.h
> @@ -109,7 +109,7 @@
>  #define GOT_ERR_NO_HISTEDIT_CMD	92
>  #define GOT_ERR_HISTEDIT_SYNTAX	93
>  #define GOT_ERR_HISTEDIT_CANCEL	94
> -/* 95 is currently unused */
> +#define GOT_ERR_COMMIT_MSG_NOCHANGE 95
>  #define GOT_ERR_HISTEDIT_BUSY	96
>  #define GOT_ERR_HISTEDIT_CMD	97
>  #define GOT_ERR_HISTEDIT_PATH	98
> @@ -252,7 +252,7 @@ static const struct got_error {
>  	{ GOT_ERR_NO_HISTEDIT_CMD,"no histedit commands provided" },
>  	{ GOT_ERR_HISTEDIT_SYNTAX,"syntax error in histedit command list" },
>  	{ GOT_ERR_HISTEDIT_CANCEL,"histedit operation cancelled" },
> -	{ 95, "unused error code" },
> +	{ GOT_ERR_COMMIT_MSG_NOCHANGE, "no changes made to commit message" },
>  	{ GOT_ERR_HISTEDIT_BUSY,"histedit operation is in progress in this "
>  	    "work tree and must be continued or aborted first" },
>  	{ GOT_ERR_HISTEDIT_CMD, "bad histedit command" },
>