Re: [PATCH v5 1/2] reset: enable '-' short-hand for previous branch
Thank you all for your reviews and feedback. I will try and submit
this patch by taking my time to think about the various solutions and
coming up with the best one. I will also try and see if I can assist
Junio with his JFF patch.
I have learned a lot during the process of implementing this micro
project and believe that if I get selected for a gsoc under git, it
will help me become a better developer overall.
I have gone over the ideas page and I am interested in the git bisect
--first-parent and git bisect fixed/unfixed projects. I am looking
the source code at git-bisect.sh and will try and come up with a
proposal for review by the git community.
Thank you all for you time and patience.
Regards,
Sudhanshu
On Sat, Mar 14, 2015 at 2:18 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Fri, Mar 13, 2015 at 2:18 PM, Sudhanshu Shekhar
sudshekha...@gmail.com wrote:
git reset '-' will reset to the previous branch. To reset a file named
- use either git reset ./- or git reset -- -.
Change error message to treat single - as an ambigous revision or
path rather than a bad flag.
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: Sudhanshu Shekhar sudshekha...@gmail.com
---
I have changed the logic to ensure argv[0] isn't changed at any point.
Creating a modified_argv0 variable let's me do that.
I couldn't think of any other way to achieve this, apart from changing things
directly in the sha1_name.c file (like Junio's changes). Please let me know
if some further changes are needed.
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..bc50e14 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];
+ const char *modified_argv0 = argv[0];
This variable is only needed inside the 'if (argv[0])' block below, so
its declaration should be moved there. Also the initialization to
argv[0] is wasted since the assignment is overwritten below.
The variable name itself could be better. Unlike a name such as
'orig_arg0', modified doesn't tell us much. Modified how? Modified
to be what? Consideration where and how the variable is used, we can
see that it will be holding a value that _might_ be a rev. This
suggests a better name such as 'maybe_rev' or something similar.
/*
* Possible arguments are:
*
@@ -205,10 +206,17 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if (!strcmp(argv[0], -)) {
+ modified_argv0 = @{-1};
+ }
+ else {
Style: cuddle the 'else' and braces: } else {
+ modified_argv0 = argv[0];
+ }
The unnecessary braces make this harder to read than it could be since
it is so spread out vertically. Dropping the braces would help. The
ternary operator ?: might improve readability (though it might also
make it worse).
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
- rev = argv[0];
+ rev = modified_argv0;
argv += 2;
}
/*
@@ -216,14 +224,15 @@ static void parse_args(struct pathspec *pathspec,
* 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))) {
+ else if ((!argv[1] !get_sha1_committish(modified_argv0,
unused)) ||
+(argv[1] !get_sha1_treeish(modified_argv0,
unused))) {
/*
* Ok, argv[0] looks like a commit/tree; it should
not
* be a filename.
*/
verify_non_filename(prefix, argv[0]);
- rev = *argv++;
+ rev = modified_argv0;
+ argv++;
Good. This is much better than the previous rounds, and is the sort of
solution I had hoped to see when prodding you in previous reviews to
avoid argv[] kludges. Unlike the previous band-aid approach, this
demonstrates that you took the time to understand the overall logic
flow rather than merely plopping in a quick fix.
} else {
/* Otherwise we treat this as a filename */
verify_filename(prefix, argv[0], 1);
diff --git a/setup.c b/setup.c
index 979b13f..b621b62 100644
--- a/setup.c
+++ b/setup.c
@@ -200,7 +200,7 @@ void
Re: [PATCH v5 2/2] t7102: add 'reset -' tests
Hi,
On Sat, Mar 14, 2015 at 2:40 AM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Fri, Mar 13, 2015 at 2:18 PM, Sudhanshu Shekhar
sudshekha...@gmail.com wrote:
Add following test cases:
1) Confirm error message when git reset is used with no previous branch
2) Confirm git reset - works like git reset @{-1}
3) Confirm - is always treated as a commit unless the -- file option
is specified
4) Confirm git reset - works normally even when a file named @{-1} is
present
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Helped-by: David Aguilar dav...@gmail.com
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
---
Eric: Thank you for pointing out the mistake. The '' after the Here
Docs was causing the issue. I have removed the concatenation from
there, hope that's okay.
The needs to go on the first line, not the last line of the here-doc.
However, that was not my main concern in the previous review. What
disturbed me was that the new tests, which were supposed to be
checking if - behaved as @{-1}, were succeeding even without patch
1/2 applied which implemented the - alias for @{-1}. That seems
wrong. I don't think you particularly addressed that issue in this
version (even though the first couple tests will now fail without 1/2
due to the changed error message).
Actually, The issue was caused due a HERE docs error. If you run this
patch now, you will see that 7 out of the 8 test cases fail.
Regarding the @{-1} test case, I created it as a check for Junio's
comment on the error message generated by git reset - when a file
named @{-1} is there. Since, in this situation git reset @{-1} will
return an error (but reset - shouldn't).
Reminder: Wrap commentary to about column 72, as you would the commit
message. (I re-wrapped it manually to reply to it.)
I have renamed the folder to 'dash' as suggested by you, keeping the
old name only where it made sense.
Considering that the test titles already tell us the intent of the
tests, I don't find that the directory name no_previous adds much
value to tests checking the behavior of - with no previous branch. A
single, consistent name used throughout all these tests would be less
surprising and place smaller cognitive load on the reader.
More below.
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..18523c1 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,162 @@ test_expect_success 'reset --mixed sets up work tree' '
test_cmp expect actual
'
+test_expect_success 'reset - with no previous branch fails' '
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ test_must_fail git reset - 2actual
+ )
+ test_i18ngrep ambiguous argument no_previous/actual
+'
+
+test_expect_success 'reset - while having file named - and no previous
branch fails' '
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ -
+ test_must_fail git reset - 2actual
+ )
+ test_i18ngrep ambiguous argument no_previous/actual
+'
+
+
Style: Unnecessary extra blank line.
+test_expect_success \
+ 'reset - in the presence of file named - with previous branch resets
commit' '
+ cat expect -EOF
Place the at the end of this line. Also, prefix EOF with a
backslash to indicate that you don't intend any interpolation to occur
within the here-doc. So:
cat expect -\EOF
Ditto for the remaining tests.
Thank you for taking the time out to point out these style changes to
me. There is a lot I have to learn about open source contribution yet
and I believe during the course of this microproject, I did learn
something about this (By making some very silly mistakes). Thank you
to all the developers who took time out to review my code and point
out the mistakes I had done.
Regards,
Sudhanshu
--
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 v5 2/2] t7102: add 'reset -' tests
Add following test cases:
1) Confirm error message when git reset is used with no previous branch
2) Confirm git reset - works like git reset @{-1}
3) Confirm - is always treated as a commit unless the -- file option
is specified
4) Confirm git reset - works normally even when a file named @{-1} is
present
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Helped-by: David Aguilar dav...@gmail.com
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
---
Eric: Thank you for pointing out the mistake. The '' after the Here Docs
was causing the issue. I have removed the concatenation from there, hope
that's okay.
Regarding the @{-1} test case, I created it as a check for Junio's comment
on the error message generated by git reset - when a file named @{-1} is
there.
Since, in this situation git reset @{-1} will return an error (but reset -
shouldn't).
I have renamed the folder to 'dash' as suggested by you, keeping the old name
only
where it made sense.
t/t7102-reset.sh | 158 +++
1 file changed, 158 insertions(+)
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..18523c1 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,162 @@ test_expect_success 'reset --mixed sets up work tree' '
test_cmp expect actual
'
+test_expect_success 'reset - with no previous branch fails' '
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ test_must_fail git reset - 2actual
+ )
+ test_i18ngrep ambiguous argument no_previous/actual
+'
+
+test_expect_success 'reset - while having file named - and no previous branch
fails' '
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ -
+ test_must_fail git reset - 2actual
+ )
+ test_i18ngrep ambiguous argument no_previous/actual
+'
+
+
+test_expect_success \
+ 'reset - in the presence of file named - with previous branch resets
commit' '
+ cat expect -EOF
+ Unstaged changes after reset:
+ M -
+ M file
+ EOF
+ git init dash
+ test_when_finished rm -rf dash
+ (
+ cd dash
+ -
+ file
+ git add file -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random -
+ echo wow file
+ git add file -
+ git reset - ../actual
+ )
+ test_cmp expect actual
+'
+test_expect_success \
+ 'reset - in the presence of file named - with -- option resets commit' '
+ cat expect -EOF
+ Unstaged changes after reset:
+ M -
+ M file
+ EOF
+ git init dash
+ test_when_finished rm -rf dash
+ (
+ cd dash
+ -
+ file
+ git add file -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random -
+ echo wow file
+ git add file -
+ git reset - -- ../actual
+ )
+ test_cmp expect actual
+'
+
+test_expect_success 'reset - in the presence of file named - with -- file
option resets file' '
+ cat expect -EOF
+ Unstaged changes after reset:
+ M -
+ EOF
+ git init dash
+ test_when_finished rm -rf dash
+ (
+ cd dash
+ -
+ file
+ git add file -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random -
+ echo wow file
+ git add file -
+ git reset -- - ../actual
+ )
+ test_cmp expect actual
+'
+test_expect_success \
+ 'reset - in the presence of file named - with both pre and post --
option resets file' '
+ cat expect -EOF
+ Unstaged changes after reset:
+ M -
+ EOF
+ git init dash
+ test_when_finished rm -rf dash
+ (
+ cd dash
+ -
+ file
+ git add file -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random -
+ echo wow file
+ git add file -
+ git reset - -- - ../actual
+ )
+ test_cmp expect actual
+'
+
+test_expect_success 'reset - works same as reset @{-1}' '
+ git init dash
+ test_when_finished rm -rf dash
+ (
+ cd dash
+ echo file1 file1
+ git add file1
+ git commit -m base commit
+ git checkout -b temp
+ echo new file file
[PATCH v5 1/2] reset: enable '-' short-hand for previous branch
git reset '-' will reset to the previous branch. To reset a file named
- use either git reset ./- or git reset -- -.
Change error message to treat single - as an ambigous revision or
path rather than a bad flag.
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: Sudhanshu Shekhar sudshekha...@gmail.com
---
I have changed the logic to ensure argv[0] isn't changed at any point.
Creating a modified_argv0 variable let's me do that.
I couldn't think of any other way to achieve this, apart from changing things
directly in the sha1_name.c file (like Junio's changes). Please let me know
if some further changes are needed.
builtin/reset.c | 17 +
setup.c | 2 +-
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..bc50e14 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];
+ const char *modified_argv0 = argv[0];
/*
* Possible arguments are:
*
@@ -205,10 +206,17 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if (!strcmp(argv[0], -)) {
+ modified_argv0 = @{-1};
+ }
+ else {
+ modified_argv0 = argv[0];
+ }
+
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
- rev = argv[0];
+ rev = modified_argv0;
argv += 2;
}
/*
@@ -216,14 +224,15 @@ static void parse_args(struct pathspec *pathspec,
* 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))) {
+ else if ((!argv[1] !get_sha1_committish(modified_argv0,
unused)) ||
+(argv[1] !get_sha1_treeish(modified_argv0,
unused))) {
/*
* Ok, argv[0] looks like a commit/tree; it should not
* be a filename.
*/
verify_non_filename(prefix, argv[0]);
- rev = *argv++;
+ rev = modified_argv0;
+ argv++;
} else {
/* Otherwise we treat this as a filename */
verify_filename(prefix, argv[0], 1);
diff --git a/setup.c b/setup.c
index 979b13f..b621b62 100644
--- a/setup.c
+++ b/setup.c
@@ -200,7 +200,7 @@ void verify_filename(const char *prefix,
int diagnose_misspelt_rev)
{
if (*arg == '-')
- die(bad flag '%s' used after filename, arg);
+ die(ambiguous argument '%s': unknown revision or path, arg);
if (check_filename(prefix, arg))
return;
die_verify_filename(prefix, arg, diagnose_misspelt_rev);
--
2.3.1.277.gd67f9d5.dirty
--
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 v3 2/2] Added tests for reset -
Added the following test cases:
1) Confirm error message when git reset is used with no previous branch
2) Confirm git reset - works like git reset @{-1}
3) Confirm - is always treated as a commit unless the -- file option
is specified
4) Confirm git reset - works normally even when a file named @{-1} is
present
Helped-by: David Aguilar dav...@gmail.com
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
---
I have tried to keep each test self sufficient. Please let me know if any
changes are required.
Thank you!
t/t7102-reset.sh | 139 +++
1 file changed, 139 insertions(+)
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..0faf241 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,143 @@ test_expect_success 'reset --mixed sets up work tree' '
test_cmp expect actual
'
+test_expect_success 'reset - with no previous branch' '
+ git init no_previous
+ (
+ cd no_previous
+ test_must_fail git reset - 2output
+ )
+ test_i18ngrep bad flag no_previous/output
+'
+
+test_expect_success 'reset - while having file named - and no previous branch'
'
+ git init no_previous
+ (
+ cd no_previous
+ ./-
+ test_must_fail git reset - 2output
+ )
+ test_i18ngrep bad flag no_previous/output
+'
+
+
+test_expect_success 'reset - in the presence of file named - with previous
branch' '
+ echo Unstaged changes after reset: expect
+ echo M - expect
+ echo M 1 expect
+ git init no_previous
+ (
+ cd no_previous
+ ./-
+ 1
+ git add 1 -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random ./-
+ echo wow 1
+ git add 1 -
+ git reset - ../output
+ )
+ rm -rf no_previous
+ test_cmp output expect
+'
+test_expect_success 'reset - in the presence of file named - with -- option' '
+ echo Unstaged changes after reset: expect
+ echo M - expect
+ echo M 1 expect
+ git init no_previous
+ (
+ cd no_previous
+ ./-
+ 1
+ git add 1 -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random ./-
+ echo wow 1
+ git add 1 -
+ git reset - -- ../output
+ )
+ rm -rf no_previous
+ test_cmp output expect
+'
+
+test_expect_success 'reset - in the presence of file named - with -- file
option' '
+ echo Unstaged changes after reset: expect
+ echo M - expect
+ git init no_previous
+ (
+ cd no_previous
+ ./-
+ 1
+ git add 1 -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random ./-
+ echo wow 1
+ git add 1 -
+ git reset -- - ../output
+ )
+ rm -rf no_previous
+ test_cmp output expect
+'
+test_expect_success 'reset - in the presence of file named - with both pre and
post -- option' '
+ echo Unstaged changes after reset: expect
+ echo M - expect
+ git init no_previous
+ (
+ cd no_previous
+ ./-
+ 1
+ git add 1 -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random ./-
+ echo wow 1
+ git add 1 -
+ git reset - -- - ../output
+ )
+ rm -rf no_previous
+ test_cmp output expect
+'
+
+test_expect_success 'reset - works same as reset @{-1}' '
+ git init no_previous
+ (
+ cd no_previous
+ echo random random
+ git add random
+ git commit -m base commit
+ git checkout -b temp
+ echo new-file new-file
+ git add new-file
+ git commit -m added new-file
+ git reset -
+ git status --porcelain ../first
+ git add new-file
+ git commit -m added new-file
+ git reset @{-1}
+ git status --porcelain ../second
+ )
+ test_cmp first second
+'
+
+test_expect_success 'reset - with file named @{-1}' '
+ echo Unstaged changes after reset: expect
+ echo M @{-1} expect
+ git init no_previous
+ (
+ cd no_previous
+ echo random ./@{-1}
+ git add ./@{-1}
+ git commit -m base commit
+ git checkout -b new_branch
+ echo additional stuff ./@{-1
[PATCH v3 1/2] reset: enable '-' short-hand for previous branch
'git reset -' will reset to the previous branch. It will behave similar
to @{-1} except when a file named '@{-1}' is present. To refer to a file
named '-', use ./- or the -- flag.
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: Sudhanshu Shekhar sudshekha...@gmail.com
---
Thank you all for your feedback. I have made changes and I hope this patch
meets community standards. Please let me know if any further changes are
required.
Regards,
Sudhanshu
builtin/reset.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..8abd300 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 substituted_minus = 0;
/*
* Possible arguments are:
*
@@ -205,6 +206,10 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if (!strcmp(argv[0], -)) {
+ argv[0] = @{-1};
+ substituted_minus = 1;
+ }
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
@@ -222,15 +227,21 @@ static void parse_args(struct pathspec *pathspec,
* Ok, argv[0] looks like a commit/tree; it should not
* be a filename.
*/
+ if (substituted_minus)
+ argv[0] = -;
verify_non_filename(prefix, argv[0]);
+ if (substituted_minus)
+ argv[0] = @{-1};
rev = *argv++;
} else {
+ /* We were treating - as a commit and not a file */
+ if (substituted_minus)
+ argv[0] = -;
/* Otherwise we treat this as a filename */
verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
-
if (read_cache() 0)
die(_(index file corrupt));
--
2.3.1.279.gd534259
--
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 v3 2/2] Added tests for reset -
Hi, On Tue, Mar 10, 2015 at 6:56 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Sudhanshu Shekhar sudshekha...@gmail.com writes: +test_expect_success 'reset - while having file named - and no previous branch' ' I like having the expected behavior in the test name too. e.g. add fails at the end of the sentence. Sure. I will update this in the next patch. +test_expect_success 'reset - in the presence of file named - with previous branch' ' + echo Unstaged changes after reset: expect + echo M - expect + echo M 1 expect Here and elsewhere: why not cat expect -EOF Unstaged changes after reset: M - M 1 ? I was confused whether to use cat or echo. I thought using cat will disrupt the indentation as the EOF needs to be on a single line. This is why I chose echo. Please let me know your thoughts on this. + rm -rf no_previous That would be best done in a test_when_finished, so that the directory is removed regardless of whether the test failed before this line or not. Thanks for pointing this out. I will update this accordingly. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ Regards, Sudhanshu -- 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 v2 1/2] Teach reset the same short-hand as checkout
Hi, On Tue, Mar 10, 2015 at 8:04 AM, Junio C Hamano gits...@pobox.com wrote: Sudhanshu Shekhar sudshekha...@gmail.com writes: - now means the previous branch. Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com Thanks-to: Eric Sunshine, Junio C Hamano, Matthieu Moy --- These look unusual for a few reasons: your S-o-b should be at the end, we usually say Helped-by: instead, and we do not use these with multiple names on a single line. Please do not try to be original without a good reason. We may start counting the number of times people appear on these footers to see how much contribution those who do not directly author commits (read: those who mentor others) are making. Thank you for telling me this. I will it keep it in mind from next time onwards. builtin/reset.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) The comment I gave in the thread that ends at $gmane/265112 would apply equally to this patch, I think. cf. http://thread.gmane.org/gmane.comp.version-control.git/264986/focus=265112 I have rectified this in my new patch and will send it soon. Kindly do let me know if there are any other changes required. Thank you. Regards, Sudhanshu -- 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 v3 1/2] reset: enable '-' short-hand for previous branch
git reset -' will reset to the previous branch. It will behave similar
to @{-1} except when a file named '@{-1}' is present. To refer to a file
named '-', use ./- or the -- flag.
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: Sudhanshu Shekhar sudshekha...@gmail.com
---
Eric, I have added a user_input variable to record the input entered by the
user. This way I can avoid the multiple 'if' clauses. Thank you for the
suggestion.
I have also removed the unrelated change that I had unintentionally committed.
I am sending this patch on the thread for further review. Once both the patches
are reviewed and accepted, I will create a new mail for it. Hope that is okay.
Regards,
Sudhanshu
builtin/reset.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..b428241 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 substituted_minus = 0;
+ char *user_input = argv[0];
/*
* Possible arguments are:
*
@@ -205,6 +207,10 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if (!strcmp(argv[0], -)) {
+ argv[0] = @{-1};
+ substituted_minus = 1;
+ }
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
@@ -222,9 +228,12 @@ 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]);
+ verify_non_filename(prefix, user_input);
rev = *argv++;
} else {
+ /* We were treating - as a commit and not a file */
+ if (substituted_minus)
+ argv[0] = -;
/* Otherwise we treat this as a filename */
verify_filename(prefix, argv[0], 1);
}
--
2.3.1.278.ge5c7b1f.dirty
--
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 v3 2/2] t7102: add 'reset -' tests
Add following test cases:
1) Confirm error message when git reset is used with no previous branch
2) Confirm git reset - works like git reset @{-1}
3) Confirm - is always treated as a commit unless the -- file option is
specified
4) Confirm git reset - works normally even when a file named @{-1} is present
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Helped-by: David Aguilar dav...@gmail.com
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
---
First of all, thank you for being so incredibly patient and helpful. I am very
grateful for your remarks and reviews. I have implemented your suggestions in
this patch. Please let me know if I have missed out on anything else. Also,
sorry for sending PATCH 1/2 on the wrong thread, I entered the Message-ID
incorrectly (still getting used to send-email :/ ).
Regards,
Sudhanshu
t/t7102-reset.sh | 159 +++
1 file changed, 159 insertions(+)
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..d3a5874 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,163 @@ test_expect_success 'reset --mixed sets up work tree' '
test_cmp expect actual
'
+test_expect_success 'reset - with no previous branch fails' '
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ test_must_fail git reset - 2actual
+ )
+ test_i18ngrep bad flag no_previous/actual
+'
+
+test_expect_success 'reset - while having file named - and no previous branch
fails' '
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ -
+ test_must_fail git reset - 2actual
+ )
+ test_i18ngrep bad flag no_previous/actual
+'
+
+test_expect_success \
+ 'reset - in the presence of file named - with previous branch resets
commit' '
+ cat expect -\EOF
+ Unstaged changes after reset:
+ M -
+ M file
+ EOF
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ -
+ file
+ git add file -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random -
+ echo wow file
+ git add file -
+ git reset - ../actual
+ )
+ test_cmp expect actual
+'
+
+test_expect_success \
+ 'reset - in the presence of file named - with -- option resets commit' '
+ cat expect -\EOF
+ Unstaged changes after reset:
+ M -
+ M file
+ EOF
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ -
+ file
+ git add file -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random -
+ echo wow file
+ git add file -
+ git reset - -- ../actual
+ )
+ test_cmp expect actual
+'
+
+test_expect_success 'reset - in the presence of file named - with -- file
option resets file' '
+ cat expect -\EOF
+ Unstaged changes after reset:
+ M -
+ M file
+ EOF
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ -
+ file
+ git add file -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random -
+ echo wow file
+ git add file -
+ git reset -- - ../actual
+ )
+ test_cmp expect actual
+'
+
+test_expect_success \
+ 'reset - in the presence of file named - with both pre and post --
option resets file' '
+ cat expect -\EOF
+ Unstaged changes after reset: expect
+ M -
+ EOF
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ -
+ file
+ git add file -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random -
+ echo wow file
+ git add file -
+ git reset - -- - ../actual
+ )
+ test_cmp expect actual
+'
+
+test_expect_success 'reset - works same as reset @{-1}' '
+ git init no_previous
+ test_when_finished rm -rf no_previous
+ (
+ cd no_previous
+ echo file1 file1
+ git add file1
+ git commit -m base commit
+ git checkout -b temp
+ echo new file file
[PATCH v3 1/2] reset: enable '-' short-hand for previous branch
git reset -' will reset to the previous branch. It will behave similar
to @{-1} except when a file named '@{-1}' is present. To refer to a file
named '-', use ./- or the -- flag.
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: Sudhanshu Shekhar sudshekha...@gmail.com
---
Eric, I have added a user_input variable to record the input entered by the
user. This way I can avoid the multiple 'if' clauses. Thank you for the
suggestion.
I have also removed the unrelated change that I had unintentionally committed.
I am sending this patch on the thread for further review. Once both the patches
are reviewed and accepted, I will create a new mail for it. Hope that is okay.
Regards,
Sudhanshu
builtin/reset.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..b428241 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 substituted_minus = 0;
+ char *user_input = argv[0];
/*
* Possible arguments are:
*
@@ -205,6 +207,10 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if (!strcmp(argv[0], -)) {
+ argv[0] = @{-1};
+ substituted_minus = 1;
+ }
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
@@ -222,9 +228,12 @@ 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]);
+ verify_non_filename(prefix, user_input);
rev = *argv++;
} else {
+ /* We were treating - as a commit and not a file */
+ if (substituted_minus)
+ argv[0] = -;
/* Otherwise we treat this as a filename */
verify_filename(prefix, argv[0], 1);
}
--
2.3.1.278.ge5c7b1f.dirty
--
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 v2 2/2] Added test cases for git reset -
1) Confirm error message when git reset is used with no previous branch
2) Confirm git reset - works like git reset @{-1}
3) Confirm - is always treated as a commit unless the -- file option
is specified
Signed-off-by: Sudhanshu Shekharsudshekha...@gmail.com
Thanks-to: David Aguilardav...@gmail.com
---
David, thank you for your remarks. I have tried to incorporate your suggestions
into this patch.
Please let me know if I have missed out on anything else.
t/t7102-reset.sh | 121 +++
1 file changed, 121 insertions(+)
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..fe43f64 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,125 @@ test_expect_success 'reset --mixed sets up work tree' '
test_cmp expect actual
'
+cat expect EOF
+fatal: bad flag '-' used after filename
+EOF
+
+test_expect_success 'reset - with no previous branch' '
+ git init no_previous --quiet
+ (
+ cd no_previous
+ test_must_fail git reset - 2output
+ )
+ test_cmp expect no_previous/output
+'
+
+test_expect_success 'reset - while having file named - and no previous branch'
'
+ git init no_previous --quiet
+ (
+ cd no_previous
+ touch ./-
+ test_must_fail git reset - 2output
+ )
+ test_cmp expect no_previous/output
+'
+
+cat expect EOF
+Unstaged changes after reset:
+M -
+M 1
+EOF
+
+test_expect_success 'reset - in the prescence of file named - with previous
branch' '
+ git init no_previous --quiet
+ (
+ cd no_previous
+ touch ./- 1
+ git add 1 -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random ./-
+ echo wow 1
+ git add 1 -
+ git reset - ../output
+ )
+ rm -rf no_previous
+ test_cmp output expect
+'
+test_expect_success 'reset - in the presence of file named - with -- option' '
+ git init no_previous --quiet
+ (
+ cd no_previous
+ touch ./- 1
+ git add 1 -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random ./-
+ echo wow 1
+ git add 1 -
+ git reset - -- ../output
+ )
+ rm -rf no_previous
+ test_cmp output expect
+'
+
+cat expect EOF
+Unstaged changes after reset:
+M -
+EOF
+
+test_expect_success 'reset - in the presence of file named - with -- file
option' '
+ git init no_previous --quiet
+ (
+ cd no_previous
+ touch ./- 1
+ git add 1 -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random ./-
+ echo wow 1
+ git add 1 -
+ git reset -- - ../output
+ )
+ rm -rf no_previous
+ test_cmp output expect
+'
+test_expect_success 'reset - in the presence of file named - with both pre and
post -- option' '
+ git init no_previous --quiet
+ (
+ cd no_previous
+ touch ./- 1
+ git add 1 -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random ./-
+ echo wow 1
+ git add 1 -
+ git reset - -- - ../output
+ )
+ rm -rf no_previous
+ test_cmp output expect
+'
+
+test_expect_success 'reset - works same as reset @{-1}' '
+ git init no_previous --quiet
+ (
+ cd no_previous
+ echo random random
+ git add random
+ git commit -m base commit
+ git checkout -b temp
+ echo new-file new-file
+ git add new-file
+ git commit -m added new-file
+ git reset -
+ git status --porcelain ../first
+ git add new-file
+ git commit -m added new-file
+ git reset @{-1}
+ git status --porcelain ../second
+ )
+ test_cmp first second
+'
+
test_done
--
2.3.1.168.g0c82976.dirty
--
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 v2 1/2] Teach reset the same short-hand as checkout
- now means the previous branch.
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
Thanks-to: Eric Sunshine, Junio C Hamano, Matthieu Moy
---
builtin/reset.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..02f33ef 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 substituted_minus = 0;
/*
* Possible arguments are:
*
@@ -205,6 +206,10 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if (!strcmp(argv[0], -)) {
+ argv[0] = @{-1};
+ substituted_minus = 1;
+ }
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
@@ -225,12 +230,14 @@ static void parse_args(struct pathspec *pathspec,
verify_non_filename(prefix, argv[0]);
rev = *argv++;
} else {
+ /* We were treating - as a commit and not a file */
+ if (substituted_minus)
+ argv[0] = -;
/* Otherwise we treat this as a filename */
verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
-
if (read_cache() 0)
die(_(index file corrupt));
--
2.3.1.168.g0c82976.dirty
--
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] Teach reset the same short-hand as checkout
- now means the previous branch.
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
Thanks-to: Junio C Hamano, Matthieu Moy, Eric Sunshine
---
Thank you all for your feedback. Please let me know if I am missing out on
anything else.
builtin/reset.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..02f33ef 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 substituted_minus = 0;
/*
* Possible arguments are:
*
@@ -205,6 +206,10 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if (!strcmp(argv[0], -)) {
+ argv[0] = @{-1};
+ substituted_minus = 1;
+ }
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
@@ -225,12 +230,14 @@ static void parse_args(struct pathspec *pathspec,
verify_non_filename(prefix, argv[0]);
rev = *argv++;
} else {
+ /* We were treating - as a commit and not a file */
+ if (substituted_minus)
+ argv[0] = -;
/* Otherwise we treat this as a filename */
verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
-
if (read_cache() 0)
die(_(index file corrupt));
--
2.3.1.168.g0c82976.dirty
--
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 test cases for git reset -
Four test cases have been added
1) when user does reset - without any previous branch = Leads to error
2) when user does reset - with a previous branch = Ensure it
behaves like at {-1}
Other two deal with the situation when we have a file named '-'. We
ignore such a file and - is always treated either as a previous branch
or a bad filename. Users who wish to reset a file named '-' should
specify
it as './-'
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
---
I have created test cases for git reset -. @Junio, I tried incorporating your
suggestions while developing these test cases.
However, since the verify_filename function ignores files starting with -,
git reset - will always refer to the branch only. Kindly let me know your
thoughts and views on this and also your reviews about the test cases I have
created.
Regards,
Sudhanshu
t/t7102-reset.sh | 62
1 file changed, 62 insertions(+)
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..ade3d6a 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,66 @@ test_expect_success 'reset --mixed sets up work tree' '
test_cmp expect actual
'
+cat expect EOF
+fatal: bad flag '-' used after filename
+EOF
+
+test_expect_success 'reset - with no previous branch' '
+ git init no_previous --quiet
+ (
+ cd no_previous
+ )
+ test_must_fail git reset - 2output
+ test_cmp expect output
+'
+
+test_expect_success 'reset - while having file named - and no previous branch'
'
+ git init no_previous --quiet
+ (
+ cd no_previous
+ touch ./-
+ )
+ test_must_fail git reset - 2output
+ test_cmp expect output
+'
+
+cat expect EOF
+Unstaged changes after reset:
+M -
+M 1
+EOF
+
+test_expect_success 'reset - in the prescence of file named - with previou
branch' '
+ git init no_previous --quiet
+ cd no_previous
+ touch ./- 1
+ git add 1 -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random ./-
+ echo wow 1
+ git add 1 -
+ git reset - output
+ test_cmp output ../expect
+'
+test_expect_success 'reset - works same as reset @{-1}' '
+ git init no_previous --quiet
+ cd no_previous
+ echo random random
+ git add random
+ git commit -m base commit
+ git checkout -b temp
+ echo new-file new-file
+ git add new-file
+ git commit -m added new-file
+ git reset -
+
+ git status ../first
+ git add new-file
+ git commit -m added new-file
+ git reset @{-1}
+ git status ../second
+ test_cmp ../first ../second
+'
+
test_done
--
2.3.1.168.g0c82976.dirty
--
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] reset: allow - short hand for previous commit
Teach reset the same shorthand as checkout and merge. - means the
previous commit.
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
---
builtin/reset.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..9f8967d 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 substituted_minus = 0;
/*
* Possible arguments are:
*
@@ -205,6 +206,10 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if(!strcmp(argv[0], -)) {
+ argv[0] = @{-1};
+ substituted_minus = 1;
+ }
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
@@ -225,12 +230,14 @@ static void parse_args(struct pathspec *pathspec,
verify_non_filename(prefix, argv[0]);
rev = *argv++;
} else {
+ /* We were treating - as a commit and not a file */
+ if(substituted_minus)
+ argv[0] = -;
/* Otherwise we treat this as a filename */
verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
-
if (read_cache() 0)
die(_(index file corrupt));
--
2.3.1.168.g0c82976.dirty
From 21f0298c17768aaa11ff0a677cdefc8f54ac9515 Mon Sep 17 00:00:00 2001
From: Sudhanshu Shekhar sudshekha...@gmail.com
Date: Sun, 8 Mar 2015 02:13:57 +0530
Subject: [PATCH 2/2] Added test cases for reset -
Four test cases have been added
1) when user does reset - without any previous branch = Leads to error
2) when user does reset - with a previous branch = Ensure it
behaves like @{-1}
Other two deal with the situation when we have a file named '-'. We
ignore such a file and - is always treated either as a previous branch
or a bad filename. Users who wish to reset a file named '-' should specify
it as './-'
---
builtin/reset.c | 4 ++--
t/t7102-reset.sh | 62
2 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 9f8967d..02f33ef 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -206,7 +206,7 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
- if(!strcmp(argv[0], -)) {
+ if (!strcmp(argv[0], -)) {
argv[0] = @{-1};
substituted_minus = 1;
}
@@ -231,7 +231,7 @@ static void parse_args(struct pathspec *pathspec,
rev = *argv++;
} else {
/* We were treating - as a commit and not a file */
- if(substituted_minus)
+ if (substituted_minus)
argv[0] = -;
/* Otherwise we treat this as a filename */
verify_filename(prefix, argv[0], 1);
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 98bcfe2..4b8d7f5 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -568,4 +568,66 @@ test_expect_success 'reset --mixed sets up work tree' '
test_cmp expect actual
'
+cat expect EOF
+fatal: bad flag '-' used after filename
+EOF
+
+test_expect_success 'reset - with no previous branch' '
+ git init no_previous --quiet
+ (
+ cd no_previous
+ )
+ test_must_fail git reset - 2 output
+ test_cmp expect output
+'
+
+test_expect_success 'reset - while having file named - and no previous branch'
'
+ git init no_previous --quiet
+ (
+ cd no_previous
+ touch ./-
+ )
+ test_must_fail git reset - 2 output
+ test_cmp expect output
+'
+
+cat expect EOF
+Unstaged changes after reset:
+M -
+M 1
+EOF
+
+test_expect_success 'reset - in the prescence of file named - with previou
branch' '
+ git init no_previous --quiet
+ cd no_previous
+ touch ./- 1
+ git add 1 -
+ git commit -m add base files
+ git checkout -b new_branch
+ echo random ./-
+ echo wow 1
+ git add 1 -
+ git reset - output
+ test_cmp output ../expect
+'
+test_expect_success 'reset - works same as reset @{-1}' '
+ git init no_previous --quiet
+ cd no_previous
+ echo random random
+ git add random
+ git commit -m base commit
+ git checkout -b temp
+ echo new-file new-file
+ git add new-file
+ git commit -m added new-file
+ git reset -
+
+ git status ../first
[PATCH] reset: allow - short hand for previous commit
From: SudShekhar sudshekha...@gmail.com
Teach reset the same shorthand as checkout and merge. - means the
previous commit.
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
---
This is done as a microproject for gsoc purposes. I am looking forward to your
feedback/comments. Thanks
builtin/reset.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..3e0378b 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -205,6 +205,8 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if(!strcmp(argv[0],-))
+ argv[0]=@{-1};
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
--
2.3.1
--
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] reset: allow - short hand for previous commit
Teach reset the same shorthand as checkout and merge. - means the
previous commit.
Signed-off-by: Sudhanshu Shekhar sudshekha...@gmail.com
---
builtin/reset.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 4c08ddc..9f8967d 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 substituted_minus = 0;
/*
* Possible arguments are:
*
@@ -205,6 +206,10 @@ static void parse_args(struct pathspec *pathspec,
*/
if (argv[0]) {
+ if(!strcmp(argv[0], -)) {
+ argv[0] = @{-1};
+ substituted_minus = 1;
+ }
if (!strcmp(argv[0], --)) {
argv++; /* reset to HEAD, possibly with paths */
} else if (argv[1] !strcmp(argv[1], --)) {
@@ -225,12 +230,14 @@ static void parse_args(struct pathspec *pathspec,
verify_non_filename(prefix, argv[0]);
rev = *argv++;
} else {
+ /* We were treating - as a commit and not a file */
+ if(substituted_minus)
+ argv[0] = -;
/* Otherwise we treat this as a filename */
verify_filename(prefix, argv[0], 1);
}
}
*rev_ret = rev;
-
if (read_cache() 0)
die(_(index file corrupt));
--
2.3.1.168.gfd4bc34.dirty
--
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] reset: allow - short hand for previous commit
Hi,
Matthieu Moy matthieu@grenoble-inp.fr writes:
Sudhanshu Shekhar sudshekha...@gmail.com writes:
From: SudShekhar sudshekha...@gmail.com
Please, set your configuration to have the same identity for commit and
send-email. It seems your commiter ID (user.name) does not contain your
last name.
Actually, the token does not match either of the two names; it looks
like two names smashed together into a single nickname token.
Sorry, I had set my irc nick as the commiter id. I have changed this to my name.
builtin/reset.c | 2 ++
Doesn't this deserve a test?
+ if(!strcmp(argv[0],-))
+ argv[0]=@{-1};
Wrong spacing (around = and after ,).
Thanks for pointing this out. I have corrected it.
What should worry us even more is what the user would get when @{-1}
does not resolve to something the command can use. It would be bad
if we give an error message with @{-1} in it that the user never
typed (and may not even understand what it means).
I apologize for having overlooked this use case. I have made some
changes and I think this should work now. I will email the updated
patch in the next email (I hope this is the correct procedure). Please
let me know your thoughts.
Also, this use case hasn't been handled in the case of git merge.
Another thing, can someone guide me regarding the right place to add a
test case? Should it be t7102-reset.sh or some other file?
--
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
