git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Junio C Hamano <gitster@pobox.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Make 'git show' more useful
Date: Mon, 13 Jul 2009 14:41:12 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.2.01.0907131425490.13838@localhost.localdomain> (raw)


For some reason, I ended up doing

	git show HEAD~5..

as an odd way of asking for a log. I realize I should just have used "git 
log", but at the same time it does make perfect conceptual sense. After 
all, you _could_ have done

	git show HEAD HEAD~1 HEAD~2 HEAD~3 HEAD~4

and saying "git show HEAD~5.." is pretty natural. It's not like "git show" 
only ever showed a single commit (or other object) before either! So 
conceptually, giving a commit range is a very sensible operation, even 
though you'd traditionally have used "git log" for that.

However, doing that currently results in an error

	fatal: object ranges do not make sense when not walking revisions

which admittedly _also_ makes perfect sense - from an internal git 
implementation standpoint in 'revision.c'.

However, I think that asking to show a range makes sense to a user, while 
saying "object ranges no not make sense when not walking revisions" only 
makes sense to a git developer.

So on the whole, of the two different "makes perfect sense" behaviors, I 
think I originally picked the wrong one. And quite frankly, I don't really 
see anybody actually _depending_ on that error case. So why not change it?

So rather than error out, just turn that non-walking error case into a 
"silently turn on walking" instead.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

This is a total throw-away patch. I'm not going to re-send. Take it or 
not, I don't care, I spent more time writing this discussion than I did on 
the patch. I just happened to notice this behavior, and decided I probably 
would prefer this behavior. But it doesn't really matter in any kind of 
big picture.

And, admittedly, you can _already_ do this by just adding "--do-walk" 
whenever you specify a range. And equally admittedly, you can already 
confuse git by adding the "--no-walk" _after_ specifying the range, ie you 
can do this:

	git log HEAD~5.. --no-walk

and it will actually turn into a really odd way of saying "git show HEAD" 
(notice: _not_ "HEAD~5". HEAD). And even with this change you can do the 
reverse:

	git show HEAD~5.. --no-walk

it it will first implicitly turn off walking ("git show"), then it will 
implicitly turn it on again (the commit range triggers the code in this 
patch), and then _after_ it has seen the commit range it will explicitly 
turn off walking again.

In other words, regardless of this patch you can do crazy things. I really 
don't think anybody cares. This patch is not meant to disable crazy 
things, it's meant to enable a reasonably sane user behavior.

 revision.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/revision.c b/revision.c
index a31434b..9f5dac5 100644
--- a/revision.c
+++ b/revision.c
@@ -133,7 +133,7 @@ void mark_parents_uninteresting(struct commit *commit)
 static void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, const char *name, unsigned mode)
 {
 	if (revs->no_walk && (obj->flags & UNINTERESTING))
-		die("object ranges do not make sense when not walking revisions");
+		revs->no_walk = 0;
 	if (revs->reflog_info && obj->type == OBJ_COMMIT &&
 			add_reflog_for_walk(revs->reflog_info,
 				(struct commit *)obj, name))

             reply	other threads:[~2009-07-13 21:41 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-13 21:41 Linus Torvalds [this message]
2009-07-13 22:11 ` Make 'git show' more useful Junio C Hamano
2009-07-13 23:43   ` [PATCH] " Paolo Bonzini
2009-07-14  0:00     ` Linus Torvalds
2009-07-14  1:25       ` Johannes Schindelin
2009-07-14  1:47         ` Linus Torvalds
2009-07-14  6:25         ` Paolo Bonzini
2009-07-14 10:44           ` Johannes Schindelin
2009-07-14 12:08             ` [PATCH] t4202-log.sh: Test git log --no-walk sort order Michael J Gruber
2009-07-14 12:21               ` Johannes Sixt
2009-07-14 12:38                 ` Michael J Gruber
2009-07-14 12:45                 ` [PATCHv2] " Michael J Gruber
2009-07-14 14:13                   ` Johannes Schindelin
2009-07-14 14:28                     ` Michael J Gruber
2009-07-17 14:28                     ` [PATCH] " Michael J Gruber
2009-07-17 14:53                       ` Johannes Schindelin
2009-07-13 23:42 ` Make 'git show' more useful Johannes Schindelin

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=alpine.LFD.2.01.0907131425490.13838@localhost.localdomain \
    --to=torvalds@linux-foundation.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).