Re: [PATCH/RFC] WIP: log: allow "-" as a short-hand for "previous branch"

From: Siddharth Kannan <kannan.siddharth12@gmail.com>
Date: Feb 7, 2017, 19:14 UTC
Message-ID: <20170207191450.GA5569@ubuntu-512mb-blr1-01.localdomain>
In-Reply-To: <xmqqtw86zzk4.fsf@gitster.mtv.corp.google.com>

On Mon, Feb 06, 2017 at 03:09:47PM -0800, Junio C Hamano wrote:

> The focus of GSoC being mentoring those who are new to the open
> source development, and hopefully retain them in the community after
> GSoC is over, we do expect microprojects to be suitable for those
> who are new to the codebase.

Okay, understood! Since I have spent time here anyway, I guess I will continue on this instead of going over to a new micro project.

Show 17 quoted lines

> 
> > (c) -> Else look for "r1^-"
> > ...
> > Case (c) is a bit confusing. This could be something like "-^-", and
> > something like "^-" could mean "Not commits on previous branch" or it
> > could mean "All commits on this branch except for the parent of HEAD"
> 
> Do you mean:
> 
>     "git rev-parse ^-" does not mean "git rev-parse HEAD^-", but we
>     probably would want to, and if that is what is going to happen,
>     "^-" should mean "HEAD^-", and cannot be used for "^@{-1}"?
> 
> It's friend "^!" does not mean "HEAD^!", and "^@" does not mean
> "HEAD^@", either (the latter is somewhat borked, though, and "^@"
> translates to "^HEAD" because confusingly "@" stands for "HEAD"
> sometimes).

Yes, I meant that whether we should use ^- as ^@{-1} or HEAD^-.

Oh! So, that's why running `git log ^@` leads to an empty set!

> 
> So my gut feeling is that it is probably OK to make "^-" mean
> "^@{-1}"; it may be prudent to at least initially keep "^-" an error
> like it currently is already, though.

I agree with your gut feeling, and would like to _not_ exclude only this case. This way, across the code and implementation, there wouldn't be any particular cases which would have to be excluded.

Show 10 quoted lines

> > So, this patch reduces to the following 2 tasks:
> > 
> > 1. Teach setup_revisions that something starting with "-" can be
> > an
> > argument as well
> > 2. Teach get_sha1_basic that "-" means the tip of the previous
> > branch
> > perhaps by replacing it with "@{-1}" just before the reflog
> > parsing is
> > done

Making a change in sha1_name.c will touch a lot of commands (setup_revisions is called from everywhere in the codebase), so, I am still trying to figure out how to do this such that the rest of the codepath remains unchanged.

I hope that you do not mind this side-effect, but rather, you intended for this to happen, right? More commands will start supporting this shorthand, suddenly. (such as format-patch, whatchanged, diff to name a very few).

Best Regards,

Siddharth.