[PATCH/RFC 0/4] Adding '-' notation as @{-1} (pu, d40f108)
This is an attempt to allow '-' everywhere a revision is normally allowed.
I previously attempted this as a microproject and the subject was disscussed
at : http://article.gmane.org/gmane.comp.version-control.git/265672
Currently, something like '-~2' does not work. I tried tracing the execution
of, say 'log -~2' vs 'log master -~2' and noticed when calling dwim_ref() with
'-~2', it returns 0 (no refs found) whereas when given 'master~2', it returned
non-zero. However I'm not sure how exactly dwim_ref() works.
Kenny Lee Sin Cheong (4):
Add - as @{-1} support for the rev-parse command
t1505: add tests for '-' notation in rev-parse
Handle arg as revision first, then option.
t0102: add tests for '-' notation
builtin/rev-parse.c | 37 +-
revision.c| 61 +++
sha1_name.c | 2 +-
t/t0102-previous-shorthand.sh | 40
t/t1505-rev-parse-last.sh | 12 ++---
5 files changed, 101 insertions(+), 51 deletions(-)
create mode 100644 t/t0102-previous-shorthand.sh
--
2.3.3.203.g8ffb468.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/RFC 4/4] t0102: add tests for '-' notation
Signed-off-by: Kenny Lee Sin Cheong kenny.le...@gmail.com
---
t/t0102-previous-shorthand.sh | 40
1 file changed, 40 insertions(+)
create mode 100644 t/t0102-previous-shorthand.sh
diff --git a/t/t0102-previous-shorthand.sh b/t/t0102-previous-shorthand.sh
new file mode 100644
index 000..919b055
--- /dev/null
+++ b/t/t0102-previous-shorthand.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+test_description='previous branch syntax @{-n}'
+
+. ./test-lib.sh
+
+test_expect_success 'branch -d -' '
+ test_commit A
+ git checkout -b junk2
+ git checkout -
+ test $(git symbolic-ref HEAD) = refs/heads/master
+ git branch -d -
+ test_must_fail git rev-parse --verify refs/heads/junk2
+'
+
+test_expect_success 'merge -' '
+ git checkout A
+ test_commit B
+ git checkout A
+ test_commit C
+ test_commit D
+ git branch -f master B
+ git branch -f other
+ git checkout other
+ git checkout master
+ git merge -
+ git cat-file commit HEAD | grep Merge branch '\''other'\''
+'
+
+test_expect_success 'merge -~1' '
+ git checkout master
+ git reset --hard B
+ git checkout other
+ git checkout master
+ git merge -~1
+ git cat-file commit HEAD actual
+ grep Merge branch '\''other'\'' actual
+'
+
+test_done
--
2.3.3.203.g8ffb468.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/RFC 1/4] Add - as @{-1} support for the rev-parse command
Allows the use of the - shorthand notation, including
use with revision ranges. If we plan to allow - as a stand in every
where a revision is allowed, then - would also need to be usable in
plumbing commands, for writing tests, for example.
Checks if the argument can be interpreted as a revision range first
before checking for flags. This saves us from having to check that
something that begins with - does not get checked as a possible flag.
Signed-off-by: Kenny Lee Sin Cheong kenny.le...@gmail.com
---
builtin/rev-parse.c | 37 +++--
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 3626c61..8da95b5 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -553,6 +553,25 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
continue;
}
+ /* Not a flag argument */
+ if (try_difference(arg))
+ continue;
+ if (try_parent_shorthands(arg))
+ continue;
+ name = arg;
+ type = NORMAL;
+ if (*arg == '^') {
+ name++;
+ type = REVERSED;
+ }
+ if (!get_sha1_with_context(name, flags, sha1, unused)) {
+ if (verify)
+ revs_count++;
+ else
+ show_rev(type, sha1, name);
+ continue;
+ }
+
if (*arg == '-') {
if (!strcmp(arg, --)) {
as_is = 2;
@@ -810,24 +829,6 @@ int cmd_rev_parse(int argc, const char **argv, const char
*prefix)
continue;
}
- /* Not a flag argument */
- if (try_difference(arg))
- continue;
- if (try_parent_shorthands(arg))
- continue;
- name = arg;
- type = NORMAL;
- if (*arg == '^') {
- name++;
- type = REVERSED;
- }
- if (!get_sha1_with_context(name, flags, sha1, unused)) {
- if (verify)
- revs_count++;
- else
- show_rev(type, sha1, name);
- continue;
- }
if (verify)
die_no_single_rev(quiet);
if (has_dashdash)
--
2.3.3.203.g8ffb468.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/RFC 3/4] Handle arg as revision first, then option.
Check the argument as a revision at first. If it fails, then tries to
check it as an option, and finally as a pathspec.
Returns -1 when we have an ambiguous revision range, such as
master..next, to allow the argument to get checked as an option before
calling die() from verify_non_filename(). This is because we are
allowing - to be given in a revision range, but making the revision
check first. Otherwise, an ambiguous argument that starts with
- (let's say an option) would die even though its normal behaviour is
to silently return. Instead we check for ambiguity in a revision after
making sure that the argument cannot be parsed as an option.
This problem is discussed in:
http://article.gmane.org/gmane.comp.version-control.git/265672
Signed-off-by: Kenny Lee Sin Cheong kenny.le...@gmail.com
---
revision.c | 61 +
sha1_name.c | 2 +-
2 files changed, 34 insertions(+), 29 deletions(-)
diff --git a/revision.c b/revision.c
index 570945a..1ea290f 100644
--- a/revision.c
+++ b/revision.c
@@ -1516,7 +1516,10 @@ int handle_revision_arg(const char *arg_, struct
rev_info *revs, int flags, unsi
if (!cant_be_filename) {
*dotdot = '.';
- verify_non_filename(revs-prefix, arg);
+ if (is_inside_work_tree()
!is_inside_git_dir()
+ check_filename(revs-prefix, arg)) {
+ return -1;
+ }
}
a_obj = parse_object(from_sha1);
@@ -2198,40 +2201,39 @@ int setup_revisions(int argc, const char **argv, struct
rev_info *revs, struct s
read_from_stdin = 0;
for (left = i = 1; i argc; i++) {
const char *arg = argv[i];
- if (arg[0] == '-' arg[1] !starts_with(arg + 1, ..)) {
- int opts;
-
- opts = handle_revision_pseudo_opt(submodule,
- revs, argc - i, argv + i,
- flags);
- if (opts 0) {
- i += opts - 1;
- continue;
- }
+ if (handle_revision_arg(arg, revs, flags, revarg_opt)) {
+ if (arg[0] == '-' arg[1] !starts_with(arg + 1,
..)) {
+ int opts;
+
+ opts = handle_revision_pseudo_opt(submodule,
+ revs, argc -
i, argv + i,
+ flags);
+ if (opts 0) {
+ i += opts - 1;
+ continue;
+ }
- if (!strcmp(arg, --stdin)) {
- if (revs-disable_stdin) {
- argv[left++] = arg;
+ if (!strcmp(arg, --stdin)) {
+ if (revs-disable_stdin) {
+ argv[left++] = arg;
+ continue;
+ }
+ if (read_from_stdin++)
+ die(--stdin given twice?);
+ read_revisions_from_stdin(revs,
prune_data);
continue;
}
- if (read_from_stdin++)
- die(--stdin given twice?);
- read_revisions_from_stdin(revs, prune_data);
- continue;
- }
- opts = handle_revision_opt(revs, argc - i, argv + i,
left, argv);
- if (opts 0) {
- i += opts - 1;
+ opts = handle_revision_opt(revs, argc - i, argv
+ i, left, argv);
+ if (opts 0) {
+ i += opts - 1;
+ continue;
+ }
+ if (opts 0)
+ exit(128);
continue;
}
- if (opts 0)
- exit(128);
- continue;
- }
-
- if (handle_revision_arg(arg, revs, flags, revarg_opt)) {
int j;
if (seen_dashdash || *arg == '^')
die(bad revision '%s', arg
[PATCH/RFC 2/4] t1505: add tests for '-' notation in rev-parse
Signed-off-by: Kenny Lee Sin Cheong kenny.le...@gmail.com
---
t/t1505-rev-parse-last.sh | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh
index 4969edb..a1976ad 100755
--- a/t/t1505-rev-parse-last.sh
+++ b/t/t1505-rev-parse-last.sh
@@ -33,19 +33,23 @@ test_expect_success 'setup' '
# and 'side' should be the last branch
test_expect_success '@{-1} works' '
- test_cmp_rev side @{-1}
+ test_cmp_rev side @{-1}
+ test_cmp_rev side -
'
test_expect_success '@{-1}~2 works' '
- test_cmp_rev side~2 @{-1}~2
+ test_cmp_rev side~2 @{-1}~2
+ test_cmp_rev side~2 -~2
'
test_expect_success '@{-1}^2 works' '
- test_cmp_rev side^2 @{-1}^2
+ test_cmp_rev side^2 @{-1}^2
+ test_cmp_rev side^2 -^2
'
test_expect_success '@{-1}@{1} works' '
- test_cmp_rev side@{1} @{-1}@{1}
+ test_cmp_rev side@{1} @{-1}@{1}
+ test_cmp_rev side@{1} -@{1}
'
test_expect_success '@{-2} works' '
--
2.3.3.203.g8ffb468.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 2/2] Add revision range support on - and @{-1}
On Tue, Mar 17 2015 at 06:16:38 PM, Junio C Hamano gits...@pobox.com wrote:
I also notice that handle_revision_arg() would die() by calling it
directly or indirectly via verify_non_filename(), etc., but the
caller actually is expecting it to silently return non-zero when it
finds an argument that cannot be interpreted as a revision or as a
revision range.
If we feed the function a string that has .. in it, with
cant_be_filename unset, and if that string _can_ be parsed as a
valid range (e.g. master..next), we would check if a file whose
name is that string and die, e.g.
$ master..next ; git log master..next
fatal: ambigous argument 'master..next': both revision and filename
If we swap the order to do the revision first before option,
however, we would end up getting the same for a name that begins
with - and has .. in it. I see no guarantee that future
possible option name cannot be misinterpreted as a range to trigger
this check.
If I'm understanding correctly, the problem of checking revisions before
arg is that an option fed to handle_revision_arg() might die() before getting
checked as an option in cases where a file with the same name exists?
But doesn't verify_non_filename() already return silently if arg begins
with -? It die() only after making that check.
If an option with .. in it such as -$opt..ion is really given to
handle_revision_arg() then verify_non_filename should not be a problem.
But git cmd -$option for any value of $option does not have to be
disambiguated when there is a file whose name is -$option. The
existing die()'s in the handle_revision_arg() function _will_ break
that promise. Currently, because we check the options first,
handle_revision_arg() does not cause us any problem, but swapping
the order will have fallouts.
The only other way handle_revision_arg() can die() is if given a ..
range, either revisions return null when passed their sha1 to
parse_object().
So something like you proposed earlier:
if(try to see if it is a revision or a revision range) {
/* if failed ... */
if (starts with '-') {
do the option thing;
continue;
}
/*
* args must be pathspecs from here on.
* We already checked that rev arg cannot be
* interpreted as a filename at this point
*/
if(dashdash)
verify_filename()
} else {
got_rev_arg = 1;
}
should work. I'm still getting familiar to how it works so I might be missing
something but shouldn't this be fine? At least concerning the possible fallouts
that you've raised.
If we want to really do the swapping (and I think that is the only
sensible way if we wanted to allow - and any extended SHA-1 that
begins with - as the previous branch), I think the OK, it looks
like a revision (or revision range); as we didn't see dashdash, it
must not be a filename check has to be moved to the caller, perhaps
like this:
if (try to see if it is a revision or a revision range) {
/* failed */
...
} else {
/* it can be read as a revision or a revision range */
if (!seen_dashdash)
verify_non_filename(arg);
got_rev_arg = 1;
}
If what I'm saying makes sense, then verify_non_filename(arg) would be
already working as intended in handle_revision_arg(), so moving it to
the caller wouldn't be necessary.
The missing cases should also silently return failure and have the
caller deal with that.
--
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: [v3 PATCH 1/2] reset: add '-' shorthand for '@{-1}'
On Wed, Mar 18 2015 at 04:29:44 AM, Sundararajan R dyou...@gmail.com wrote:
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.
I might be wrong but I think any pathspec that begins with - needs to
be preceded by either a -- marker or be specified as ./-filename,
else verify_filename just die. Therefore you would need to do something
like git reset ./- if you wanted to reset a file. I don't know if given
simply - as filename is desired since options starts with -.
I don't know if you saw but Junio posted a while ago about about
allowing - as a stand-in everywhere a revision was allowed. He updated
a version on pu : d40f108d
On Tue, Mar 17 2015 at 02:49:48 AM, Junio C Hamano gits...@pobox.com wrote:
Junio C Hamano gits...@pobox.com writes:
if (try to see if it is a revision or regvision range) {
/* if failed ... */
if (starts with '-') {
do the option thing;
continue;
}
/* args must be pathspecs from here on */
check the '--' disambiguation;
add pathspec to prune-data;
} else {
got_rev_arg = 1;
}
See $gmane/265672
--
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 2/2] Add revision range support on - and @{-1}
On Tue, Mar 17 2015 at 02:49:48 AM, Junio C Hamano gits...@pobox.com wrote:
Junio C Hamano gits...@pobox.com writes:
if (try to see if it is a revision or regvision range) {
/* if failed ... */
if (starts with '-') {
do the option thing;
continue;
}
/* args must be pathspecs from here on */
check the '--' disambiguation;
add pathspec to prune-data;
} else {
got_rev_arg = 1;
}
but I didn't trace the logic myself to see if that would work.
You're right. I was actually going to try and check all possible
suffixes of - but your solution saves us from doing that, and it
didn't break any tests.
On a similar note, would it be relevant to add similar changes to
rev-parse? While trying to write some test, I noticed that rev-parse
doesn't support -. If I'm not mistaking it assumes everything that starts
with -
must be an option. But since it is a plumbing tool I don't know if it
would be worth it or even an improvement.
--
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/RFC 0/2][GSoC] revision.c: Allow - as stand-in for @{-1} everywhere a branch is allowed
This is an attempt at a microproject for GSoC
An attempt to add revision range support to Junio's JFF patch sent a few days
ago. The first patch is the a copy of the one he posted.
I was wondering if it was a good idea to add support for commands like
rev..-. Files that starts with - requires -- or a ./ format but what
if we have a file named next..- and call git log next..- ?
Junio C Hamano (1):
- and @{-1} on various programs
Kenny Lee Sin Cheong (1):
Add revision range support on - and @{-1}
builtin/checkout.c | 3 ---
builtin/merge.c| 3 +--
builtin/revert.c | 2 --
revision.c | 16 +--
sha1_name.c| 57 +-
5 files changed, 50 insertions(+), 31 deletions(-)
--
2.3.2.225.gebdc58a
--
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] - and @{-1} on various programs
From: Junio C Hamano gits...@pobox.com
JFF stands for just for fun.
This is not meant to give out a model answer and is known to be
incomplete, but I was wondering if it would be a better direction to
allow - as a stand-in for @{-1} everywhere we allow a branch
name, losing workarounds at the surface level we have for checkout,
merge and revert.
The first three paths are to remove the surface workarounds that
become unnecessary. The one in sha1_name.c is the central change.
The change in revision.c is to allow a single - to be recognized
as a potential revision name (without this change, what begins with
- is either an option or an unknown option).
So you could do things like git reset - $path but also things like
git log - after switching out of a branch.
What does not work are what needs further tweaking in revision.c
parser. git checkout master git checkout next git log -..
should show what next has on top of master but I didn't touch the
range notation so it does not work, for example.
builtin/checkout.c | 3 ---
builtin/merge.c| 3 +--
builtin/revert.c | 2 --
revision.c | 2 +-
sha1_name.c| 57 +-
5 files changed, 37 insertions(+), 30 deletions(-)
Signed-off-by: Kenny Lee Sin Cheong kenny.le...@gmail.com
---
builtin/checkout.c | 3 ---
builtin/merge.c| 3 +--
builtin/revert.c | 2 --
revision.c | 2 +-
sha1_name.c| 57 +-
5 files changed, 37 insertions(+), 30 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 3e141fc..f86bad7 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -951,9 +951,6 @@ static int parse_branchname_arg(int argc, const char **argv,
else if (dash_dash_pos = 2)
die(_(only one reference expected, %d given.), dash_dash_pos);
- if (!strcmp(arg, -))
- arg = @{-1};
-
if (get_sha1_mb(arg, rev)) {
/*
* Either case (3) or (4), with something not being
diff --git a/builtin/merge.c b/builtin/merge.c
index 3b0f8f9..03b260f 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1164,8 +1164,7 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
argc = setup_with_upstream(argv);
else
die(_(No commit specified and
merge.defaultToUpstream not set.));
- } else if (argc == 1 !strcmp(argv[0], -))
- argv[0] = @{-1};
+ }
}
if (!argc)
usage_with_options(builtin_merge_usage,
diff --git a/builtin/revert.c b/builtin/revert.c
index 56a2c36..dc98b4e 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -170,8 +170,6 @@ static void parse_args(int argc, const char **argv, struct
replay_opts *opts)
opts-revs-no_walk = REVISION_WALK_NO_WALK_UNSORTED;
if (argc 2)
usage_with_options(usage_str, options);
- if (!strcmp(argv[1], -))
- argv[1] = @{-1};
memset(s_r_opt, 0, sizeof(s_r_opt));
s_r_opt.assume_dashdash = 1;
argc = setup_revisions(argc, argv, opts-revs, s_r_opt);
diff --git a/revision.c b/revision.c
index 66520c6..7778bbd 100644
--- a/revision.c
+++ b/revision.c
@@ -2198,7 +2198,7 @@ int setup_revisions(int argc, const char **argv, struct
rev_info *revs, struct s
read_from_stdin = 0;
for (left = i = 1; i argc; i++) {
const char *arg = argv[i];
- if (*arg == '-') {
+ if (arg[0] == '-' arg[1]) {
int opts;
opts = handle_revision_pseudo_opt(submodule,
diff --git a/sha1_name.c b/sha1_name.c
index 95f9f8f..7a621ba 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -483,6 +483,8 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1,
break;
}
}
+ } else if (len == 1 str[0] == '-') {
+ nth_prior = 1;
}
/* Accept only unambiguous ref paths. */
@@ -491,13 +493,16 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1,
if (nth_prior) {
struct strbuf buf = STRBUF_INIT;
- int detached;
+ int status;
if (interpret_nth_prior_checkout(str, len, buf) 0) {
- detached = (buf.len == 40 !get_sha1_hex(buf.buf,
sha1));
+ if (get_sha1(buf.buf, sha1))
+ /* bad---the previous branch no longer exists?
*/
+ status = -1;
+ else
+ status = 0; /* detached */
strbuf_release(buf);
- if (detached
[PATCH 2/2] Add revision range support on - and @{-1}
Currently it is not be possible to do something like git checkout
master git checkout next git log -.. to see what master has on
top of master.
Allows use of the revision range such as rev..- or -..rev to see
what HEAD has on top of rev or vice versa, respectively.
Also allows use of symmetric differences such as rev...- and -...rev
This is written on top of Junio's Just For Fun patch ($Gmane/265260).
Signed-off-by: Kenny Lee Sin Cheong kenny.le...@gmail.com
---
revision.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/revision.c b/revision.c
index 7778bbd..a79b443 100644
--- a/revision.c
+++ b/revision.c
@@ -1490,6 +1490,7 @@ int handle_revision_arg(const char *arg_, struct rev_info
*revs, int flags, unsi
int symmetric = *next == '.';
unsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM);
static const char head_by_default[] = HEAD;
+ static const char prev_rev[] = @{-1};
unsigned int a_flags;
*dotdot = 0;
@@ -1499,6 +1500,13 @@ int handle_revision_arg(const char *arg_, struct
rev_info *revs, int flags, unsi
next = head_by_default;
if (dotdot == arg)
this = head_by_default;
+ /* Allows -..rev and rev..- */
+ if (!strcmp(this, -)) {
+ this = prev_rev;
+ }
+ if (!strcmp(next, -)) {
+ next = prev_rev;
+ }
if (this == head_by_default next == head_by_default
!symmetric) {
/*
@@ -2198,7 +2206,7 @@ int setup_revisions(int argc, const char **argv, struct
rev_info *revs, struct s
read_from_stdin = 0;
for (left = i = 1; i argc; i++) {
const char *arg = argv[i];
- if (arg[0] == '-' arg[1]) {
+ if (arg[0] == '-' !strstr(arg, ..)) {
int opts;
opts = handle_revision_pseudo_opt(submodule,
@@ -2220,6 +2228,7 @@ int setup_revisions(int argc, const char **argv, struct
rev_info *revs, struct s
continue;
}
+
opts = handle_revision_opt(revs, argc - i, argv + i,
left, argv);
if (opts 0) {
i += opts - 1;
@@ -2229,7 +2238,10 @@ int setup_revisions(int argc, const char **argv, struct
rev_info *revs, struct s
exit(128);
continue;
}
-
+ if (strstr(arg, ..)) {
+ handle_revision_arg(arg, revs, flags, revarg_opt);
+ continue;
+ }
if (handle_revision_arg(arg, revs, flags, revarg_opt)) {
int j;
--
2.3.2.225.gebdc58a
--
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
