threads / rfc / 39449

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

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

## tl;dr

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

replies: 9people: 3as markdown or json

Guillaume Pagès· May 28, 2015, 14:20 UTC · lore

Preparatory commit for a git rebase --status command. This command will indicate the state of the process in the rebase, and the reason why it stopped.

Signed-off-by: Guillaume Pagès <guillaume.pages@ensimag.grenoble-inp.fr>
---
The observations from Matthieu Moy have been taken into account 

It is an almost empty code sent to validate the global architecture of this command. It is written 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 will likely use code from wt_status to implement this functionnality. The command calls a helper from a shell script, as it is made in bisect (bisect--helper.c).

 Makefile                         |  2 ++
 builtin.h                        |  1 +
 builtin/rebase--status--helper.c | 23 +++++++++++++++++++++++
 git-rebase.sh                    |  6 +++++-
 git.c                            |  1 +
 rebase--status.c                 |  6 ++++++
 rebase--status.h                 |  7 +++++++
 7 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 builtin/rebase--status--helper.c
 create mode 100644 rebase--status.c
 create mode 100644 rebase--status.h
Show changes to 7 files +45 −1

Makefile, builtin.h, builtin/rebase--status--helper.c, 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/builtin/rebase--status--helper.c b/builtin/rebase--status--helper.c
new file mode 100644
index 0000000..efda29c
--- /dev/null
+++ b/builtin/rebase--status--helper.c
@@ -0,0 +1,23 @@
+#include "builtin.h"
+#include "cache.h"
+#include "parse-options.h"
+#include "rebase--status.h"
+
+static const char * const git_rebase_status_helper_usage[] = {
+	N_("git rebase--status--helper"),
+	NULL
+};
+
+int cmd_rebase_status__helper(int argc, const char **argv, const char *prefix)
+{
+	struct option options[] = {
+		
+	};
+
+	argc = parse_options(argc, argv, prefix, options,
+			     git_rebase_status_helper_usage, 0);
+
+
+	/* next-all */
+	return rebase_status();
+}
diff --git a/git-rebase.sh b/git-rebase.sh
index 47ca3b9..4e1f3e1 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,9 @@ abort)
 edit-todo)
 	run_specific_rebase
 	;;
+status)
+	exec git rebase--status--helper
+	;;
 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..1962349
