Refactor merge/update git command calls (#23366)

Follow #22568

* Remove unnecessary ToTrustedCmdArgs calls 
    * the FAQ in  #22678
* Quote: When using ToTrustedCmdArgs, the code will be very complex (see
the changes for examples). Then developers and reviewers can know that
something might be unreasonable.
* The `signArg` couldn't be empty, it's either `-S{keyID}` or
`--no-gpg-sign`.
* Use `signKeyID` instead, add comment "empty for no-sign, non-empty to
sign"
* 5-line code could be extracted to a common `NewGitCommandCommit()` to
handle the `signKeyID`, but I think it's not a must, current code is
clear enough.

1 files changed, 7 insertions, 2 deletions

diff --git a/services/pull/merge.go b/services/pull/merge.go
index afa924fc10..12e01e8ce7 100644
--- a/services/pull/merge.go
+++ b/services/pull/merge.go
@@ -332,8 +332,13 @@ func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *use
 }
 
 func commitAndSignNoAuthor(ctx *mergeContext, message string) error {
-	if err := git.NewCommand(ctx, "commit").AddArguments(ctx.signArg...).AddOptionFormat("--message=%s", message).
-		Run(ctx.RunOpts()); err != nil {
+	cmdCommit := git.NewCommand(ctx, "commit").AddOptionFormat("--message=%s", message)
+	if ctx.signKeyID == "" {
+		cmdCommit.AddArguments("--no-gpg-sign")
+	} else {
+		cmdCommit.AddOptionFormat("-S%s", ctx.signKeyID)
+	}
+	if err := cmdCommit.Run(ctx.RunOpts()); err != nil {
 		log.Error("git commit %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String())
 		return fmt.Errorf("git commit %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String())
 	}