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

2015-05-29 Thread Guillaume Pages
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
--
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: [RFC/PATCH v2] create a skeleton for the command git rebase --status

2015-05-29 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Guillaume Pages guillaume.pa...@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.

 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.

 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.
--
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: [RFC/PATCH v2] create a skeleton for the command git rebase --status

2015-05-29 Thread Matthieu Moy
Guillaume Pages guillaume.pa...@ensimag.grenoble-inp.fr writes:

 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.

 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/
--
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: [RFC/PATCH v2] create a skeleton for the command git rebase --status

2015-05-28 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 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'.
--
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: [RFC/PATCH v2] create a skeleton for the command git rebase --status

2015-05-28 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Junio C Hamano gits...@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.

--
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: [RFC/PATCH v2] create a skeleton for the command git rebase --status

2015-05-28 Thread Matthieu Moy
Junio C Hamano gits...@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/
--
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


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

2015-05-28 Thread Guillaume Pagès
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.pa...@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

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

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

2015-05-28 Thread Matthieu Moy
Guillaume Pages guillaume.pa...@ensimag.grenoble-inp.fr writes:

 Hi, 

 Paul Tan pyoka...@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/
--
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: [RFC/PATCH v2] create a skeleton for the command git rebase --status

2015-05-28 Thread Matthieu Moy
Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr writes:

 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.

 +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/
--
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