threads / patch / 26909

patchreset: update help text

Subject: [PATCH] reset: update help text

## tl;dr

9 messages between Mar 29, 2011 and Apr 2, 2011. Diffs are folded; open one to read it.

replies: 8people: 3as markdown or json

Nguyễn Thái Ngọc Duy· Mar 29, 2011, 13:20 UTC · lore
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(-)
Show changes to builtin/reset.c +3 −3
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
Nguyen Thai Ngoc Duy· Mar 29, 2011, 13:29 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] reset: update help text

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
Jonathan Nieder· Mar 29, 2011, 21:04 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] reset: update help text

(+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.
Show 10 quoted lines
> --- 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.
Show 8 quoted lines
>  		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),
Nguyen Thai Ngoc Duy· Mar 31, 2011, 12:31 UTC · re: Jonathan Nieder · lore

Re: [PATCH] reset: update help text

2011/3/30 Jonathan Nieder <jrnieder@gmail.com>:
Show 19 quoted lines
> 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
Jonathan Nieder· Mar 31, 2011, 18:42 UTC · re: Nguyen Thai Ngoc Duy · lore

Re: [PATCH] reset: update help text

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

Junio C Hamano· Mar 31, 2011, 21:44 UTC · re: Jonathan Nieder · lore

Re: [PATCH] reset: update help text

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 18 quoted lines
> [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?

Jonathan Nieder· Mar 31, 2011, 21:57 UTC · re: Junio C Hamano · lore

Re: [PATCH] reset: update help text

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
Show 21 quoted lines
>> [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.
Nguyen Thai Ngoc Duy· Apr 2, 2011, 08:43 UTC · re: Jonathan Nieder · lore

Re: [PATCH] reset: update help text

On Fri, Apr 1, 2011 at 1:42 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 18 quoted lines
> 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
Junio C Hamano· Mar 31, 2011, 18:45 UTC · re: Jonathan Nieder · lore

Re: [PATCH] reset: update help text

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 18 quoted lines
> 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.

← back to recent threads