[PATCH] reset: update help text

[PATCH] reset: update help text

[Nguyễn Thái Ngọc Duy]

State --mixed is the default so users don't have to open up man page.

Also make it clear how --hard and --merge are different.

Move -q to bottom because it's not that important to pay attention to as
the first item.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/reset.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index eb5f98c..7419c9e 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -243,17 +243,17 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	struct commit *commit;
 	char *reflog_action, msg[1024];
 	const struct option options[] = {
-		OPT__QUIET(&quiet, "be quiet, only report errors"),
 		OPT_SET_INT(0, "mixed", &reset_type,
-						"reset HEAD and index", MIXED),
+				"reset HEAD and index (default)", MIXED),
 		OPT_SET_INT(0, "soft", &reset_type, "reset only HEAD", SOFT),
 		OPT_SET_INT(0, "hard", &reset_type,
 				"reset HEAD, index and working tree", HARD),
 		OPT_SET_INT(0, "merge", &reset_type,
-				"reset HEAD, index and working tree", MERGE),
+				"like --hard but keep local changes in working tree", MERGE),
 		OPT_SET_INT(0, "keep", &reset_type,
 				"reset HEAD but keep local changes", KEEP),
 		OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
+		OPT__QUIET(&quiet, "be quiet, only report errors"),
 		OPT_END()
 	};
 
-- 
1.7.4.74.g639db

Re: [PATCH] reset: update help text

[Nguyen Thai Ngoc Duy]

2011/3/29 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:
> Also make it clear how --hard and --merge are different.

By the way I don't like the option names --hard, --mixed and --soft.
They do not remind me what they actually do. Every time I need to do
non-default operation, I have to open man page.

I've been thinking of --ref, --index and --worktree as a more friendly
version, which can be combined and converted to --soft, --mixed and
--hard internally. Invalid combinations will be rejected. That
thinking went well until I found out there were --merge and --keep,
and was stuck.
-- 
Duy

Re: [PATCH] reset: update help text

[Jonathan Nieder]

(+cc: Christian, Michael, and Thomas)
Nguyễn Thái Ngọc Duy wrote:

> State --mixed is the default so users don't have to open up man page.
>
> Also make it clear how --hard and --merge are different.

Thanks; I think the goal of this patch is a good one.

> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -243,17 +243,17 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
>  	struct commit *commit;
>  	char *reflog_action, msg[1024];
>  	const struct option options[] = {
> -		OPT__QUIET(&quiet, "be quiet, only report errors"),
>  		OPT_SET_INT(0, "mixed", &reset_type,
> -						"reset HEAD and index", MIXED),
> +				"reset HEAD and index (default)", MIXED),

Nice.

>  		OPT_SET_INT(0, "soft", &reset_type, "reset only HEAD", SOFT),
>  		OPT_SET_INT(0, "hard", &reset_type,
>  				"reset HEAD, index and working tree", HARD),
>  		OPT_SET_INT(0, "merge", &reset_type,
> -				"reset HEAD, index and working tree", MERGE),
> +				"like --hard but keep local changes in working tree", MERGE),
>  		OPT_SET_INT(0, "keep", &reset_type,
>  				"reset HEAD but keep local changes", KEEP),

The description does not make it obvious to me how these two (--merge
and --keep) differ.  I think the intent of the options are:

 --keep:
	start working on a different commit, carrying over local changes
	(like "git checkout")
 --merge:
	return to <commit>, cancelling a merge-like operation that
	creates some unmerged and some clean index entries

Maybe something along these lines could be ok starting point?

	OPT_SET_INT(0, "keep", &reset_type,
		"move to <commit>, carrying over local changes in working tree",
		KEEP),
	OPT_SET_INT(0, "merge", &reset_type,
		"return to <commit>, cancelling failed merge or cherry-pick",
		MERGE),

Re: [PATCH] reset: update help text

[Nguyen Thai Ngoc Duy]

2011/3/30 Jonathan Nieder <jrnieder@gmail.com>:
> The description does not make it obvious to me how these two (--merge
> and --keep) differ.  I think the intent of the options are:
>
>  --keep:
>        start working on a different commit, carrying over local changes
>        (like "git checkout")
>  --merge:
>        return to <commit>, cancelling a merge-like operation that
>        creates some unmerged and some clean index entries
>
> Maybe something along these lines could be ok starting point?
>
>        OPT_SET_INT(0, "keep", &reset_type,
>                "move to <commit>, carrying over local changes in working tree",
>                KEEP),
>        OPT_SET_INT(0, "merge", &reset_type,
>                "return to <commit>, cancelling failed merge or cherry-pick",
>                MERGE),
>

To be honest, I have no idea what the above describes. I read 9bc454d
(reset: add option "--keep" to "git reset" - 2010-01-19) and figured
that --keep is like --merge except that "git diff" before and after
the reset is exactly the same, is it? I have never used --keep before.
-- 
Duy

Re: [PATCH] reset: update help text

[Jonathan Nieder]

Nguyen Thai Ngoc Duy wrote:

> To be honest, I have no idea what the above describes. I read 9bc454d
> (reset: add option "--keep" to "git reset" - 2010-01-19) and figured
> that --keep is like --merge except that "git diff" before and after
> the reset is exactly the same, is it? I have never used --keep before.

