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

[PATCH 07/18] revert: Eliminate global "commit" variable

From
Ramkumar Ramachandra <artagnon@gmail.com>
Date
Jul 19, 2011, 17:17 UTC
Message-ID
<1311095876-3098-8-git-send-email-artagnon@gmail.com>
In-Reply-To
<1311095876-3098-1-git-send-email-artagnon@gmail.com>

Functions which act on commits currently rely on a file-scope static variable to be set before they're called. Consequently, the API and corresponding callsites are ugly and unclear. Remove this variable and change their API to accept the commit to act on as additional argument so that the callsites change from looking like

commit = prepare_a_commit(); act_on_commit();

to looking like

commit = prepare_a_commit(); act_on_commit(commit);

This change is also in line with our long-term goal of exposing some of these functions through a public API.

Inspired-by: Christian Couder <chriscool@tuxfamily.org>
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/revert.c |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c
index 5dbea40..ab8dcd3 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -37,7 +37,6 @@ static const char * const cherry_pick_usage[] = {
 
 static int edit, record_origin, no_commit, mainline, signoff, allow_ff;
 static enum { REVERT, CHERRY_PICK } action;
-static struct commit *commit;
 static int commit_argc;
 static const char **commit_argv;
 static int allow_rerere_auto;
@@ -116,25 +115,25 @@ struct commit_message {
 	const char *message;
 };
 
-static int get_message(const char *raw_message, struct commit_message *out)
+static int get_message(struct commit *commit, struct commit_message *out)
 {
 	const char *encoding;
 	const char *abbrev, *subject;
 	int abbrev_len, subject_len;
 	char *q;
 
-	if (!raw_message)
+	if (!commit->buffer)
 		return -1;
-	encoding = get_encoding(raw_message);
+	encoding = get_encoding(commit->buffer);
 	if (!encoding)
 		encoding = "UTF-8";
 	if (!git_commit_encoding)
 		git_commit_encoding = "UTF-8";
 
 	out->reencoded_message = NULL;
-	out->message = raw_message;
+	out->message = commit->buffer;
 	if (strcmp(encoding, git_commit_encoding))
-		out->reencoded_message = reencode_string(raw_message,
+		out->reencoded_message = reencode_string(commit->buffer,
 					git_commit_encoding, encoding);
 	if (out->reencoded_message)
 		out->message = out->reencoded_message;
@@ -182,7 +181,7 @@ static char *get_encoding(const char *message)
 	return NULL;
 }
 
-static void write_cherry_pick_head(void)
+static void write_cherry_pick_head(struct commit *commit)
 {
 	int fd;
 	struct strbuf buf = STRBUF_INIT;
@@ -350,7 +349,7 @@ static int run_git_commit(const char *defmsg)
 	return run_command_v_opt(args, RUN_GIT_CMD);
 }
 
-static int do_pick_commit(void)
+static int do_pick_commit(struct commit *commit)
 {
 	unsigned char head[20];
 	struct commit *base, *next, *parent;
@@ -412,7 +411,7 @@ static int do_pick_commit(void)
 		return error(_("%s: cannot parse parent commit %s"),
 			me, sha1_to_hex(parent->object.sha1));
 
-	if (get_message(commit->buffer, &msg) != 0)
+	if (get_message(commit, &msg) != 0)
 		return error(_("Cannot get commit message for %s"),
 			sha1_to_hex(commit->object.sha1));
 
@@ -465,7 +464,7 @@ static int do_pick_commit(void)
 			strbuf_addstr(&msgbuf, ")\n");
 		}
 		if (!no_commit)
-			write_cherry_pick_head();
+			write_cherry_pick_head(commit);
 	}
 
 	if (!strategy || !strcmp(strategy, "recursive") || action == REVERT) {
@@ -543,6 +542,7 @@ static void read_and_refresh_cache(const char *me)
 static int revert_or_cherry_pick(int argc, const char **argv)
 {
 	struct rev_info revs;
+	struct commit *commit;
 
 	git_config(git_default_config, NULL);
 	me = action == REVERT ? "revert" : "cherry-pick";
@@ -565,7 +565,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	prepare_revs(&revs);
 
 	while ((commit = get_revision(&revs))) {
-		int res = do_pick_commit();
+		int res = do_pick_commit(commit);
 		if (res)
 			return res;
 	}

-- 
1.7.4.rc1.7.g2cf08.dirty
Previous: Ramkumar RamachandraNext: Ramkumar Ramachandra
Message 18 of 34 in “[GSoC update] Sequencer for inclusion v2”
  1. Ramkumar RamachandraJul 19, 2011
  2. 01/18 advice: Introduce error_resolve_conflictRamkumar Ramachandra, Jul 19, 2011
  3. Phil HordJul 21, 2011
  4. Jeff KingJul 22, 2011
  5. Ramkumar RamachandraJul 24, 2011
  6. Junio C HamanoJul 24, 2011
  7. Phil HordJul 25, 2011
  8. 02/18 config: Introduce functions to write non-standard fileRamkumar Ramachandra, Jul 19, 2011
  9. Jeff KingJul 19, 2011
  10. Jonathan NiederJul 19, 2011
  11. Junio C HamanoJul 19, 2011
  12. Jeff KingJul 19, 2011
  13. Ramkumar RamachandraJul 24, 2011
  14. 03/18 revert: Simplify and inline add_message_to_msgRamkumar Ramachandra, Jul 19, 2011
  15. 04/18 revert: Don't check lone argument in get_encodingRamkumar Ramachandra, Jul 19, 2011
  16. 05/18 revert: Rename no_replay to record_originRamkumar Ramachandra, Jul 19, 2011
  17. 06/18 revert: Propogate errors upwards from do_pick_commitRamkumar Ramachandra, Jul 19, 2011
  18. 07/18 revert: Eliminate global "commit" variableRamkumar Ramachandra, Jul 19, 2011
  19. 08/18 revert: Introduce struct to keep command-line optionsRamkumar Ramachandra, Jul 19, 2011
  20. 09/18 revert: Separate cmdline parsing from functional codeRamkumar Ramachandra, Jul 19, 2011
  21. 10/18 revert: Don't create invalid replay_opts in parse_argsRamkumar Ramachandra, Jul 19, 2011
  22. 11/18 revert: Save data for continuing after conflict resolutionRamkumar Ramachandra, Jul 19, 2011
  23. 12/18 revert: Save command-line options for continuing operationRamkumar Ramachandra, Jul 19, 2011
  24. 13/18 revert: Make pick_commits functionally act on a commit listRamkumar Ramachandra, Jul 19, 2011
  25. 14/18 revert: Introduce --reset to remove sequencer stateRamkumar Ramachandra, Jul 19, 2011
  26. 15/18 reset: Make reset remove the sequencer stateRamkumar Ramachandra, Jul 19, 2011
  27. 16/18 revert: Remove sequencer state when no commits are pendingRamkumar Ramachandra, Jul 19, 2011
  28. 17/18 revert: Don't implictly stomp pending sequencer operationRamkumar Ramachandra, Jul 19, 2011
  29. 18/18 revert: Introduce --continue to continue the operationRamkumar Ramachandra, Jul 19, 2011
  30. Junio C HamanoJul 19, 2011
  31. Ramkumar RamachandraJul 20, 2011
  32. Junio C HamanoJul 19, 2011
  33. Ramkumar RamachandraJul 24, 2011
  34. Junio C HamanoJul 24, 2011

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.