All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Thomas Rast <trast@student.ethz.ch>
Cc: git@vger.kernel.org,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Johannes Sixt <johannes.sixt@telecom.at>,
	Johan Herland <johan@herland.net>
Subject: Re: [PATCH/RFC v4 4/5] checkout: implement "-" abbreviation, add docs and tests
Date: Sat, 17 Jan 2009 11:57:44 -0800	[thread overview]
Message-ID: <7vwsct66yv.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <1232208597-29249-5-git-send-email-trast@student.ethz.ch> (Thomas Rast's message of "Sat, 17 Jan 2009 17:09:56 +0100")

Thomas Rast <trast@student.ethz.ch> writes:

> @@ -133,6 +133,10 @@ the conflicted merge in the specified paths.
>  +
>  When this parameter names a non-branch (but still a valid commit object),
>  your HEAD becomes 'detached'.
> ++
> +As a special case, the "`@\{-N\}`" syntax for the N-th last branch
> +checks out the branch (instead of detaching).  You may also specify
> +"`-`" which is synonymous with "`@\{-1\}`".

I mildly disagree with this wording.

The new syntax is supposed to be a new way to name a branch, not a random
non-branch committish that is special cased by "git checkout".  I would
further suggest that we should teach "git rev-parse --symbolic-full-name"
and "git rev-parse --symbolic" about the new syntax, so that scripts can
use the syntax to find out the same information.

The "-" thing deserves a mention here in the documentation.  That _is_ a
special case that only applies to the "git checkout" command.

> diff --git a/builtin-checkout.c b/builtin-checkout.c
> index dc1de06..b0a101b 100644
> --- a/builtin-checkout.c
> +++ b/builtin-checkout.c
> @@ -679,6 +679,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
>  		arg = argv[0];
>  		has_dash_dash = (argc > 1) && !strcmp(argv[1], "--");
>  
> +		if (!strcmp(arg, "-"))
> +			arg = "@{-1}";
> +

This is not quite nice as it could be, but it probably is Ok.  If the
interpretation of @{-1} errors out, the user won't see an error message
that talks about "-" but instead the user will see "@{-1}".

Also it will look somewhat inconsistent to the end user who does not know
the internals for "-" claim to be a synonym for @{-1} but it really isn't.
For example, "git checkout -^0" does not work as "git checkout @{-1}^0".

To avoid such confusion, we could instead make "git checkout - <ENTER>" a
synonym for "git checkout @{-1} <ENTER>", without claiming to make "-" a
synonym for "@{-1}".  In other words, "git checkout -" can become a very
narrow, focused special case that does not allow anything else, such as
pathspecs, "--" separator, nor --force and other options.

  parent reply	other threads:[~2009-01-17 19:59 UTC|newest]

Thread overview: 102+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-15  0:06 [PATCH] checkout: implement "-" shortcut name for last branch Thomas Rast
2009-01-15  0:12 ` [PATCH v2] " Thomas Rast
2009-01-15  7:27   ` Johannes Sixt
2009-01-15 13:15     ` Johannes Schindelin
2009-01-15 13:59       ` Thomas Rast
2009-01-15 14:09         ` Johannes Schindelin
2009-01-15 14:17           ` Johannes Schindelin
2009-01-15 20:12             ` Junio C Hamano
2009-01-15 20:35               ` Johannes Schindelin
2009-01-16 12:52               ` [PATCH] revision walker: include a detached HEAD in --all Johannes Schindelin
2009-01-16 13:12                 ` Santi Béjar
2009-01-16 13:17                   ` Johannes Schindelin
2009-01-16 13:22                     ` David Kastrup
2009-01-16 13:46                     ` Santi Béjar
2009-01-16 13:50                       ` Santi Béjar
2009-01-18  6:01                 ` Junio C Hamano
2009-01-18  6:36                   ` Junio C Hamano
2009-01-18 14:06                     ` Johannes Schindelin
2009-01-18 13:42                   ` Johannes Schindelin
2009-01-15 16:32       ` [PATCH v2] checkout: implement "-" shortcut name for last branch Johan Herland
2009-01-15 16:50         ` Johannes Schindelin
2009-01-15 20:11   ` Junio C Hamano
2009-01-15 20:50     ` Junio C Hamano
2009-01-17  3:30       ` [PATCH/RFC v3 0/6] N-th last checked out branch Thomas Rast
2009-01-17  5:52         ` Johannes Schindelin
2009-01-17 13:38           ` Thomas Rast
2009-01-17 13:40             ` [PATCH/RFC v3bis 1/2] sha1_name: implement @{-N} syntax for N-th last checked out Thomas Rast
2009-01-17 13:40               ` [PATCH/RFC v3bis 2/2] checkout: implement '@{-N}' and '-' special abbreviations Thomas Rast
2009-01-17 15:04             ` [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice Johannes Schindelin
2009-01-17 16:09               ` [PATCH/RFC v4 0/5] N-th last checked out branch Thomas Rast
2009-01-17 16:09                 ` [PATCH/RFC v4 1/5] checkout: implement "@{-N}" shortcut name for N-th last branch Thomas Rast
2009-01-17 16:09                   ` [PATCH/RFC v4 2/5] sha1_name: tweak @{-N} lookup Thomas Rast
2009-01-17 16:09                     ` [PATCH/RFC v4 3/5] sha1_name: support @{-N} syntax in get_sha1() Thomas Rast
2009-01-17 16:09                       ` [PATCH/RFC v4 4/5] checkout: implement "-" abbreviation, add docs and tests Thomas Rast
2009-01-17 16:09                         ` [PATCH/RFC v4 5/5] interpret_nth_last_branch(): avoid traversing the reflogs twice Thomas Rast
2009-01-17 18:08                           ` [PATCH 6/5] Fix parsing of @{-1}@{1} Johannes Schindelin
2009-01-17 20:02                             ` Junio C Hamano
2009-01-17 21:22                               ` Johannes Schindelin
2009-01-17 19:57                         ` Junio C Hamano [this message]
2009-01-17 17:55                       ` [PATCH/RFC v4 3/5] sha1_name: support @{-N} syntax in get_sha1() Johannes Schindelin
2009-01-17 19:37                       ` Junio C Hamano
2009-01-18  0:54                     ` [PATCH/RFC v4 2/5] sha1_name: tweak @{-N} lookup Junio C Hamano
2009-01-17 16:49                 ` [PATCH/RFC v4 0/5] N-th last checked out branch Johannes Schindelin
2009-01-17 19:13               ` [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice Junio C Hamano
2009-01-17 19:29                 ` Johannes Schindelin
2009-01-18  0:43                   ` Junio C Hamano
2009-01-18  1:12                     ` Johannes Schindelin
2009-01-18  7:25                       ` Junio C Hamano
2009-01-18 20:59                         ` Johannes Schindelin
2009-01-19  8:08                           ` Junio C Hamano
2009-01-19  8:19                             ` Junio C Hamano
2009-01-19 12:33                               ` Johannes Schindelin
2009-01-20  0:11                                 ` Thomas Rast
2009-01-20  0:23                                   ` Johannes Schindelin
2009-01-20  0:41                                     ` Thomas Rast
2009-01-20  6:21                                 ` [PATCH] interpret_nth_last_branch(): plug small memleak Junio C Hamano
2009-01-20 10:15                                   ` Johannes Schindelin
2009-01-20  6:22                                 ` [PATCH] Introduce for_each_recent_reflog_ent() Junio C Hamano
2009-01-20 10:15                                   ` Johannes Schindelin
2009-01-20  8:35                                 ` [PATCH] interpret_nth_last_branch(): avoid traversing the reflogs twice Junio C Hamano
2009-01-21  0:16                                 ` [VALGRIND PATCH for nth_last patch series] Fix invalid memory access Johannes Schindelin
2009-01-21  8:45                                   ` Junio C Hamano
2009-01-21  9:18                                     ` Thomas Rast
2009-01-21 10:13                                       ` Junio C Hamano
2009-01-21 12:06                                         ` Johannes Schindelin
2009-01-24 22:21                                       ` Thomas Rast
2009-01-24 22:23                                         ` [PATCH next] t1505: remove debugging cruft Thomas Rast
2009-01-25 20:35                                           ` Junio C Hamano
2009-01-21 11:56                                     ` [VALGRIND PATCH for nth_last patch series] Fix invalid memory access Johannes Schindelin
2009-01-19 12:41                               ` [PATCH] @{-<n>}: avoid crash with corrupt reflog Johannes Schindelin
2009-01-19 14:57                                 ` Johannes Schindelin
2009-01-17  3:30       ` [PATCH/RFC v3 1/6] reflog: refactor parsing and checking Thomas Rast
2009-01-17  5:35         ` Johannes Schindelin
2009-01-17  3:30       ` [PATCH/RFC v3 2/6] reflog: refactor log open+mmap Thomas Rast
2009-01-17  5:40         ` Johannes Schindelin
2009-01-17  3:30       ` [PATCH/RFC v3 3/6] reflog: make for_each_reflog_ent use mmap Thomas Rast
2009-01-17  3:30       ` [PATCH/RFC v3 4/6] reflog: add backwards iterator Thomas Rast
2009-01-17  3:30       ` [PATCH/RFC v3 5/6] sha1_name: implement @{-N} syntax for N-th last checked out Thomas Rast
2009-01-17  3:30       ` [PATCH/RFC v3 6/6] checkout: implement '@{-N}' and '-' special abbreviations Thomas Rast
2009-01-16 12:31     ` [PATCH v2] checkout: implement "-" shortcut name for last branch Johannes Schindelin
2009-01-15  0:45 ` [PATCH] " Johannes Schindelin
2009-01-15 14:01   ` Thomas Rast
2009-01-15 14:14     ` Johannes Schindelin
2009-01-15 17:05       ` Thomas Rast
2009-01-15 18:34         ` Johannes Schindelin
2009-01-16  9:08       ` Thomas Rast
2009-01-16 11:18         ` Johannes Schindelin
2009-01-18  1:38           ` [TOY PATCH] git-resurrect: find traces of a branch name and resurrect it Thomas Rast
2009-01-18 16:19             ` Johannes Schindelin
2009-01-20  9:01               ` Thomas Rast
2009-01-20 16:57                 ` Boyd Stephen Smith Jr.
2009-01-20 20:50                   ` Boyd Stephen Smith Jr.
2009-01-23 20:03                     ` [PATCH] contrib " Thomas Rast
2009-01-23 21:00                       ` Boyd Stephen Smith Jr.
2009-01-26 11:54                         ` Thomas Rast
2009-01-26 12:40                           ` [PATCH v2] " Thomas Rast
2009-01-27  6:31                             ` Junio C Hamano
2009-01-30 22:52                               ` Thomas Rast
2009-02-01 21:34                               ` [PATCH v3] " Thomas Rast
2009-02-02  2:31                                 ` Junio C Hamano
2009-02-04 10:04                                   ` [PATCH v4] " Thomas Rast
2009-02-05  8:38                                     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vwsct66yv.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=johan@herland.net \
    --cc=johannes.schindelin@gmx.de \
    --cc=johannes.sixt@telecom.at \
    --cc=trast@student.ethz.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.