{"thread":{"id":"38737","subject":"[PATCH] [GSoC Microproject]Adding \"-\" shorthand for \"@{-1}\" in RESET command","startedAt":"2015-03-07T01:57:36Z","lastAt":"2015-03-08T07:34:29Z","messageCount":2,"participants":["Sundararajan R","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"257221","messageId":"1425693456-21163-1-git-send-email-dyoucme@gmail.com","threadId":"38737","inReplyTo":null,"subject":"[PATCH] [GSoC Microproject]Adding \"-\" shorthand for \"@{-1}\" in RESET command","fromName":"Sundararajan R","fromEmail":"dyoucme@gmail.com","sentAt":"2015-03-07T01:57:36Z","receivedAt":"2015-03-07T01:57:36Z","isPatch":true,"sender":{"key":"dyoucme@gmail.com","avatar":null},"body":"Hi all, I am a GSoC '15 aspirant for git.\nIn this commit I have directly associated \"-\" to \"@{-1}\" except when it refers to a filename. \nAll the given tests pass(except those which shouldn't).\nI have to add a failsafe for the case in when there is no branch as \"@{-1}\". For this I have a \nrough idea that I would have to call get-sha1() on @{-1} to check if there is an object matching \nwith it. But I am not able to think of the details.\nPlease guide me with that and give feedback for this patch.\nSigned-off-by: Sundararajan R \n---\n builtin/reset.c | 12 +++++++++++-\n 1 file changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 4c08ddc..62764d4 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -203,8 +203,16 @@ static void parse_args(struct pathspec *pathspec,\n \t *\n \t * At this point, argv points immediately after [-opts].\n \t */\n-\n+\tint flag=0; /* \n+\t\t * \"-\" may refer to filename in which case we should be giving more precedence \n+\t\t * to filename than equating argv[0] to \"@{-1}\" \n+\t\t */\n \tif (argv[0]) {\n+\t\tif (!strcmp(argv[0], \"-\") && !argv[1]) /* \"-\" is the only argument */\n+\t\t{\n+\t\t\targv[0]=\"@{-1}\";\n+\t\t\tflag=1;\n+\t\t}\n \t\tif (!strcmp(argv[0], \"--\")) {\n \t\t\targv++; /* reset to HEAD, possibly with paths */\n \t\t} else if (argv[1] && !strcmp(argv[1], \"--\")) {\n@@ -226,6 +234,8 @@ static void parse_args(struct pathspec *pathspec,\n \t\t\trev = *argv++;\n \t\t} else {\n \t\t\t/* Otherwise we treat this as a filename */\n+\t\t\tif(flag)\n+\t\t\t\targv[0]=\"-\";\n \t\t\tverify_filename(prefix, argv[0], 1);\n \t\t}\n \t}\n-- \n2.1.0\n"},{"id":"257269","messageId":"xmqqfv9gt01m.fsf@gitster.dls.corp.google.com","threadId":"38737","inReplyTo":"1425693456-21163-1-git-send-email-dyoucme@gmail.com","subject":"Re: [PATCH] [GSoC Microproject]Adding \"-\" shorthand for \"@{-1}\" in RESET command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-08T07:34:29Z","receivedAt":"2015-03-08T07:34:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sundararajan R writes:\n\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 4c08ddc..62764d4 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -203,8 +203,16 @@ static void parse_args(struct pathspec *pathspec,\n> \t *\n> \t * At this point, argv points immediately after [-opts].\n> \t */\n> -\n> +\tint flag=0; /* \n> +\t\t * \"-\" may refer to filename in which case we should be giving more precedence \n> +\t\t * to filename than equating argv[0] to \"@{-1}\" \n> +\t\t */\n\nComment on a separate line. More importantly, think if you can give\nthe variable a more meaningful name so that you do not have to\nexplain.\n\nYou are missing SPs requested by the coding guideline everywhere in\nyour patch.\n\n\n> \tif (argv[0]) {\n> +\t\tif (!strcmp(argv[0], \"-\") && !argv[1]) /* \"-\" is the only argument */\n> +\t\t{\n> +\t\t\targv[0]=\"@{-1}\";\n> +\t\t\tflag=1;\n> +\t\t}\n> \t\tif (!strcmp(argv[0], \"--\")) {\n> \t\t\targv++; /* reset to HEAD, possibly with paths */\n> \t\t} else if (argv[1] && !strcmp(argv[1], \"--\")) {\n> @@ -226,6 +234,8 @@ static void parse_args(struct pathspec *pathspec,\n\nAround here not shown by this patch there are a few uses of argv[0],\nand the most important one is\n\n\t\t\tverify_non_filename(prefix, argv[0]);\n\njust before the line below (see below).\n\n> \t\t\trev = *argv++;\n> \t\t} else {\n> \t\t\t/* Otherwise we treat this as a filename */\n> +\t\t\tif(flag)\n> +\t\t\t\targv[0]=\"-\";\n> \t\t\tverify_filename(prefix, argv[0], 1);\n> \t\t}\n> \t}\n\nBy the way, do you understand the intent of the existing checks in\nthis codepath that uses verify_filename() and verify_non_filename()?\n\nThe idea is to allow users to write \"git reset X\" and \"git reset Y\nZ\" safely in an unambiguous way.\n\n * X could be a commit (e.g. \"git reset master\"), to update the\n current branch to point at the same commit as 'master' and update\n the index to match.\n\n * X could be a pathspec (e.g. \"git reset hello.c\"), to grab the\n blob object for X out of the HEAD and put it in the index.\n\n * Y could be a tree-ish and Z a pathspec (e.g. \"git reset HEAD^\n hello.c\"), to grab the blob object for Z out of tree-ish Y and\n put it to the index.\n\n * Both Y and Z could be pathspecs (e.g. \"git reset hello.c\n goodbye.c\"), to revert the index entries for these two paths to\n what the HEAD records.\n\nIf you happen to have a file whose name is 'master', and if you are\nworking on your 'topic' branch, what would this do?\n\n $ git reset master\n\nIs this a request to revert the index entry for path 'master' from\nthe HEAD? Or is it a request to update the current branch to be the\nsame as the 'master' branch and repopulate the index from there?\n\nWhat does the existing code try to do, and how does it do it? It\ndetects the ambiguity and refuses to do either, to make sure it\ncauses no harm.\n\nNow, with your change, does the result still honor this \"when\nambiguous, stop without causing harm to the user\" principle? What\nhappens when your user has a file whose name is \"-\" in the working\ntree? What happens when your user has a file whose name is \"@{-1}\"\nin the working tree?\n"}]}