Hey Matthieu,
On Sun, Feb 12, 2017 at 10:48:56AM +0100, Matthieu Moy wrote:
> Siddharth Kannan <kannan.siddharth12@xxxxxxxxx> writes:
>
> > sha1_name.c | 5 ++++
> > t/t4214-log-shorthand.sh | 73 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 78 insertions(+)
> > create mode 100755 t/t4214-log-shorthand.sh
> >
> > diff --git a/sha1_name.c b/sha1_name.c
> > index 73a915f..d774e46 100644
> > --- a/sha1_name.c
> > +++ b/sha1_name.c
> > @@ -947,6 +947,11 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned l
> > if (!ret)
> > return 0;
> >
> > + if (!strcmp(name, "-")) {
> > + name = "@{-1}";
> > + len = 5;
> > + }
>
> After you do that, the existing "turn - into @{-1}" pieces of code
> become useless and you should remove it (probably in a further patch).
Yeah, this is currently also implemented in checkout, apart from the
grepped list that you have supplied here. I will find all the
instances, and ensure that they work, and remove them. (This will
require some more digging into the codepath the commands, to ensure
that get_sha1_1 is called somewhere down the line)
>
> > diff --git a/t/t4214-log-shorthand.sh b/t/t4214-log-shorthand.sh
> > ...
> > +test_expect_success 'setup' '
> > + echo hello >world &&
> > + git add world &&
> > + git commit -m initial &&
> > + echo "hello second time" >>world &&
> > ...
>
> You may use test_commit to save a few lines of code.
Oh, yeah! I will use that. I need to work on improving the tests, as
well as adding the documentation.
>
> > +test_expect_success 'symmetric revision range should work when one end is left empty' '
> > + git checkout testing-2 &&
> > + git checkout master &&
> > + git log ...@{-1} > expect.first_empty &&
> > + git log @{-1}... > expect.last_empty &&
> > + git log ...- > actual.first_empty &&
> > + git log -... > actual.last_empty &&
>
> Nitpick: we stick the > and the filename (as you did in most places
> already).
Sorry, slipped my mind!
>
> It may be worth adding tests for more cases like
>
> * Check what happens with suffixes, i.e. -^, -@{yesterday} and -~.
These do not work right now. The first and last cases here are handled
by peel_onion, if I remember correctly. I have to find out why exactly
these are not working. Thanks for mentioning this!
>
> * -..- -> to make sure you handle the presence of two - properly.
>
> * multiple separate arguments to make sure you handle them all, e.g.
> "git log - -", "git log HEAD -", "git log - HEAD".
Yeah, will add these tests.
>
> The last two may be overkill, but the first one is probably important.
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
--
Regards,
Siddharth Kannan.