--- /dev/null
+++ b/rebase--status.c
@@ -0,0 +1,6 @@
+#include "rebase--status.h"
+
+int rebase_status(){
+	printf("git rebase --status is not yet implemented\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.6.g818b3fd
Matthieu Moy· May 28, 2015, 15:39 UTC · re: Guillaume Pagès · lore

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

Guillaume Pagès <guillaume.pages@ensimag.grenoble-inp.fr> writes:
Show 6 quoted lines
> It is an almost empty code sent to validate the global architecture of
> this command.  It is written 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 will likely use code from wt_status to implement this
> functionnality. The command calls a helper from a shell script, as it
> is made in bisect (bisect--helper.c).

I think this paragraph makes sense in the commit message. My previous remark was about the wording, not about the relevance of the arguments.

Show 10 quoted lines
> +static const char * const git_rebase_status_helper_usage[] = {
> +	N_("git rebase--status--helper"),
> +	NULL
> +};
> +
> +int cmd_rebase_status__helper(int argc, const char **argv, const char *prefix)
> +{
> +	struct option options[] = {
> +		
> +	};

You need to tell parse_options when the array is over (that's C ...) => OPT_END().

> +	argc = parse_options(argc, argv, prefix, options,
> +			     git_rebase_status_helper_usage, 0);

Actually, you don't need option-parsing at all since you don't pass arguments to the helper, but why not have this in case we need it later.

No need to resend now, you'll send a new version when you have something to build on top (or if someone has more important remarks).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Guillaume Pages· May 28, 2015, 15:43 UTC · lore

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

Hi, 
"Paul Tan" <pyokagan@gmail.com> : 
Show 7 quoted lines
> I haven't kept up with the discussion, but I'm wondering: since you 
> need the functionality in wt-status.c, why not implement it in git 
> status? In fact, git-status already shows if there is a rebase in 
> progress, so why not extend it to say which patch/todo line the rebase 
> stopped on? It feels much more natural to me to use git-status to 
> check the status of the rebase, instead of git rebase --status. 
> Thanks, Paul 

It's a question I have asked myself but since git rebase --status will certainly display more information maybe it's better to separate the two commands, and to indicate the user that git rebase --status exists in git status during a rebase. If we choose to use git status, it could be an option to display the full information since it currently gives very few information. I'm not sure what -verbose does but it could be its role.

Thanks, 
Guillaume Pagès 
Matthieu Moy· May 28, 2015, 15:53 UTC · re: Guillaume Pages · lore

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

Guillaume Pages <guillaume.pages@ensimag.grenoble-inp.fr> writes:
Show 13 quoted lines
> Hi, 
>
> "Paul Tan" <pyokagan@gmail.com> : 
>> I haven't kept up with the discussion, but I'm wondering: since you 
>> need the functionality in wt-status.c, why not implement it in git 
>> status? In fact, git-status already shows if there is a rebase in 
>> progress, so why not extend it to say which patch/todo line the rebase 
>> stopped on? It feels much more natural to me to use git-status to 
>> check the status of the rebase, instead of git rebase --status. 
>> Thanks, Paul 
>
> It's a question I have asked myself but since git rebase --status will 
> certainly display more information 

Historically, I had the idea of "git rebase --status" and Ensimag students started working on it 2 years ago. The same question came back then during the discussion: why not just in status? This lead to the header in the output of "git status" ("You are currently ...", for rebase/am/bisect/...).

But I think there are more relevant information to show (e.g. list of already applied commits, remaining list of commits, possibly truncated if the list is overly long, and information that rebase gave you when stopping like the path to the file being applied). Having them all in "git status" would make the output really long, for little benefit in day-to-day use.

So, to me, it makes sense to have a separate command "tell me everything you know about the current state of the rebase".

> If we choose to use git status, it could be an option to display the
> full information since it currently gives very few information. I'm
> not sure what -verbose does but it could be its role.

"git status --verbose" is already taken for "show me the diff together with status".

(I'm also dreaming of a "git status --tutor" that would show detailed explanations with pointers to the documentation & all for each section of "status". Useless for non-beginners, but could be a nice way to teach Git to beginners)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Junio C Hamano· May 28, 2015, 18:18 UTC · re: Matthieu Moy · lore

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

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
Show 6 quoted lines
> But I think there are more relevant information to show (e.g. list of
> already applied commits, remaining list of commits, possibly truncated
> if the list is overly long, and information that rebase gave you when
> stopping like the path to the file being applied). Having them all in
> "git status" would make the output really long, for little benefit in
> day-to-day use.
Sorry, I do not quite agree with this reasoning.

Isn't "git status" during a rebase that shows "really long" information to help the rebase operation a good thing? In day-to-day use when you are not in the middle of rebase, the command would not show what remains to be done, would it?

I may be biased, because I rarely use 'git status' while running 'git rebase' (with or without interactive). But to me, 'git diff' would be a more appropriate tool to help me unstuck in managing the current step of conflict resolution than 'git status' gives me during either a rebase or a merge as "Unmerged paths" anyway.

If this topic enhances 'git status' with the in-progress rebase information, I'd view it as turning 'git status' from 'a more or less useless command during rebase' to 'a useful command'.

Matthieu Moy· May 28, 2015, 19:40 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
> If this topic enhances 'git status' with the in-progress rebase
> information, I'd view it as turning 'git status' from 'a more or
> less useless command during rebase' to 'a useful command'.

For day-to-day operations, what we already have in status already qualifies as 'useful command' to me:

$ git status
rebase in progress; onto 7f9a792
You are currently rebasing branch 'master' on '7f9a792'.
  (fix conflicts and then run "git rebase --continue")
  (use "git rebase --skip" to skip this patch)
  (use "git rebase --abort" to check out the original branch)

I like the output of "git status" to be concise. OTOH, there are tons of information in .git/rebase-merge/ that could be displayed to the user.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Junio C Hamano· May 28, 2015, 19:50 UTC · re: Matthieu Moy · lore

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

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
Show 15 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> If this topic enhances 'git status' with the in-progress rebase
>> information, I'd view it as turning 'git status' from 'a more or
>> less useless command during rebase' to 'a useful command'.
>
> For day-to-day operations, what we already have in status already
> qualifies as 'useful command' to me:
>
> $ git status
> rebase in progress; onto 7f9a792
> You are currently rebasing branch 'master' on '7f9a792'.
>   (fix conflicts and then run "git rebase --continue")
>   (use "git rebase --skip" to skip this patch)
>   (use "git rebase --abort" to check out the original branch)

Not really. How would you decide if 7f9a792 is worth keeping or rebase is better be aborted without knowing where you are?

> I like the output of "git status" to be concise.

Sure, as long as concise and useful, I am all for it. The above however does not show anything I already know in my prompt. I would say no thanks to "concise and useless".

> OTOH, there are tons of information in .git/rebase-merge/ that
> could be displayed to the user.
Surely, that is why "git status" during a rebase should show them.
Guillaume Pages· May 29, 2015, 10:40 UTC · re: Junio C Hamano · lore

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

After this discussion I eventually agree that it would be better upgrading git status than creating a new command.When people use git status, it means that they need information to continue their work, so if you don't even know that you are in a rebase, you will very likely need information about the current rebase.

During a classic rebase we could have output like:

rebase in progress; onto d9d448a You are currently rebasing branch 'branche1' on 'd9d448a'. (fix conflicts and then run "git rebase --continue") (use "git rebase --skip" to skip this patch) (use "git rebase --abort" to check out the original branch) (5 commits applied, 3 remainings) Failed to apply:

252c273 commit message

Unmerged paths: (use "git reset HEAD <file>..." to unstage) (use "git add <file>..." to mark resolution)

both modified: file1
And during an interactive rebase:

rebase in progress; onto 247c883 You are currently editing a commit while rebasing branch 'b1.1' on '247c883'. (use "git commit --amend" to amend the current commit) (use "git rebase --continue" once you are satisfied with your changes)

Last commands done (5 commands done) :
pick 62609785 commit message1 reword 85ae9001 new commit message2
(See more at .git/rebase-merge/done)
Next commands to do (3 remainings commands) :
squash 62609785 commit message3 pick 85ae9001 commit message4
(See more at .git/rebase-merge/git-rebase-todo)

Changes not staged for commit: (use "git add <file>..." to update what will be committed) (use "git checkout -- <file>..." to discard changes in working directory)

modified: file1 ...

Is it a good practice to send the user find information in the .git directory?

Thanks
Guillaume
Matthieu Moy· May 29, 2015, 15:00 UTC · re: Guillaume Pages · lore

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

Guillaume Pages <guillaume.pages@ensimag.grenoble-inp.fr> writes:
Show 7 quoted lines
> rebase in progress; onto d9d448a You are currently rebasing branch
> 'branche1' on 'd9d448a'.  (fix conflicts and then run "git rebase
> --continue") (use "git rebase --skip" to skip this patch) (use "git
> rebase --abort" to check out the original branch) (5 commits applied,
> 3 remainings) Failed to apply:
>
> 252c273 commit message

You messed-up something with the formatting, but I agree that this would be nice. I'd reverse order between the hints ("(use ... to ...)") and the "Failed to apply: ...".

> And during an interactive rebase:
>
> rebase in progress;
This could even become "interactive rebase in progress".

Most of the time, you're supposed to remember whether you ran "git rebase" with -i, but a typical use-case is when a newbie requests help like "I don't know what I did, but can you fix it?", and then any information can be valuable.

Show 11 quoted lines
> Last commands done (5 commands done) :
>
> pick 62609785 commit message1 reword 85ae9001 new commit message2
>
> (See more at .git/rebase-merge/done)
>
> Next commands to do (3 remainings commands) :
>
> squash 62609785 commit message3 pick 85ae9001 commit message4
>
> (See more at .git/rebase-merge/git-rebase-todo)

Not sure the blank lines are meant to be there, but I wouldn't put them in the actual output. I'd format it as

Last commands done (5 commands done) :
  pick 62609785 commit message1
  (see more at .git/rebase-merge/done)
(lower-case "see" to be consistant with other hints)
> Is it a good practice to send the user find information in the .git
> directory?

We usually avoid doing that and provide commands to do this (e.g. "git rebase --edit-todo" instead of asking the user to do $EDITOR .git/rebase-merge/git-rebase-todo), but the ones you show seem OK to me.

There's already at least one instance of this when a rebase fails:
  Patch failed at 0001 foo
  The copy of the patch that failed is found in:
     /tmp/clone/.git/rebase-apply/patch
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Junio C Hamano· May 29, 2015, 15:44 UTC · re: Matthieu Moy · lore

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

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
Show 12 quoted lines
> Guillaume Pages <guillaume.pages@ensimag.grenoble-inp.fr> writes:
> ...
>> And during an interactive rebase:
>>
>> rebase in progress;
>
> This could even become "interactive rebase in progress".
>
> Most of the time, you're supposed to remember whether you ran "git
> rebase" with -i, but a typical use-case is when a newbie requests help
> like "I don't know what I did, but can you fix it?", and then any
> information can be valuable.
Yeah, I like that.
Show 8 quoted lines
> Not sure the blank lines are meant to be there, but I wouldn't put them
> in the actual output. I'd format it as
>
> Last commands done (5 commands done) :
>   pick 62609785 commit message1
>   (see more at .git/rebase-merge/done)
>
> (lower-case "see" to be consistant with other hints)
Nice.
Show 6 quoted lines
>> Is it a good practice to send the user find information in the .git
>> directory?
>
> We usually avoid doing that and provide commands to do this (e.g. "git
> rebase --edit-todo" instead of asking the user to do $EDITOR
> .git/rebase-merge/git-rebase-todo), but the ones you show seem OK to me.

Yeah, I think it is OK to suggest peeking into (not writing) files like this. We may update the internal implementation but then the suggestion can and should be updated when that happens.

← back to recent threads