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

[Matthieu Moy]

Guillaume Pagès guillaume.pa...@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

 --- /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/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Matthieu Moy]

Guillaume Pagès guillaume.pa...@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/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Guillaume Pagès]

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.pa...@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

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 000..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 000..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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html