threads / rfc / 39447

RFC patchcreate a skeleton for the command git rebase --status

Subject: [PATCH/RFC] create a skeleton for the command git rebase --status

## tl;dr

3 messages between May 28, 2015 and May 28, 2015. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Guillaume Pagès· May 28, 2015, 12:08 UTC · lore

It is an almost empty code I send to validate the global architecture of this command. I choose to write it in C because git status is already in C and it seems that it is the current tendency to port shell code to C. Moreover I would like to use code from wt_status to implement this functionnality. I wrote a helper that I call from shell script, as it is made in bisect (bisect--helper.c).

Signed-off-by: Guillaume Pagès <guillaume.pages@ensimag.grenoble-inp.fr>
---
 Makefile         | 2 ++
 builtin.h        | 1 +
 git-rebase.sh    | 7 ++++++-
 git.c            | 1 +
 rebase--status.c | 6 ++++++
 rebase--status.h | 7 +++++++
 6 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 rebase--status.c
 create mode 100644 rebase--status.h
Show changes to 6 files +23 −1

Makefile, builtin.h, git-rebase.sh, git.c, rebase--status.c, rebase--status.h

diff --git a/Makefile b/Makefile
index e0caec3..e3b3e63 100644
--- a/Makefile
+++ b/Makefile
@@ -853,6 +853,7 @@ LIB_OBJS += prompt.o
 LIB_OBJS += quote.o
 LIB_OBJS += reachable.o
 LIB_OBJS += read-cache.o
+LIB_OBJS += rebase--status.o
 LIB_OBJS += reflog-walk.o
 LIB_OBJS += refs.o
 LIB_OBJS += remote.o
@@ -969,6 +970,7 @@ BUILTIN_OBJS += builtin/prune-packed.o
 BUILTIN_OBJS += builtin/prune.o
 BUILTIN_OBJS += builtin/push.o
 BUILTIN_OBJS += builtin/read-tree.o
+BUILTIN_OBJS += builtin/rebase--status--helper.o
 BUILTIN_OBJS += builtin/receive-pack.o
 BUILTIN_OBJS += builtin/reflog.o
 BUILTIN_OBJS += builtin/remote.o
diff --git a/builtin.h b/builtin.h
index c47c110..5071a08 100644
--- a/builtin.h
+++ b/builtin.h
@@ -99,6 +99,7 @@ extern int cmd_prune(int argc, const char **argv, const char *prefix);
 extern int cmd_prune_packed(int argc, const char **argv, const char *prefix);
 extern int cmd_push(int argc, const char **argv, const char *prefix);
 extern int cmd_read_tree(int argc, const char **argv, const char *prefix);
+extern int cmd_rebase_status__helper(int argc, const char **argv, const char *prefix);
 extern int cmd_receive_pack(int argc, const char **argv, const char *prefix);
 extern int cmd_reflog(int argc, const char **argv, const char *prefix);
 extern int cmd_remote(int argc, const char **argv, const char *prefix);
diff --git a/git-rebase.sh b/git-rebase.sh
index 47ca3b9..8454071 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -43,6 +43,7 @@ continue!          continue
 abort!             abort and check out the original branch
 skip!              skip current patch and continue
 edit-todo!         edit the todo list during an interactive rebase
+status!            show the status of the current rebase
 "
 . git-sh-setup
 . git-sh-i18n
@@ -238,7 +239,7 @@ do
 	--verify)
 		ok_to_skip_pre_rebase=
 		;;
-	--continue|--skip|--abort|--edit-todo)
+	--continue|--skip|--abort|--edit-todo|--status)
 		test $total_argc -eq 2 || usage
 		action=${1##--}
 		;;
@@ -401,6 +402,10 @@ abort)
 edit-todo)
 	run_specific_rebase
 	;;
+status)
+	git rebase--status--helper
+	die
+	;;
 esac
 
 # Make sure no rebase is in progress
diff --git a/git.c b/git.c
index 9efd1a3..3ebc144 100644
--- a/git.c
+++ b/git.c
@@ -410,6 +410,7 @@ static struct cmd_struct commands[] = {
 	{ "prune-packed", cmd_prune_packed, RUN_SETUP },
 	{ "push", cmd_push, RUN_SETUP },
 	{ "read-tree", cmd_read_tree, RUN_SETUP },
+	{ "rebase--status--helper", cmd_rebase_status__helper, RUN_SETUP },
 	{ "receive-pack", cmd_receive_pack },
 	{ "reflog", cmd_reflog, RUN_SETUP },
 	{ "remote", cmd_remote, RUN_SETUP },
diff --git a/rebase--status.c b/rebase--status.c
new file mode 100644
index 0000000..d67af52
--- /dev/null
+++ b/rebase--status.c
@@ -0,0 +1,6 @@
+#include "rebase--status.h"
+
+int rebase_status(){
+	printf("Rebase in progress\n");
+	return 0;
+}
diff --git a/rebase--status.h b/rebase--status.h
new file mode 100644
index 0000000..17d22a1
--- /dev/null
+++ b/rebase--status.h
@@ -0,0 +1,7 @@
+#ifndef REBASE__STATUS_H
+#define REBASE__STATUS_H
+
+
+extern int rebase_status();
+
+#endif
-- 
2.0.5.5.g1d968ca.dirty
Matthieu Moy· May 28, 2015, 12:36 UTC · re: Guillaume Pagès · lore

Re: [PATCH/RFC] create a skeleton for the command git rebase --status

Guillaume Pagès <guillaume.pages@ensimag.grenoble-inp.fr> writes:
> It is an almost empty code I send to validate the global architecture
> of this command.  I choose to write

Avoid personal wording (I choose to write ... because -> The code is written ... because). What matters in the commit message is the reason for the choice, not who made it.

> +BUILTIN_OBJS += builtin/rebase--status--helper.o
No builtin/rebase--status--helper.c in your patch, is it intended?
> +status)
> +	git rebase--status--helper
> +	die

"die" is used to exit with an error code ($? != 0). You just mean exit $? here, to exit with the same code as the helper.

And actually, you don't need to keep your script alive while the helper is ran, so you can write

exec git rebase--status--helper
Show 7 quoted lines
> --- /dev/null
> +++ b/rebase--status.c
> @@ -0,0 +1,6 @@
> +#include "rebase--status.h"
> +
> +int rebase_status(){
> +	printf("Rebase in progress\n");
... or not.

Avoid leaving incorrect things like this in intermediate steps, even if you're going to fix them eventually. It's too easy to forget the actual fix and leave a "Rebase in progress" message even when there's no rebase in progress. And reviewers may get confused.

I'd have written stg like
	printf("git rebase --status is not yet implemented");
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Matthieu Moy· May 28, 2015, 13:06 UTC · re: Guillaume Pagès · lore

Re: [PATCH/RFC] create a skeleton for the command git rebase --status

Guillaume Pagès <guillaume.pages@ensimag.grenoble-inp.fr> writes:
> It is an almost empty code I send to validate the global architecture
> of this command.

One more note: I know what "git rebase --status" is about because I'm the one who suggested it and I read your previous message, but not everybody is me, and most people would appreciate a brief explanation of what "git rebase --status" is before knowing how you're starting the work.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

← back to recent threads