On Mon, Mar 9, 2015 at 4:46 PM, Sundararajan R <dyoucme@xxxxxxxxx> wrote:
> Please give feedback and suggest things I may have missed out on.
> I hope I have incorporated all the suggestions.
If you haven't already, read Documentation/SubmittingPatches. Pay
particular attention to section #2 which explains how to write a good
commit message, and to section #4 to learn where to place patch
commentary not intended as part of the permanent commit history. The
above lines are commentary, not meant as part of the commit message.
Place such commentary below the '---' line just before the diffstat.
> Subject: Adding - shorthand for @{-1} in RESET command
Prefix the first line of the commit message with the module or command
you re changing. Drop capitalization. Write in imperative mood. For
instance:
reset: add '-' shorthand for '@{-1}'
> Signed-off-by: Sundararajan R <dyoucme@xxxxxxxxx>
> Thanks-to: Junio C Hamano
Place your sign-off last. Use Helped-by: rather than Thanks-to: and
include the person's full name and email address.
> ---
Here, just below the '---' line is where you should place commentary
not intended for the permanent commit record.
> I have attempted to resolve the ambiguity when there exists a file named -
> by communicating to the user that he/she can use ./- when he/she wants to refer
> to the - file. I perform this check using the check_filename() function.
This is important information for the commit message itself above the
'---' line, though you would want to rephrase it just to state the
facts. No need to mention "I did this" or "I did that" since the patch
itself implies that you made the changes.
> builtin/reset.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 4c08ddc..2bdd5cd 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -192,6 +192,7 @@ static void parse_args(struct pathspec *pathspec,
> {
> const char *rev = "HEAD";
> unsigned char unused[20];
> + int file_named_minus=0;
Style: Here and elsewhere, add a space around '='.
> /*
> * Possible arguments are:
> *
> @@ -205,6 +206,12 @@ static void parse_args(struct pathspec *pathspec,
> */
>
> if (argv[0]) {
> + if (!strcmp(argv[0], "-") && !argv[1]) {
> + if(!check_filename(prefix,"-"))
Style: Here and elsewhere, add space after 'if'.
Style: Add space after comma.
> + argv[0]="@{-1}";
> + else
> + file_named_minus=1;
> + }
> if (!strcmp(argv[0], "--")) {
> argv++; /* reset to HEAD, possibly with paths */
> } else if (argv[1] && !strcmp(argv[1], "--")) {
> @@ -226,7 +233,14 @@ static void parse_args(struct pathspec *pathspec,
> rev = *argv++;
> } else {
> /* Otherwise we treat this as a filename */
> - verify_filename(prefix, argv[0], 1);
> + if(file_named_minus) {
> + die(_("ambiguous argument '-': both revision and filename\n"
> + "Use ./- for file named -\n"
> + "Use '--' to separate paths from revisions, like this:\n"
> + "'git <command> [<revision>...] -- [<file>...]'"));
This seems odd. If arguments following '--' are unconditionally
treated as paths, why is it be necessary to tell the user to spell out
file '-' as './-'? Shouldn't "git reset -- -" be sufficient?
> + }
> + else
> + verify_filename(prefix, argv[0], 1);
> }
> }
> *rev_ret = rev;
> --
> 2.1.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html