Re: [PATCH] [GSoC Microproject]Adding "-" shorthand for "@{-1}" in RESET command

[Junio C Hamano <gitster@pobox.com>]

Sundararajan R <dyoucme@gmail.com> writes:

> I am sorry for the mistakes in the code formatting. It was because I was in 
> a hurry that day and I wanted to submit a working patch.

No need to apologize for mistakes.  Mistakes are expected part of
being human and the review process is designed to catch exactly
that.

The development is not a race to see who gets there first.  It is a
collaborative process to get to a better place together.  Once you
got something "working", stop and review your work to see if your
definition of "working" is sensible.  Is there a corner case you
missed?  Is the code formatted in a similar way as the existing code
around the area you are touching?  Are there better ways to do what
you did?  Take your time to make sure you would be happy with what
you are sending out.

> In the new patch I 
> am making, I am using check_filename() to see if there are files named "-" 
> and "@{-1}" in the working tree . Is this an appropriate way to check or is 
> there something else suggested? 

I think you are making it unnecessarily hard.  With your patch, the
code would look like this:

 	if (argv[0]) {
+		if (!strcmp(argv[0], "-") && !argv[1])  /* "-" is the only argument */
+		{
+			argv[0]="@{-1}";
+			flag=1;
+		}
 		if (!strcmp(argv[0], "--")) {
 			argv++; /* reset to HEAD, possibly with paths */
 		} else if (argv[1] && !strcmp(argv[1], "--")) {
 			rev = argv[0];
 			argv += 2;
 		}
 		/*
 		 * Otherwise, argv[0] could be either <rev> or <paths> and
 		 * has to be unambiguous. If there is a single argument, it
 		 * can not be a tree
 		 */
 		else if ((!argv[1] && !get_sha1_committish(argv[0], unused)) ||
 			 (argv[1] && !get_sha1_treeish(argv[0], unused))) {
 			/*
 			 * Ok, argv[0] looks like a commit/tree; it should not
 			 * be a filename.
 			 */
 			verify_non_filename(prefix, argv[0]);
 			rev = *argv++;
 		} else {
 			/* Otherwise we treat this as a filename */
+			if(flag)
+				argv[0]="-";
 			verify_filename(prefix, argv[0], 1);
 		}
 	}

I was wondering what you are passing to verify_non_filename() that
you did not touch.  It would see "@{-1}", try to make sure that the
working tree does not have a file with that name, and if there is
the end user would be warned about ambiguity.

If the user typed "git reset @{-1}", then that warning is very
sensible, but when the end user only typed "git reset -", is there
any ambiguity?