[PATCH 2/3] sequencer: add "--reset-hard" option to "git sequencer--helper"

[PATCH 2/3] sequencer: add "--reset-hard" option to "git sequencer--helper"

[Christian Couder]

This new option uses the "reset_almost_hard()" function to perform
a reset.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-sequencer--helper.c |   65 ++++++++++++++++++++++++++++++++++++------
 1 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/builtin-sequencer--helper.c b/builtin-sequencer--helper.c
index 82a830d..e173671 100644
--- a/builtin-sequencer--helper.c
+++ b/builtin-sequencer--helper.c
@@ -18,6 +18,7 @@ static unsigned char head_sha1[20];
 
 static const char * const git_sequencer_helper_usage[] = {
 	"git sequencer--helper --make-patch <commit>",
+	"git sequencer--helper --reset-hard <commit> <reflog-msg> <verbosity>",
 	NULL
 };
 
@@ -172,27 +173,71 @@ static int set_verbosity(int verbose)
 	return 0;
 }
 
+/* v should be "" or "t" or "\d" */
+static int parse_verbosity(const char *v)
+{
+	/* "" means verbosity = 1 */
+	if (!v[0])
+		return set_verbosity(1);
+
+	if (v[1])
+		return 1;
+
+	if (v[0] == 't')
+		return set_verbosity(2);
+
+	if (!isdigit(v[0]))
+		return 1;
+
+	return set_verbosity(v[0] - '0');
+}
+
 int cmd_sequencer__helper(int argc, const char **argv, const char *prefix)
 {
-	char *commit = NULL;
-	struct commit *c;
+	char *patch_commit = NULL;
+	char *reset_commit = NULL;
 	struct option options[] = {
-		OPT_STRING(0, "make-patch", &commit, "commit",
+		OPT_STRING(0, "make-patch", &patch_commit, "commit",
 			   "create a patch from commit"),
+		OPT_STRING(0, "reset-hard", &reset_commit, "commit",
+			   "reset to commit"),
 		OPT_END()
 	};
 
 	argc = parse_options(argc, argv, prefix, options,
 			     git_sequencer_helper_usage, 0);
 
-	if (!commit)
-		usage_with_options(git_sequencer_helper_usage, options);
+	if (patch_commit) {
+		struct commit *c = get_commit(patch_commit);
+		if (!c)
+			return 1;
 
-	c = get_commit(commit);
-	if (!c)
-		return 1;
+		make_patch(c);
 
-	make_patch(c);
+		return 0;
+	}
 
-	return 0;
+	if (reset_commit) {
+		unsigned char sha1[20];
+
+		if (argc != 2)
+			usage_with_options(git_sequencer_helper_usage,
+					   options);
+
+		if (get_sha1(reset_commit, sha1)) {
+			error("Could not find '%s'", reset_commit);
+			return 1;
+		}
+
+		reflog = (char *)argv[0];
+
+		if (parse_verbosity(argv[1])) {
+			error("bad verbosity '%s'", argv[1]);
+			return 1;
+		}
+
+		return reset_almost_hard(sha1);
+	}
+
+	usage_with_options(git_sequencer_helper_usage, options);
 }
-- 
1.6.4.133.g8a5c8

Re: [PATCH 2/3] sequencer: add "--reset-hard" option to "git sequencer--helper"

[Daniel Barkalow]

On Mon, 3 Aug 2009, Christian Couder wrote:

> This new option uses the "reset_almost_hard()" function to perform
> a reset.

Shouldn't it make it possible to do an almost-hard reset (that is, keep a 
dirty working tree's changes while changing the index and HEAD to a 
different commit)? AFAICT, this series doesn't expose the interesting new
functionality it provides.

Also, I thought that we'd found that other built-ins could be simplified 
through the use of functions you're adding here. In particular, checkout 
wants to change the index and working tree while preserving dirty working 
tree changes. So it would probably be better for it to go in a library 
object, where sequencer--helper would just make it available to shell 
code.
	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 2/3] sequencer: add "--reset-hard" option to "git sequencer--helper"

[Christian Couder]

On Tuesday 04 August 2009, Daniel Barkalow wrote:
> On Mon, 3 Aug 2009, Christian Couder wrote:
> > This new option uses the "reset_almost_hard()" function to perform
> > a reset.
>
> Shouldn't it make it possible to do an almost-hard reset (that is, keep a
> dirty working tree's changes while changing the index and HEAD to a
> different commit)?

Yeah, I will improve the documentation of this function as Junio requested.

> AFAICT, this series doesn't expose the interesting new 
> functionality it provides.
>
> Also, I thought that we'd found that other built-ins could be simplified
> through the use of functions you're adding here. In particular, checkout
> wants to change the index and working tree while preserving dirty working
> tree changes. So it would probably be better for it to go in a library
> object, where sequencer--helper would just make it available to shell
> code.

Yeah, I agree, but right now I'd rather keep the code quite close to what it 
is on the sequencer repo if possible.

Thanks,
Christian.

Re: [PATCH 2/3] sequencer: add "--reset-hard" option to "git sequencer--helper"

[Daniel Barkalow]

On Tue, 4 Aug 2009, Christian Couder wrote:

> On Tuesday 04 August 2009, Daniel Barkalow wrote:
> > On Mon, 3 Aug 2009, Christian Couder wrote:
> > > This new option uses the "reset_almost_hard()" function to perform
> > > a reset.
> >
> > Shouldn't it make it possible to do an almost-hard reset (that is, keep a
> > dirty working tree's changes while changing the index and HEAD to a
> > different commit)?
> 
> Yeah, I will improve the documentation of this function as Junio requested.

It's not just the documentation; AFAICT, there's no way to get allow_dirty 
set, which in turn makes a bunch of this code unreachable. It would be 
good to expose that from the start, even if nothing uses it immediately, 
so that people to see whether it behaves as expected.

	-Daniel
*This .sig left intentionally blank*