I use "git reset --keep" to

 - discard a bad commit: git reset --keep HEAD^
 - start working against a different commit:

	git checkout -b topic &&
	... hack hack hack without committing ... &&
	: "oops, I thought I was on master but I was somewhere else" &&
	git reset --keep master

The spirit of the thing[1] is:

 * if the diff "HEAD -> <commit>" touch paths in which we have local
   changes, error out;
 * otherwise, checkout the relevant paths from <commit> but leave the
   paths in which we have local changes alone.

which indeed means "git diff --cached" and "git diff" before and after
would be exactly the same.  But there are some edge cases, in which
the diff "HEAD -> <commit>" makes the same change we did and the
reset --keep is still allowed.

This is totally different from --merge.  The only legitimate use of
--merge is to cancel a merge you just performed, imho --- the effect
otherwise is too scary ("git add <path>; git reset --merge elsewhere"
--- bye, bye, changes made at <path>).

Hope that helps,
Jonathan

[1] Detailed semantics: for each path listed by "git diff --name-only
HEAD <commit>":

 * if the worktree, index, and HEAD match, make the index and worktree
   match <commit>.

 * otherwise, if it is "not easy" to keep local changes, error out.
   We are not going to do a three-way merge.  That is,

   - if the index matches neither HEAD nor <commit>, error out;
   - if the index matches HEAD but not the worktree, error out.

 * otherwise, it is "easy", so keep local changes.

   - if the index already matches <commit>, leave the index and
     worktree alone.
   - if the index and worktree match HEAD, make them match the
     <commit> instead.

These are the same rules used by fast-forward merges and plain
"git checkout".

Re: [PATCH] reset: update help text

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> [1] Detailed semantics: for each path listed by "git diff --name-only
> HEAD <commit>":
>
>  * if the worktree, index, and HEAD match, make the index and worktree
>    match <commit>.
>
>  * otherwise, if it is "not easy" to keep local changes, error out.
>    We are not going to do a three-way merge.  That is,
>
>    - if the index matches neither HEAD nor <commit>, error out;
>    - if the index matches HEAD but not the worktree, error out.
>
>  * otherwise, it is "easy", so keep local changes.
>
>    - if the index already matches <commit>, leave the index and
>      worktree alone.
>    - if the index and worktree match HEAD, make them match the
>      <commit> instead.

The latter item in the last "otherwise" is the same as the first "if all
match, move to the one from the named commit", isn't it?

Re: [PATCH] reset: update help text

[Jonathan Nieder]

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> [1] Detailed semantics: for each path listed by "git diff --name-only
>> HEAD <commit>":
>>
>>  * if the worktree, index, and HEAD match, make the index and worktree
>>    match <commit>.
>>
>>  * otherwise, if it is "not easy" to keep local changes, error out.
>>    We are not going to do a three-way merge.  That is,
>>
>>    - if the index matches neither HEAD nor <commit>, error out;
>>    - if the index matches HEAD but not the worktree, error out.
>>
>>  * otherwise, it is "easy", so keep local changes.
>>
>>    - if the index already matches <commit>, leave the index and
>>      worktree alone.
>>    - if the index and worktree match HEAD, make them match the
>>      <commit> instead.
>
> The latter item in the last "otherwise" is the same as the first "if all
> match, move to the one from the named commit", isn't it?

Yes, sorry for the nonsense.

Re: [PATCH] reset: update help text

[Nguyen Thai Ngoc Duy]

On Fri, Apr 1, 2011 at 1:42 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Nguyen Thai Ngoc Duy wrote:
>
>> To be honest, I have no idea what the above describes. I read 9bc454d
>> (reset: add option "--keep" to "git reset" - 2010-01-19) and figured
>> that --keep is like --merge except that "git diff" before and after
>> the reset is exactly the same, is it? I have never used --keep before.
>
> I use "git reset --keep" to
>
>  - discard a bad commit: git reset --keep HEAD^
>  - start working against a different commit:
>
>        git checkout -b topic &&
>        ... hack hack hack without committing ... &&
>        : "oops, I thought I was on master but I was somewhere else" &&
>        git reset --keep master
>
> ..

Thanks. Your changes make sense to me now.
-- 
Duy

Re: [PATCH] reset: update help text

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> The description does not make it obvious to me how these two (--merge
> and --keep) differ.  I think the intent of the options are:
>
>  --keep:
> 	start working on a different commit, carrying over local changes
> 	(like "git checkout")
>  --merge:
> 	return to <commit>, cancelling a merge-like operation that
> 	creates some unmerged and some clean index entries
>
> Maybe something along these lines could be ok starting point?
>
> 	OPT_SET_INT(0, "keep", &reset_type,
> 		"move to <commit>, carrying over local changes in working tree",
> 		KEEP),
> 	OPT_SET_INT(0, "merge", &reset_type,
> 		"return to <commit>, cancelling failed merge or cherry-pick",
> 		MERGE),

I cannot think of anything better, but I can still show my concern that
"move to" and "return to" might not convey exactly what we are trying to
express.

But I agree that your phrasing definitely is a good starting point.