[PATCH v4 2/2] reset: add tests for git reset -
The failure case which occurs on teaching git the '-' shorthand is when there exists no branch pointed to by '@{-1}'. In this case, if there is a file named - in the working tree, the user can be unambiguously assumed to be referring to it while issuing this command. The ambiguous case occurs when the @{-1} branch exists and file named '-' also exists in the working tree. This are also treated as a failure case but here the user is given advice as to how he can proceed. Another potentially tricky case is when the file '@{-1}' exists. In this case, the command should succeed as the user hasn't mentioned the file '@{-1}' and can be safely assumed to be referring to the @{-1} branch. Add tests to check the handling of these cases. Also add a test to verify that reset - behaves like reset @{-1} when none of the above cases are true. Helped-by: Junio C Hamano gits...@pobox.com Helped-by: Eric Sunshine sunsh...@sunshineco.com Helped-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Sundararajan R dyou...@gmail.com --- Have made the edits suggested by Matthew and Kevin. t/t7102-reset.sh | 74 1 file changed, 74 insertions(+) diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh index 98bcfe2..a605c32 100755 --- a/t/t7102-reset.sh +++ b/t/t7102-reset.sh @@ -568,4 +568,78 @@ test_expect_success 'reset --mixed sets up work tree' ' test_cmp expect actual ' +test_expect_success 'reset - with no @{-1} branch should fail' ' + test_when_finished rm -rf new + git init new + ( + cd new + test_must_fail git reset - 2../actual + ) + test_i18ngrep bad flag actual +' + +test_expect_success 'reset - with no @{-1} branch and file named - should succeed' ' + test_when_finished rm -rf new + git init new + ( + cd new + echo Hello - + git add - + git reset - ../actual + ) + test_must_be_empty actual +' + +test_expect_success 'reset - with @{-1} branch and file named - should fail' ' + test_when_finished rm -rf new + git init new + ( + cd new + echo Hello - + git add - + git commit -m first_commit + git checkout -b new_branch + - + git add - + test_must_fail git reset - 2../actual + ) + test_i18ngrep ambiguous argument actual +' + +test_expect_success 'reset - with @{-1} branch and file named @{-1} should succeed' ' + test_when_finished rm -rf new + git init new + ( + cd new + echo Hello @{-1} + git add @{-1} + git commit -m first_commit + git checkout -b new_branch + @{-1} + git add @{-1} + git reset - ../actual + ) + test_i18ngrep Unstaged actual +' + +test_expect_success 'reset - with @{-1} branch and no file named - should succeed' ' + test_when_finished rm -rf new + git init new + ( + cd new + echo Hey new_file + git add new_file + git commit -m first_commit + git checkout -b new_branch + new_file + git add new_file + git reset - + git status -uno --porcelain actual + git add new_file + git reset @{-1} + git status -uno --porcelain expected + test_cmp actual expected + ) +' + test_done -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/2] reset: add '-' shorthand for '@{-1}'
Teaching reset the - shorthand involves checking if any file named '-' exists. check_filename() is used to perform this check. When the @{-1} branch does not exist then it can be safely assumed that the user is referring to the file '-',if any. If this file exists then it is reset. Otherwise, a bad flag error is shown. But if the @{-1} branch exists then it becomes ambiguous without the explicit '--' disambiguation as to whether the user wants to reset the file '-' or if he wants to reset the working tree to the previous branch. Hence the program dies with a message about the ambiguous argument. When none of the above cases hold, - behaves like @{-1}. Helped-by: Junio C Hamano gits...@pobox.com Helped-by: Eric Sunshine sunsh...@sunshineco.com Signed-off-by: Sundararajan R dyou...@gmail.com --- Corrected a minor style error. builtin/reset.c | 21 +++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/builtin/reset.c b/builtin/reset.c index 4c08ddc..80dd5d5 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -192,6 +192,8 @@ static void parse_args(struct pathspec *pathspec, { const char *rev = HEAD; unsigned char unused[20]; + int file_named_minus = 0; + int shorthand = 0; /* * Possible arguments are: * @@ -205,6 +207,12 @@ static void parse_args(struct pathspec *pathspec, */ if (argv[0]) { + if (!strcmp(argv[0], -) !argv[1]) { + argv[0] = @{-1}; + shorthand = 1; + if (check_filename(prefix, -)) + file_named_minus = 1; + } if (!strcmp(argv[0], --)) { argv++; /* reset to HEAD, possibly with paths */ } else if (argv[1] !strcmp(argv[1], --)) { @@ -222,11 +230,20 @@ static void parse_args(struct pathspec *pathspec, * Ok, argv[0] looks like a commit/tree; it should not * be a filename. */ - verify_non_filename(prefix, argv[0]); + if (file_named_minus) { + die(_(ambiguous argument '-': both revision and filename\n + Use '--' to separate paths from revisions, like this:\n + 'git command [revision...] -- [file...]')); + } + else if (!shorthand) + verify_non_filename(prefix, argv[0]); rev = *argv++; } else { /* Otherwise we treat this as a filename */ - verify_filename(prefix, argv[0], 1); + if (shorthand) + argv[0] = -; + if (!file_named_minus) + 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fwd: [RFC] [GSoC Proposal Draft] Unifying git branch -l,git tag -l and git for-each-ref
Hi all, I have attempted a microproject [1][2] and this is my first draft of the proposal.I have included only the matter regarding my approach to solving the problem and shall add my personal details later. Please be kind enough to go through my proposal and suggest modifications or detailing wherever required. Also give me feedback on whether my approach to solving the problem is correct. In the meantime I am reading up the code of Jeff's attempt at unification,here [3] for preparing my final proposal. Title - Unifying git branch -l,git tag -l and git for-each-ref Abstract git for-each-ref and the list modes of git branch and git tag involve selecting a subset of the refs and printing out the result. Currently the implementations are not shared in the sense that :- SELECTION 1. git branch knows --contains,--merged and --no-merged 2. git tag only knows --contains FORMATTING 1. git for-each-ref knows formatting which none of the other two commands know. SORTING 1. git tag knows sorting only on the basis of refnames 2. git for-each-ref knows sorting on the basis of all the fieldnames which can be used in its --format option The idea is to unify the computations for these processes in a common library and teach these commands all these options uniformly. Why do we need unification? These commands try to accomplish more or less the same thing . So,new features would most likely be applicable to all three of them. So, unification will allow us build new features for all these commands in one go instead of doing it separately for each of the three commands. Jeff has already worked quite a bit on unifying the selection part. I shall use that work as a starting point when I start off building the library and its API calls. Deliverables 1. The unified library will borrow the --contains implementation from git tag (due to the speed up it had received), the --merged/--no-merged implementation from git branch and the --format implementation from git for-each-ref. 2. The commands will then be taught these options by making calls to this library functions and structures. 3. Add documentation and tests for these new features. Optionals - 1. Implement the --sort option for these commands in the unified library. 2. Add documentation and tests for this feature Approach The common library will contain a structure which will store the present state of the list of refs in the sense that after we perform a computation(eg. --contains commit) on the list of refs, the new list will store the result of that computation. The structure will also have other attributes which the options structure will take in as its (void *)value attribute’s value before parsing the different options. This is to communicate to the structure about the various options(eg. --merged, --format, --sort) we want to use. The list of refs shall be fetched by the API in accordance with the command(eg. git tag) and its option(eg. --merged) which were passed to the API. Next comes the matter of printing out the results according to the format specified (the default format for the command if no format is specified). This will be done in a method similar to how git for-each-ref prints out the results in the given format. Approximate Timeline (To estimate the amount of work that can be done in summers though it may change during the project[based on advice from mentors]) May 03 - May 10 Read and understand the implementation of --contains option in git tag and the --merged/--no-merged implementation in git branch. May 11 - May 17 Go through Jeff’s work on unification to get detailed pointers on how to start with unifying selection. Finalise all the structures required and also the API calls the library would have to make for the selection options. May 18 - May 24 Start working on the API. Discuss ideas with mentor, brainstorm on the details of what function calls will be made to the API and what function calls will be made by the API. CODING PERIOD BEGINS May 25 - May 31 Implement the --contains option in the library by taking the cue from how git tag --contains is implemented. June 1 - June 7 Implement the -merge and --no-merged options similar to how they are implemented in git branch June 8 - June 11 Make computations more efficient, improve comments and start documentation. Discuss about additional features and requirements with mentors. June 12 - June 25 Teach the three commands to use the API for formatting and sorting. Add tests and refactor the code of the API if required. Complete the documentation for the new features added. MID-TERM EVALUATION June 26 - June 30 Discuss with mentors about the state and the pace with which the project is coming on. Start finalising the details of the further goals to be accomplished. July 01 - July 07 Start working on the formatting and
[GSoC Project Help] Unifying git branch -l, git tag -l and git for-each-ref
Hi all, I am a Computer Science sophomore at IIT Kanpur. I am interested in contributing to git in GSoC 2015. I have been using git for the past one year and am pretty comfortable with its commands which is what made me think about contributing to git. I have attempted the microproject “adding ‘-’ as a shorthand to @{-1} in the reset command” [1] [2] from which I learnt about how code is reviewed in the community and how a seemingly small change can end up being much more difficult. But the thing I liked the most is the warm and welcoming attitude of everyone in the community towards a newcomer like me. I wish to take up the project idea “Unifying git branch -l, git tag -l and git for-each-ref”. I am in the process of reading and understanding the codes of these three commands and figuring out similarities and differences in them. I have gone through some of the discussions regarding this on the archive and have also read Junio’s reply to Amate Yolande [3], but I haven’t been able to find patches which attempt to unify the selection process as mentioned in the description of the idea. It would be great if someone could point me towards these patches which would help me when I start designing the details of the unified implementation. Thanks a lot for your time. Regards, R Sundararajan. [1] : http://marc.info/?l=gitm=142666740415816w=2 [2] : http://marc.info/?l=gitm=142666773315899w=2 [3] : http://article.gmane.org/gmane.comp.version-control.git/264966N�r��yb�X��ǧv�^�){.n�+ا���ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf
[v3 PATCH 1/2] reset: add '-' shorthand for '@{-1}'
Teaching reset the - shorthand involves checking if any file named '-' exists. check_filename() is used to perform this check. When the @{-1} branch does not exist then it can be safely assumed that the user is referring to the file '-',if any. If this file exists then it is reset or else a bad flag error is shown. But if the @{-1} branch exists then it becomes ambiguous without the explicit '--' disambiguation as to whether the user wants to reset the file '-' or if he wants to reset the working tree to the previous branch. Hence the program dies with a message about the ambiguous argument. Helped-by: Junio C Hamano gits...@pobox.com Helped-by: Eric Sunshine sunsh...@sunshineco.com Signed-off-by: Sundararajan R dyou...@gmail.com --- Thank you Eric and Junio for your patient feedback. As verify_filename() and verify_non_filename() die and return,respectively when passed the argument '-' without actually checking if such a file exists, check_filename() has been used to perform this check. I hope it is okay. builtin/reset.c | 21 +++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/builtin/reset.c b/builtin/reset.c index 4c08ddc..a126b38 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -192,6 +192,8 @@ static void parse_args(struct pathspec *pathspec, { const char *rev = HEAD; unsigned char unused[20]; + int file_named_minus = 0; + int shorthand = 0; /* * Possible arguments are: * @@ -205,6 +207,12 @@ static void parse_args(struct pathspec *pathspec, */ if (argv[0]) { + if (!strcmp(argv[0], -) !argv[1]) { + argv[0] = @{-1}; + shorthand = 1; + if(check_filename(prefix, -)) + file_named_minus = 1; + } if (!strcmp(argv[0], --)) { argv++; /* reset to HEAD, possibly with paths */ } else if (argv[1] !strcmp(argv[1], --)) { @@ -222,11 +230,20 @@ static void parse_args(struct pathspec *pathspec, * Ok, argv[0] looks like a commit/tree; it should not * be a filename. */ - verify_non_filename(prefix, argv[0]); + if (file_named_minus) { + die(_(ambiguous argument '-': both revision and filename\n + Use '--' to separate paths from revisions, like this:\n + 'git command [revision...] -- [file...]')); + } + else if (!shorthand) + verify_non_filename(prefix, argv[0]); rev = *argv++; } else { /* Otherwise we treat this as a filename */ - verify_filename(prefix, argv[0], 1); + if (shorthand) + argv[0] = -; + if (!file_named_minus) + 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[v3 PATCH 2/2] reset: add tests for git reset -
The failure case which occurs on teaching git is taught the '-' shorthand is when there exists no branch pointed to by '@{-1}'. But, if there is a file named - in the working tree, the user can be unambiguously assumed to be referring to it while issuing this command. The ambiguous case occurs when the @{-1} branch exists and file named '-' also exists in the working tree. This are also treated as a failure case but here the user is given advice as to how he can proceed. Another potentially tricky case is when the file '@{-1}' exists. In this case, the command should succeed as the user doesn't mention the file '@{-1}' and can be safely assumed to be referring to the @{-1} branch. Add tests to check the handling of these cases. Also add a test to verify that reset - behaves like reset @{-1} when none of the above cases are true. Helped-by: Junio C Hamano gits...@pobox.com Helped-by: Eric Sunshine sunsh...@sunshineco.com Helped-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Sundararajan R dyou...@gmail.com --- Thank you for your feedback Torsten and Eric. I have made modifications suggested by you. I have also acted on Matthieu's suggestions on the archive. Please let me know if there is something else I should add. I have also removed one irrelevant test from which we come to know of nothing new. t/t7102-reset.sh | 75 1 file changed, 75 insertions(+) diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh index 98bcfe2..f5a8e76 100755 --- a/t/t7102-reset.sh +++ b/t/t7102-reset.sh @@ -568,4 +568,79 @@ test_expect_success 'reset --mixed sets up work tree' ' test_cmp expect actual ' +test_expect_success 'reset - with no @{-1} branch should fail' ' + test_when_finished rm -rf new + git init new + ( + cd new + test_must_fail git reset - 2../actual + ) + test_i18ngrep bad flag actual +' + +test_expect_success 'reset - with no @{-1} branch and file named - should succeed' ' + test_when_finished rm -rf new + expected + git init new + ( + cd new + echo Hello - + git add - + git reset - ../actual + ) + test_cmp expected actual +' + +test_expect_success 'reset - with @{-1} branch and file named - should fail' ' + test_when_finished rm -rf new + git init new + ( + cd new + echo Hello - + git add - + git commit -m first_commit + git checkout -b new_branch + - + git add - + test_must_fail git reset - 2../actual + ) + test_i18ngrep ambiguous argument actual +' + +test_expect_success 'reset - with @{-1} branch and file named @{-1} should succeed' ' + test_when_finished rm -rf new + git init new + ( + cd new + echo Hello @{-1} + git add @{-1} + git commit -m first_commit + git checkout -b new_branch + @{-1} + git add @{-1} + git reset - ../actual + ) + test_i18ngrep Unstaged actual +' + +test_expect_success 'reset - with @{-1} branch and no file named - should succeed' ' + test_when_finished rm -rf new + git init new + ( + cd new + echo Hey new_file + git add new_file + git commit -m first_commit + git checkout -b new_branch + new_file + git add new_file + git reset - + git status -uno actual + git add new_file + git reset @{-1} + git status -uno expected + test_cmp actual expected + ) +' + test_done -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[v2 PATCH 2/2] reset: add tests for git reset -
The failure case which occurs on teaching git is taught the '-' shorthand is when there exists no branch pointed to by '@{-1}'. The ambiguous cases occur when there exist files named '-' or '@{-1}' in the work tree. These are also treated as failure cases but here the user is given advice as to how he can proceed. Add tests to check the handling of these cases. Also add a test to verify that reset - behaves like reset @{-1} when none of the above cases are true. Helped-by: Junio C Hamano gits...@pobox.com Helped-by: Torsten Bögershausen tbo...@web.de Helped-by: Eric Sunshine sunsh...@sunshineco.com Helped-by: Matthieu Moy matthieu@grenoble-inp.fr Signed-off-by: Sundararajan R dyou...@gmail.com --- Thank you for your feedback Torsten and Eric. I have now made the modifications suggested by you. I have also incorporated the suggestions given by Matthieu on the archive. Please let me know if there is something else I should add. t/t7102-reset.sh | 90 1 file changed, 90 insertions(+) diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh index 98bcfe2..c05dab0 100755 --- a/t/t7102-reset.sh +++ b/t/t7102-reset.sh @@ -568,4 +568,94 @@ test_expect_success 'reset --mixed sets up work tree' ' test_cmp expect actual ' +test_expect_success 'reset - with no @{-1} should fail' ' + git init new + ( + cd new + test_must_fail git reset - 2actual + ) + test_i18ngrep unknown revision new/actual + test_when_finished rm -rf new +' + +test_expect_success 'reset - with no @{-1} and file named - should fail' ' + git init new + ( + cd new + echo Hello - + git add - + test_must_fail git reset - 2actual + ) + test_i18ngrep both revision and filename new/actual + test_when_finished rm -rf new +' + +test_expect_success 'reset - with @{-1} and file named @{-1} should fail' ' + git init new + ( + cd new + echo Hello @{-1} + git add @{-1} + git commit -m first_commit + git checkout -b new_branch + @{-1} + git add @{-1} + test_must_fail git reset - 2actual + ) + test_i18ngrep both revision and filename new/actual + test_when_finished rm -rf new +' + +test_expect_success 'reset - with @{-1} and file named - should fail' ' + git init new + ( + cd new + echo Hello - + git add - + git commit -m first_commit + git checkout -b new_branch + - + git add - + test_must_fail git reset - 2actual + ) + test_i18ngrep both revision and filename new/actual + test_when_finished rm -rf new +' + +test_expect_success 'reset - with @{-1} and file named @{-1} and - should fail' ' + git init new + ( + cd new + - + git add - + git commit -m first_commit + git checkout -b new_branch + @{-1} + git add @{-1} + test_must_fail git reset - 2actual + ) + test_i18ngrep both revision and filename new/actual + test_when_finished rm -rf new +' + +test_expect_success 'reset - with @{-1} and no file named - or @{-1} should succeed' ' + git init new + ( + cd new + echo Hey new_file + git add new_file + git commit -m first_commit + git checkout -b new_branch + new_file + git add new_file + git reset - + git status -uno file1 + git add new_file + git reset @{-1} + git status -uno file2 + ) + test_cmp new/file1 new/file2 + test_when_finished rm -rf new +' + test_done -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[v2 PATCH 1/2] reset: add '-' shorthand for '@{-1}'
Teaching reset the - shorthand involves checking if any file named '-' exists because it then becomes ambiguous as to whether the user wants to reset the file '-' or if he wants to reset the working tree to the previous branch. check_filename() is used to perform this check. A similar ambiguity occurs when the file @{-1} exits. Therefore, when the files '-' or '@{-1}' exist then the program dies with a message about the ambiguous argument. Helped-by: Junio C Hamano gits...@pobox.com Helped-by: Eric Sunshine sunsh...@sunshineco.com Signed-off-by: Sundararajan R dyou...@gmail.com --- Have made the modifications suggest by you, Eric. Removed the part where the user is told that he can use ./- instead. builtin/reset.c | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/builtin/reset.c b/builtin/reset.c index 4c08ddc..88ce0c5 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; /* * 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, -)) + 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,13 @@ 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 '--' to separate paths from revisions, like this:\n + 'git command [revision...] -- [file...]')); + } + 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] Adding - shorthand for @{-1} in RESET command
Please give feedback and suggest things I may have missed out on. I hope I have incorporated all the suggestions. Signed-off-by: Sundararajan R dyou...@gmail.com Thanks-to: Junio C Hamano --- 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. 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; /* * 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,-)) + 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...]')); + } + 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] Added tests for git reset -
As you had suggested @Junio, I have added the required tests. Please let me know if there is something is I should add. Signed-off-by: Sundararajan R dyou...@gmail.com Thanks-to: Junio C Hamano --- I have added 6 tests to check for the following cases: git reset - with no @{-1} git reset - with no @{-1} and file named - git reset - with @{-1} and file named @{-1} git reset - with @{-1} and file named - git reset - with @{-1} and file named @{-1} and - git reset - with @{-1} and no file named - or @{-1} The 1st test with no previous branch results in the error The 2nd,3rd,4th and 5th result in the ambiguous argument error The 6th test has - working like @{-1} t/t7102-reset.sh | 107 +++ 1 file changed, 107 insertions(+) diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh index 98bcfe2..a670938 100755 --- a/t/t7102-reset.sh +++ b/t/t7102-reset.sh @@ -568,4 +568,111 @@ test_expect_success 'reset --mixed sets up work tree' ' test_cmp expect actual ' +test_expect_success 'reset - with no @{-1}' ' + git init new --quiet + cd new + test_must_fail git reset - actual + touch expect + test_cmp expect actual +' + +rm -rf new + +cat expect EOF +fatal: ambiguous argument '-': both revision and filename +Use ./- for file named - +Use '--' to separate paths from revisions, like this: +'git command [revision...] -- [file...]' +EOF + +test_expect_success 'reset - with no @{-1} and file named -' ' + git init new --quiet + cd new + echo Hello - + git add - + test_must_fail git reset - 2actual + test_cmp ../expect actual +' + +cd .. +rm -rf new + +cat expect EOF +fatal: ambiguous argument '@{-1}': both revision and filename +Use '--' to separate paths from revisions, like this: +'git command [revision...] -- [file...]' +EOF + +test_expect_success 'reset - with @{-1} and file named @{-1}' ' + git init new --quiet + cd new + echo Hello @{-1} + git add @{-1} + git commit -m first_commit + git checkout -b new_branch + touch @{-1} + git add @{-1} + test_must_fail git reset - 2actual + test_cmp ../expect actual +' + +cd .. +rm -rf new + +cat expect EOF +fatal: ambiguous argument '-': both revision and filename +Use ./- for file named - +Use '--' to separate paths from revisions, like this: +'git command [revision...] -- [file...]' +EOF + +test_expect_success 'reset - with @{-1} and file named - ' ' + git init new --quiet + cd new + echo Hello - + git add - + git commit -m first_commit + git checkout -b new_branch + touch - + git add - + test_must_fail git reset - 2actual + test_cmp ../expect actual +' + +cd .. +rm -rf new + +test_expect_success 'reset - with @{-1} and file named @{-1} and - ' ' + git init new --quiet + cd new + echo Hello - + git add - + git commit -m first_commit + git checkout -b new_branch + echo Hello @{-1} + git add @{-1} + test_must_fail git reset - 2actual + test_cmp ../expect actual +' + +cd .. +rm -rf new + +test_expect_success 'reset - with @{-1} and no file named - or @{-1} ' ' + git init new --quiet + cd new + echo Hello new_file + git add new_file + git commit -m first_commit + git checkout -b new_branch + echo Hey new_file + git add new_file + git reset - + git status -uno file1 + git add new_file + git reset @{-1} + git status -uno file2 + test_cmp file1 file2 +' + test_done -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [GSoC Microproject]Adding - shorthand for @{-1} in RESET command
On Sun, Mar 8, 2015 at 1:04 PM Junio C Hamano gits...@pobox.com wrote: Sundararajan R dyou...@gmail.com writes: diff --git a/builtin/reset.c b/builtin/reset.c index 4c08ddc..62764d4 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -203,8 +203,16 @@ static void parse_args(struct pathspec *pathspec, * * At this point, argv points immediately after [-opts]. */ - + int flag=0; /* + * - may refer to filename in which case we should be giving more precedence + * to filename than equating argv[0] to @{-1} + */ Comment on a separate line. More importantly, think if you can give the variable a more meaningful name so that you do not have to explain. You are missing SPs requested by the coding guideline everywhere in your patch. 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], --)) { @@ -226,6 +234,8 @@ static void parse_args(struct pathspec *pathspec, Around here not shown by this patch there are a few uses of argv[0], and the most important one is verify_non_filename(prefix, argv[0]); just before the line below (see below). rev = *argv++; } else { /* Otherwise we treat this as a filename */ + if(flag) + argv[0]=-; verify_filename(prefix, argv[0], 1); } } By the way, do you understand the intent of the existing checks in this codepath that uses verify_filename() and verify_non_filename()? The idea is to allow users to write git reset X and git reset Y Z safely in an unambiguous way. * X could be a commit (e.g. git reset master), to update the current branch to point at the same commit as 'master' and update the index to match. * X could be a pathspec (e.g. git reset hello.c), to grab the blob object for X out of the HEAD and put it in the index. * Y could be a tree-ish and Z a pathspec (e.g. git reset HEAD^ hello.c), to grab the blob object for Z out of tree-ish Y and put it to the index. * Both Y and Z could be pathspecs (e.g. git reset hello.c goodbye.c), to revert the index entries for these two paths to what the HEAD records. If you happen to have a file whose name is 'master', and if you are working on your 'topic' branch, what would this do? $ git reset master Is this a request to revert the index entry for path 'master' from the HEAD? Or is it a request to update the current branch to be the same as the 'master' branch and repopulate the index from there? What does the existing code try to do, and how does it do it? It detects the ambiguity and refuses to do either, to make sure it causes no harm. Now, with your change, does the result still honor this when ambiguous, stop without causing harm to the user principle? What happens when your user has a file whose name is - in the working tree? What happens when your user has a file whose name is @{-1} in the working tree? -- Hi all, 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. 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? Thanks a lot. R Sundararajan. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [GSoC Microproject]Adding - shorthand for @{-1} in RESET command
Hi all, I am a GSoC '15 aspirant for git. In this commit I have directly associated - to @{-1} except when it refers to a filename. All the given tests pass(except those which shouldn't). I have to add a failsafe for the case in when there is no branch as @{-1}. For this I have a rough idea that I would have to call get-sha1() on @{-1} to check if there is an object matching with it. But I am not able to think of the details. Please guide me with that and give feedback for this patch. Signed-off-by: Sundararajan R dyou...@gmail.com --- builtin/reset.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/builtin/reset.c b/builtin/reset.c index 4c08ddc..62764d4 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -203,8 +203,16 @@ static void parse_args(struct pathspec *pathspec, * * At this point, argv points immediately after [-opts]. */ - + int flag=0; /* +* - may refer to filename in which case we should be giving more precedence +* to filename than equating argv[0] to @{-1} +*/ 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], --)) { @@ -226,6 +234,8 @@ static void parse_args(struct pathspec *pathspec, rev = *argv++; } else { /* Otherwise we treat this as a filename */ + if(flag) + argv[0]=-; verify_filename(prefix, argv[0], 1); } } -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GSoC microproject help] Allow - as a short-hand for @{-1} in more places
Hi All, I am a sophomore at Indian Institute of Technology Kanpur and am a GSoC aspirant for git. Although I have been using git from a long time, this is the first occasion when I have picked up reading its source code. Can somebody please help me by telling me how to start off with the above mentioned microproject? Thank you very much. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html