* Re: What's cooking in git.git (Aug 2009, #02; Wed, 12)
From: Junio C Hamano @ 2009-08-15 1:51 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <m3k516nic0.fsf@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
>> * jn/gitweb-blame (Thu Aug 6 19:11:52 2009 +0200) 14 commits
>> - gitweb: Create links leading to 'blame_incremental' using
>> JavaScript
>> - gitweb: Incremental blame (proof of concept)
>> - gitweb: Add optional "time to generate page" info in footer
>> + Revert the four topmost commits from jn/gitweb-blame topic
>> + gitweb: Create links leading to 'blame_incremental' using
>> JavaScript
>> + gitweb: Incremental blame (proof of concept)
>> + gitweb: Add optional "time to generate page" info in footer
>> + gitweb: Add -partial_query option to href() subroutine
>> + gitweb: Use light/dark for class names also in 'blame' view
>> + gitweb: Add author initials in 'blame' view, a la "git gui blame"
>> + gitweb: Mark commits with no "previous" in 'blame' view
>> + gitweb: Use "previous" header of git-blame -p in 'blame' view
>> + gitweb: Mark boundary commits in 'blame' view
>> + gitweb: Make .error style generic
>>
>> I haven't picked up Jakub's replacement to the second one from the tip.
>> We probably should merge up to "Use light/dark" (aef3768) to 'master' and
>> start the rest over.
>
> That would be good idea.
Thanks, will do.
> What about 'gitweb: fix variable scoping issue in git_get_projects_list'
> latest version of patch, adding (re)declaring $project_maxdepth,
> $projectroot as global variables using our? Or are you waiting for
> response of our resident Perl expert: Randal L. Schwartz (merlyn)?
Not in my queue, but no need to resend --- I can find it.
^ permalink raw reply
* Re: [RFC PATCH 1/2] add a --delete option to git push
From: Junio C Hamano @ 2009-08-15 2:01 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Jakub Narebski, Git List
In-Reply-To: <7v7hx6suiy.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> One more thing. I suspect that adding --delete and nothing else probably
> makes things worse than not doing anything.
>
> In a mental model where "push there --delete $branch" is natural, there
> are branches on the other side, and when you run 'push' command, you name
> the special operation, 'delete', that you would want to inflict on them.
>
> In such a world, there probably are other (perhaps not so special)
> operations you can inflict on the branches on the other side as well.
> They are probably called something like:
>
> push there --create $branch $commit
> push there --update $branch $commit
>
> If you give them only --delete without completing the vocabulary by giving
> these operations as well, you would force people to mix "git native" world
> model (i.e. there is no "mode" nor "opration"; there is only "list of
> instructions, each of which encodes the equivalent of 'mode'") with this
> Hg-inspired world model.
One final note on this topic. A more problematic is --rename.
In the "there are branches on the other side, and a push command can be
told to operate on them in different ways" world model, you naturally
would want to:
push there --delete $branch
push there --rename $old $new
push there --copy $old $new
The first one you can implement as a syntax sugar by turning it into the
native way of pushing ":$branch". You however cannot shoehorn rename and
copy, because the way in which git push works does not have direct access
to their "original" values of remote branches. If we do not have the
current object $old points at, you cannot emulate "rename $old $new" nor
"copy $old $new" only with simple syntax sugarcoating. We would likely
need a protocol extension to fit them in, and that is of dubious value.
Among the ones Jakub listed in another message:
1. "<scm> init <directory>" (done)
2. "hg commit --close-branch" vs slightly cryptic
"git push origin :refs/heads/feature-tweak", which can be written
simply as "git push origin :feature-tweak" I think.
(that is what this patch series is about)
3. Numeric local references, e.g. 18:a432bc and "hg checkout 18"...
but more realistic example would be "hg checkout 6324" :-P
4. sensible defaults: meaning of revert, staging area (i.e. commit -a)
5. "hg serve" (gitweb and a kind of git-daemon equivalent)
I think only #1 and #5 make sense (and I wonder if it would be enough to
mention "instaweb" and "daemon" to cover #5).
As we discussed in this thread at length, #2 is on the borderline. It
makes sense if you take only --delete out of possible vocabulary, but when
you think about it as a part of a coherent whole, it becomes somewhat
iffy---it does not fit particularly well with other parts of the system.
I think #3 and #4 comes from fundamental difference of the world views.
Regarding #3, giving monotonically increasing numbers to local commits
would not be very hard without breaking the underlying data model. You
would automatically tag the original commit whenever a new branch is
created, and count commits relative to that origin-point tag for the
current branch, perhaps following only the first parent chain, or
something like that.
For such a incrementing number be any useful, the user's history should
rarely rewind, and this also introduces a notion/illusion that a branch
somehow has its own identity, which we deliberately have rejected. I doubt
this is worth it. Most of the time you are interested in the recent past,
and HEAD~14 would be far easier to use than r19354 anyway ;-)
About #4, in general, I do not think it is worth discussing a topic that
begins with the word "sensible" when the person who uses that word does
not realize that what is sensible is in the eyes of beholder. Different
SCMs use "revert" to mean different things, because there are a lot of
combinations of _things_ you would want to revert, _states_ you would want
to revert to, and _ways_ you would want the revert to be expressed. You
may be familiar with the way BK used the word revert, or you may be
familiar with the way SVN used the word revert. There is no single
"right" use of the word.
It also is not worth repeating the discussion on fear of index, either.
This is a good example of why you should _think_ before blindly parrotting
whatever random people say on the net. They have not necessarily thought
things through before saying "git can learn from X". You need to do the
thinking for them to decide if they are making any sense when you read
these things.
^ permalink raw reply
* Re: [RFC PATCH v3 8/8] --sparse for porcelains
From: Junio C Hamano @ 2009-08-15 2:01 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Nguyen Thai Ngoc Duy, git, Johannes Schindelin
In-Reply-To: <200908142223.07994.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
>>> Hmmm... this looks like either argument for introducing --full option
>>> to git-checkout (ignore CE_VALID bit, checkout everything, and clean
>>> CE_VALID (?))...
>>>
>>> ...or for going with _separate_ bit for partial checkout, like in the
>>> very first version of this series, which otherwise functions like
>>> CE_VALID, or is just used to mark that CE_VALID was set using sparse.
How would a separate bit help? Just like you need to clear CE_VALID bit
to revert the index into a normal (or "non sparse") state somehow, you
would need to have a way to clear that separate bit anyway.
A separate bit would help only if you want to handle assume-unchanged and
sparse checkout independently. But my impression was that the recent lstat
reduction effort addressed the issue assume-unchanged were invented to
work around in the first place.
Cf. http://thread.gmane.org/gmane.comp.version-control.git/123218/focus=123252
There is no reason to use assume-unchanged to tell git not to lstat to see
if a path is up-to-date by promising that you are not going to touch it
after you checked it out.
So I do not understand why you would want a separate bit, nor why you
think a separate bit would help when changing the index state from sparse
to non-sparse (or vice versa).
^ permalink raw reply
* Re: [PATCH] block-sha1/sha1.c: silence compiler complaints by casting void* to uintptr_t
From: Brandon Casey @ 2009-08-15 2:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: Linus Torvalds, Johannes Schindelin, nico, git, Brandon Casey
In-Reply-To: <7v8whl7ss9.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Brandon Casey <casey@nrlssc.navy.mil> writes:
>
>> Linus Torvalds wrote:
>>> I suspect it might as well be cast to "const char *", and then
>>> hopefully you only need one cast.
>>>
>>> So maybe it could be written as
>>>
>>> data = (const char *) data + len;
>>>
>>> instead, and avoid the second cast (because the assignment should be ok,
>>> since it's assigning back to a "const void *").
>> Yep, that's enough. It produces an identical binary on all platforms
>> except Solaris x86 using SUNWspro compiler.
>
> Casting to "const char *" to tell compilers that we are interested in byte
> address differences/increments makes sense to me,
Yes, I agree.
> but I do not know how to interpret your last comment.
> Do you mean that SUNWspro compiler misbehave with the "cast to char *" and
> do you meed your "casting to uintptr_t to explicitly compute byte
> addresses as int" to make it behave?
No, sorry, the SUNWspro compiler produces a working binary with any of the
three versions: the original, cast to uintptr_t, or cast to const char*.
Though it emits a warning with the original.
I was just pointing out that the SUNWspro compiler does not produce an
identical binary on x86 for each of the three versions like the GNU
compiler does.
Sorry for the confusion. Linus's suggestion of casting to "const char*"
should be adopted.
-brandon
^ permalink raw reply
* Re: [PATCH] block-sha1/sha1.c: silence compiler complaints by casting void* to uintptr_t
From: Junio C Hamano @ 2009-08-15 3:16 UTC (permalink / raw)
To: Brandon Casey
Cc: Junio C Hamano, Linus Torvalds, Johannes Schindelin, nico, git,
Brandon Casey
In-Reply-To: <MecVLMUB15cEJQiZpwSBg9ysnmFSB1QYZfIQuSOStnFx2KoeScNXGQ@cipher.nrlssc.navy.mil>
Brandon Casey <casey@nrlssc.navy.mil> writes:
> I was just pointing out that the SUNWspro compiler does not produce an
> identical binary on x86 for each of the three versions like the GNU
> compiler does.
>
> Sorry for the confusion. Linus's suggestion of casting to "const char*"
> should be adopted.
Ah, I see. Thanks.
Let's queue this, then.
-- >8 --
From: Brandon Casey <drafnel@gmail.com>
Date: Fri, 14 Aug 2009 17:52:15 -0500
Subject: [PATCH] block-sha1/sha1.c: silence compiler complaints by casting void * to char *
Some compilers produce errors when arithmetic is attempted on pointers to
void. We want computations done on byte addresses, so cast them to char *
to work them around.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
block-sha1/sha1.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index e5a1007..464cb25 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -246,14 +246,14 @@ void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, unsigned long len)
memcpy(lenW + (char *)ctx->W, data, left);
lenW = (lenW + left) & 63;
len -= left;
- data += left;
+ data = ((const char *)data + left);
if (lenW)
return;
blk_SHA1_Block(ctx, ctx->W);
}
while (len >= 64) {
blk_SHA1_Block(ctx, data);
- data += 64;
+ data = ((const char *)data + 64);
len -= 64;
}
if (len)
^ permalink raw reply related
* Re: rebase-with-history -- a technique for rebasing without trashing your repo history
From: Björn Steinbrink @ 2009-08-15 3:36 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Git Mailing List
In-Reply-To: <4A85D53D.9050805@alum.mit.edu>
On 2009.08.14 23:21:01 +0200, Michael Haggerty wrote:
> Björn Steinbrink wrote:
> > On 2009.08.14 00:39:48 +0200, Michael Haggerty wrote:
> >> Björn Steinbrink wrote:
> >>> On 2009.08.13 14:46:07 +0200, Michael Haggerty wrote:
> > [...]
> > Doing a plain "git rebase subsystem topic" would of course also try to
> > rebase the "o" commits, so that problematic. Instead, you do:
> >
> > git rebase --onto subsystem O topic
> >
> > That turns O..topic (the * commits) into patches, and applies them on
> > top of O'. So the "o" commits aren't to be rebased.
> >
> > And that's exactly what your rebase-with-history would do as well. Just
> > that O is naturally a common ancestor of subsystem and topic, and so
> > just using "git rebase-w-h subsystem topic" would be enough. Conflicts
> > etc. should be 100% the same.
> >
> > If you know that your upstream is going to rebase/rewrite history, you
> > can tag (or otherwise mark) the current branching point of your branch,
> > so you can easily specify it for the --onto rebase. IOW: This is
> > primarily a social problem (tell your downstream that you rebase this or
> > that branch), but having built-in support to store the branching point
> > for rebasing _might_ be worth a thought.
>
> Recording branch points manually, coordinating merges via email -- OMG
> you are giving me flashbacks of CVS ;-)
Not merging, but rewriting history. One of the primary purposes of
rebasing is to forget the old history, the new version overrides it. And
telling someone to forget something is a social problem. You can help
the user to forget the history by tracking the branching points and I
said that git could maybe learn to do that, so the user doesn't have
to do so. Quick idea:
On branch creation, create refs/bases/<branchname> (let's call that
<base>) referencing the commit the branch initially references.
On rebase, check if <branchname>..<onto> is not empty. If so, update
refs/bases/<branchname> to reference <base>.
On reset, check if the commit the branch head is being reset to is
reachable through the commit the branch head currently references. If
not, update <base> to reference the commit we're resetting to.
Find some sane syntax for rebase that implicitly uses <base> as the
<upstream> argument, e.g. just "git rebase --onto <whatever>" could work
as "git rebase --onto <whatever> <base>".
Most likely, I missed a bunch of corner cases though...
> *Of course* you can get around all of these problems if you put the
> burden of bookkeeping on the user. The whole point of
> rebase-with-history is to have the VCS handle it automatically!
What your approach does, is simply moving the "just forget the
history" part. Instead of forgetting it at rebase time, you have to
forget it when you want to submit patches. It's obviously a bit easier
though, as you can just say "--first-parent <upstream>", assuming that
you teach format-patch to use a special first-parent diff mode for the
merge commits (see below).
> >> and merging in a topic branch makes it more difficult to create an
> >> easily-reviewable patch series. rebase-with-history has neither of
> >> these problems.
> >
> > Sure, merging is a no-go if you submit patches by email (or other,
> > similar means). But you compared that to an "enhanced" rebase approach,
> > instead of comparing your rebase approach to the currently available
> > one.
>
> In [1] I compared rebase-with-history with both of the
> currently-available options (rebase and merge). Rebase and merge can
> each deal with some of the issues that come up, but each one falls flat
> on others. I believe that rebase-with-history has the advantages of both.
And some disadvantages.
1) Cluttered history, which needs to be rewritten again when the emailed
patches are just for review, but the maintainer will actually merge from
you later.
Taking the old master, subsystem, topic example, you get (for example):
o2--o2 (subsystem)
/ \
m---m---m---m---m (master)
\ \
\ o'--o'
\ / / \
o---o *'--*' (topic)
\ / /
*---*
Now the user that maintains "topic" is back at the hard case. He now
needs to rebase onto master, using the last o' as <upstream>. The
DAG doesn't help here, the base-tracking would handle that.
2) Merge commits, which are usually displayed in a special format. So
for "git show" or "git log -p" to give useful output for those special
merges, you'd have to introduce a new "diff only against first-parent"
mode, and mark those merge in a special way, so that diff mode is used
for them, but not for real merges. And users of old git versions would
have to deal with the basic -m merge diff mode, ignoring the useless
diff for the second parent and the fact that the real merges also get
shown in that format. The base tracking doesn't have this problem
either.
> The example in [2] was taken straight from the git-rebase man page [3];
> I did not want to claim that current practice would use merging in this
> situation, but rather just to show that rebase-with-history removes the
> pain from this well-known example.
Well, the man pages says: Don't merge, rebase needs to trickle down, but
you'll likely need to use "git rebase --onto subsystem subsystem@{1}".
So the rebase-with-history really just saves that "use --onto and the
right <upstream>" from the hard case. The plain base-tracking does the
same.
Another way to reach the same goal would be just to explictly override
the old history.
m---m---m (master)
\
o---o (subsystem)
\
*---* (topic)
(Hypothetical): git rebase --override master subsystem
Leads to:
m---m---m---- (master)
\ \
o---o---O---o'--o' (subsystem)
\
*---* (topic)
Where O is an --ours merge, that just marks the old o commits as merged,
but has the same tree as the last m commit.
Now topic can be rebase using: git rebase --override subsystem topic
m---m---m---- (master)
\ \
o---o---O---o'--o' (subsystem)
\ \
*---*-------X---*'--*' (topic)
Again, X being an ours merge.
As the O and X commits have the last o and * commits as their second
parents, this even doesn't break things like "git show" and "git log
-p", as the interesting commits aren't merge commits. So "git
log -p --first-parent subsystem..topic" would do the right thing
(optionally with --no-merges to avoid the merge commit, but seeing that
doesn't hurt that much I guess).
This also trivially supports the reorder, squash, edit whatever stuff,
as it doesn't rely on 1:1 commit counterparts to exist. But it also
falls flat on its face as soon as subsystem gets "really" rewritting, so
that the old history is no longer reachable from the new history.
Björn
^ permalink raw reply
* Re: [PATCH v5 0/6] {checkout,reset,stash} --patch
From: Jeff King @ 2009-08-15 6:51 UTC (permalink / raw)
To: Thomas Rast
Cc: Junio C Hamano, git, Sverre Rabbelier, Nanako Shiraishi,
Nicolas Sebrecht, Pierre Habouzit
In-Reply-To: <cover.1250164190.git.trast@student.ethz.ch>
On Thu, Aug 13, 2009 at 02:29:38PM +0200, Thomas Rast wrote:
> Those marked (**) are the only ones that changed semantics compared to
> v4. However, I adjusted the messages to look different:
Thanks for following up on this. This is something I've wanted for a
while, and now my procrastination is paying off. ;)
> add -p Stage this hunk?
Makes sense.
> reset -p [HEAD] Reset this hunk? (**)
Actually, it says "Unstage this hunk", but I like that even better.
> reset -p other Apply this hunk to index? (**)
This doesn't make sense to me. For example:
$ git show HEAD^:file
content
$ git show :file
content
with some changes
$ git reset -p HEAD^
diff --git a/file b/file
index d95f3ad..60a1a4e 100644
--- a/file
+++ b/file
@@ -1 +1,2 @@
content
+with some changes
Apply this hunk to index [y,n,q,a,d,/,e,?]?
The hunk is _already_ in the index. You are really asking to remove it
from the index. So shouldn't it say something like "Unstage this hunk"
or "Remove this hunk from the index"?
Or did you intend to reverse the diff, as with "checkout -p" below?
> checkout -p Discard this hunk from worktree? (**)
Good, that addresses my earlier confusion.
> checkout -p HEAD Discard this hunk from index and worktree? (**)
Good. I like how it clarifies what is being touched.
> checkout -p other Apply this hunk to index and worktree? (**)
I really expected this to just be the same as the "HEAD" case. That is,
with "git checkout -p HEAD", you are saying "I'm not interested in these
bits, discard to return back to HEAD". So if I do "git checkout -p
HEAD^", that is conceptually the same thing, except going back further
in time.
But I guess you are thinking of it as "pull these changes out of
'other'", in which case showing the reverse diff makes sense.
I think this may be a situation where the user has one of two mental
models in issuing the command, and we don't necessarily know which. So I
guess what you have is fine, but I wanted to register my surprise.
> stash -p Stash this hunk?
Getting greedy, is there a reason not to have "stash apply -p" as well?
-Peff
^ permalink raw reply
* jc/shortstatus (was: What's cooking in git.git (Aug 2009, #02; Wed, 12))
From: Jeff King @ 2009-08-15 7:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtz0co3xe.fsf@alter.siamese.dyndns.org>
On Wed, Aug 12, 2009 at 07:14:53PM -0700, Junio C Hamano wrote:
> * jc/shortstatus (Tue Aug 4 23:55:22 2009 -0700) 12 commits
> - git stat -s: short status output
> - git stat: pathspec limits, unlike traditional "git status"
> - git stat: the beginning
> + wt-status: collect untracked files in a separate "collect" phase
> + Make git_status_config() file scope static to builtin-commit.c
> + wt-status: move wt_status_colors[] into wt_status structure
> + wt-status: move many global settings to wt_status structure
> + commit: --dry-run
> + status: show worktree status of conflicted paths separately
> + wt-status.c: rework the way changes to the index and work tree are
> summarized
> + diff-index: keep the original index intact
> + diff-index: report unmerged new entries
>
> Slowly progressing. I've addressed issues pointed out by Jeff in his
> review, and hopefully the whole thing would be ready to go.
I briefly looked over what's in next, and the fixes you made look good
to me. I've been running with this series for a little while, and I was
very pleased when "git status" showed me a more detailed description of
some unmerged paths the other day. :)
For the "git stat" portion still in pu, I have a few comments/questions:
1. Is "stat" a good name? I worry a little that it is _too_ similar to
"status", and that will cause confusion while they both exist. So
something like "git overview" would cause less confusion, and even
though it sucks to type, it will eventually replace "status" (and
in the meantime people can make aliases or whatever).
2. Does it really belong in builtin-commit.c anymore? It seems like
historical accident that "status" is so closely tied to commit. The
whole point of the new command is to _not_ be tied; I think of the
new command more as an extension of 'diff'. Admittedly, users don't
care where the source is located, but it helps the developers
understand the relationships between code.
3. Can you squash in the gitignore patch below? :)
---
diff --git a/.gitignore b/.gitignore
index c446290..14e0f51 100644
--- a/.gitignore
+++ b/.gitignore
@@ -128,6 +128,7 @@ git-show-index
git-show-ref
git-stage
git-stash
+git-stat
git-status
git-stripspace
git-submodule
^ permalink raw reply related
* Re: [PATCH v5 0/6] {checkout,reset,stash} --patch
From: Junio C Hamano @ 2009-08-15 7:57 UTC (permalink / raw)
To: Jeff King
Cc: Thomas Rast, git, Sverre Rabbelier, Nanako Shiraishi,
Nicolas Sebrecht, Pierre Habouzit
In-Reply-To: <20090815065125.GA23068@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> reset -p [HEAD] Reset this hunk? (**)
>> reset -p other Apply this hunk to index? (**)
>
> This doesn't make sense to me.
Not to me, either.
Let's say you have modified $path and ran "git add $path" earlier.
"reset -p -- $path" and "reset -p HEAD -- $path" both show what your index
has relative to the commit you are resetting your index to and offer to
"Unstage" [*1*]. This is consistent and feels natural.
"reset -p HEAD^ -- $path" however shows the same forward diff (i.e. how
your index is different compared to the commit HEAD^ you are resetting
to), but offers to "Apply".
When a user is resetting to the current HEAD (with or without an explicit
HEAD parameter), it is likely that the user did "git add" earlier and is
trying to reverse the effect of that. And showing a forward diff makes
sense. But when a user is resetting to a different commit, it may make
sense to show a reverse diff, saying "Here is a hunk you _could_ choose to
use to bring the current index to a different state, do you want to apply
it to do so?" Perhaps you meant to show a reverse diff and use the word
"Apply".
However, that would break down rather badly when HEAD did not change $path
since HEAD^. Logically what the "reset -p" would do to $path is the same,
but the patch shown and the operation offered to the user are opposite.
You could compare HEAD and the commit you are resetting the index to and
see if the path in question is different between the two commits, and
switch the direction---if there is no change, you show forward diff and
offer to "Remove this change out of the index", if there is a change, you
show reverse diff and offer to "Apply this change to the index". But if
the difference between HEAD and the commit you are resetting to does not
overlap with the change you staged to the index earlier from your work
tree, it is unclear such heuristics would yield a natural feel.
So I actually think you may be better off if you consistently showed a
forward diff (i.e. what patch would have been applied to the commit in
question to bring the index into its current shape), and always offer
"Remove this change out of the index?"
The same comment applies to "checkout -p HEAD" vs "checkout -p HEAD^".
I think the latter shouldn't show a reverse diff and offer "Apply?";
instead both should consitently show a forward diff (i.e. what patch would
have been applied to the commit to bring your work tree into its current
shape), and offer "Remove this change out of the index and the work tree?".
[Footnote]
*1* I actually have a slight problem with the use of word "Unstage" in
this context; "to stage", at least to me, means "adding _from the work
tree_ to the index", not just "modifying the index" from a random source.
The command is resetting the index in this case from a tree-ish and there
is no work tree involved, and the word "stage/unstage" feels out of place.
^ permalink raw reply
* Re: jc/shortstatus
From: Junio C Hamano @ 2009-08-15 8:18 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20090815070904.GA23389@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> For the "git stat" portion still in pu, I have a few comments/questions:
>
> 1. Is "stat" a good name? I worry a little that it is _too_ similar to
> "status", and that will cause confusion while they both exist. So
> something like "git overview" would cause less confusion, and even
> though it sucks to type, it will eventually replace "status" (and
> in the meantime people can make aliases or whatever).
It is handy to have both available while asking others help debugging the
version in 'pu', and that is the only reason for the separate command. It
could have been named 'git frotz' for all I care ;-)
I do not intend to make it another "git merge-recur"; I would actually
want to have it replace "status" before the series goes to 'next'.
I hopefully will have some time to start doing that over the weekend; the
first step would be to rename the branch to jc/1.7.0-status and get rid of
cmd_status() from builtin-commit.c.
A few points I haven't managed to think about, decide, nor test, are:
- What should its exit code be? Should it be consistent with the
traditional "git status" at least when no paths are given, signallying
failure when nothing is staged for committing, so that ancient out of
tree scripts people may have written would not break too badly when we
make the switch?
- What should its default mode of output be? Do people prefer "svn st"
style short-format output, or should we stay verbose and explanatory?
- Is it handling corner cases correctly? e.g. Does it operate correctly
when run from a subdirectory? How should it handle submodules? Before
the initial commit? Use of colors?
> 2. Does it really belong in builtin-commit.c anymore? It seems like
> historical accident that "status" is so closely tied to commit. The
> whole point of the new command is to _not_ be tied; I think of the
> new command more as an extension of 'diff'. Admittedly, users don't
> care where the source is located, but it helps the developers
> understand the relationships between code.
It would make sense to create a separate builtin-status.c. I haven't
looked at the dependencies yet, but it shouldn't be too bad. We'll see.
> 3. Can you squash in the gitignore patch below? :)
Yes but hopefully it won't be necessary ;-)
^ permalink raw reply
* Re: [PATCH] git submodule summary: add --files option
From: Lars Hjemli @ 2009-08-15 8:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jens Lehmann
In-Reply-To: <7v8whmjhqh.fsf@alter.siamese.dyndns.org>
On Fri, Aug 14, 2009 at 21:52, Junio C Hamano<gitster@pobox.com> wrote:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
>> git submodule summary is providing similar functionality for submodules as
>> git diff-index does for a git project (including the meaning of --cached).
>> But the analogon to git diff-files is missing, so add a --files option to
>> summarize the differences between the index of the super project and the
>> last commit checked out in the working tree of the submodule.
>>
>> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
>
> Makes sense to me. Comments?
Acked-by: Lars Hjemli <hjemli@gmail.com> with a tiny fixup:
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -129,7 +129,7 @@ summary::
in the submodule between the given super project commit and the
index or working tree (switched by --cached) are shown. If the option
--files is given, show the series of commits in the submodule between
- the index of super project the and the working tree of the submodule
+ the index of the super project and the working tree of the submodule
(this option doesn't allow to use the --cached option or to provide an
explicit commit).
^ permalink raw reply
* Re: jc/shortstatus
From: Jeff King @ 2009-08-15 8:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8whltrqj.fsf@alter.siamese.dyndns.org>
On Sat, Aug 15, 2009 at 01:18:28AM -0700, Junio C Hamano wrote:
> It is handy to have both available while asking others help debugging the
> version in 'pu', and that is the only reason for the separate command. It
> could have been named 'git frotz' for all I care ;-)
>
> I do not intend to make it another "git merge-recur"; I would actually
> want to have it replace "status" before the series goes to 'next'.
Ah, OK. I thought we were going to live through 1.6.5 with the dual
commands. But if it is going to be "status", I don't care at all what it
is called in the interim. :)
> - What should its exit code be? Should it be consistent with the
> traditional "git status" at least when no paths are given, signallying
> failure when nothing is staged for committing, so that ancient out of
> tree scripts people may have written would not break too badly when we
> make the switch?
If I were designing it from scratch, I would say the exit code should be
the opposite. That is, it is really about doing several diffs, and if
there are no changes, then we should, like diff, exit zero.
If you want to know whether there is something to commit, you wouldn't
to use this tool. If you just want to know if there is something in the
index to commit, you would use diff-index. If you want to see if
some set of commit parameters would make a commit, you would use "commit
--dry-run".
So really there is only the historical argument. I am inclined not to
worry about it. We are already breaking compatibility in other ways
(like how command line parameters are treated), so you are really only
helping people whose scripts use a subset of the current "git status"
functionality. And since this is the time for breaking, I think it's
best to make all of the changes we want, and not have another
half-breakage later.
> - What should its default mode of output be? Do people prefer "svn st"
> style short-format output, or should we stay verbose and explanatory?
Personally I prefer the long format, but maybe just because I'm used to
it. I suspect others want the short format. This really should be a
porcelain command[1], so I don't see a problem with a
status.outputformat config option.
[1] One can, after all, call diff-index, diff-files, and "ls-files -o"
to get the same information from plumbing. If we really want to provide
plumbing that pulls them all together (e.g., because it is more
efficient or more convenient to do it all in one command), then I think
we should provide "git status --porcelain".
> - Is it handling corner cases correctly? e.g. Does it operate correctly
> when run from a subdirectory? How should it handle submodules? Before
> the initial commit? Use of colors?
I'll try out a few things, but I think largely we will need to put it in
"next" to shake out any bugs.
-Peff
^ permalink raw reply
* [PATCH] git-log: allow --decorate[=short|full]
From: Lars Hjemli @ 2009-08-15 9:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
This extension to --decorate makes it possible to generate decorations
similar to pre-1.6.4 git, which is nice when the output from git-log
is used by external tools.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
Documentation/git-log.txt | 6 ++++--
builtin-log.c | 12 ++++++++++--
log-tree.c | 7 ++++---
log-tree.h | 2 +-
pretty.c | 2 +-
revision.c | 2 +-
revision.h | 6 +++++-
t/t4013-diff-various.sh | 1 +
...corate_--all => diff.log_--decorate=full_--all} | 8 ++++----
9 files changed, 31 insertions(+), 15 deletions(-)
copy t/t4013/{diff.log_--decorate_--all =>
diff.log_--decorate=full_--all} (72%)
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 34cf4e5..451839c 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -37,8 +37,10 @@ include::diff-options.txt[]
and <until>, see "SPECIFYING REVISIONS" section in
linkgit:git-rev-parse[1].
---decorate::
- Print out the ref names of any commits that are shown.
+--decorate[=short|full]::
+ Print out the ref names of any commits that are shown. If 'short' is
+ specified, the ref names will be shortened, and if 'full' is specified,
+ the ref names will not be shortened. The default option is 'short'.
--source::
Print out the ref name given on the command line by which each
diff --git a/builtin-log.c b/builtin-log.c
index 3817bf1..cb886d8 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -61,8 +61,14 @@ static void cmd_log_init(int argc, const char
**argv, const char *prefix,
for (i = 1; i < argc; i++) {
const char *arg = argv[i];
if (!strcmp(arg, "--decorate")) {
- load_ref_decorations();
- rev->show_decorations = 1;
+ rev->show_decorations = DECORATE_SHORT_REFS;
+ } else if (!strncmp(arg, "--decorate=", 11)) {
+ if (!strcmp(arg + 11, "full"))
+ rev->show_decorations = DECORATE_FULL_REFS;
+ else if (!strcmp(arg + 11, "short"))
+ rev->show_decorations = DECORATE_SHORT_REFS;
+ else
+ die("invalid --decorate option: %s", arg + 11);
} else if (!strcmp(arg, "--source")) {
rev->show_source = 1;
} else if (!strcmp(arg, "-h")) {
@@ -70,6 +76,8 @@ static void cmd_log_init(int argc, const char
**argv, const char *prefix,
} else
die("unrecognized argument: %s", arg);
}
+ if (rev->show_decorations)
+ load_ref_decorations(rev->show_decorations);
}
/*
diff --git a/log-tree.c b/log-tree.c
index 6f73c17..70223eb 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -25,7 +25,8 @@ static int add_ref_decoration(const char *refname,
const unsigned char *sha1, in
struct object *obj = parse_object(sha1);
if (!obj)
return 0;
- refname = prettify_refname(refname);
+ if (!cb_data || *(int *)cb_data & DECORATE_SHORT_REFS)
+ refname = prettify_refname(refname);
add_name_decoration("", refname, obj);
while (obj->type == OBJ_TAG) {
obj = ((struct tag *)obj)->tagged;
@@ -36,12 +37,12 @@ static int add_ref_decoration(const char *refname,
const unsigned char *sha1, in
return 0;
}
-void load_ref_decorations(void)
+void load_ref_decorations(int flags)
{
static int loaded;
if (!loaded) {
loaded = 1;
- for_each_ref(add_ref_decoration, NULL);
+ for_each_ref(add_ref_decoration, &flags);
}
}
diff --git a/log-tree.h b/log-tree.h
index 20b5caf..3f7b400 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -17,7 +17,7 @@ void log_write_email_headers(struct rev_info *opt,
struct commit *commit,
const char **subject_p,
const char **extra_headers_p,
int *need_8bit_cte_p);
-void load_ref_decorations(void);
+void load_ref_decorations(int flags);
#define FORMAT_PATCH_NAME_MAX 64
void get_patch_filename(struct commit *commit, int nr, const char *suffix,
diff --git a/pretty.c b/pretty.c
index e5328da..daa721b 100644
--- a/pretty.c
+++ b/pretty.c
@@ -571,7 +571,7 @@ static void format_decoration(struct strbuf *sb,
const struct commit *commit)
struct name_decoration *d;
const char *prefix = " (";
- load_ref_decorations();
+ load_ref_decorations(DECORATE_SHORT_REFS);
d = lookup_decoration(&name_decoration, &commit->object);
while (d) {
strbuf_addstr(sb, prefix);
diff --git a/revision.c b/revision.c
index 9f5dac5..ce24ad9 100644
--- a/revision.c
+++ b/revision.c
@@ -1052,7 +1052,7 @@ static int handle_revision_opt(struct rev_info
*revs, int argc, const char **arg
revs->simplify_by_decoration = 1;
revs->limited = 1;
revs->prune = 1;
- load_ref_decorations();
+ load_ref_decorations(DECORATE_SHORT_REFS);
} else if (!strcmp(arg, "--date-order")) {
revs->lifo = 0;
revs->topo_order = 1;
diff --git a/revision.h b/revision.h
index fb74492..16e65f6 100644
--- a/revision.h
+++ b/revision.h
@@ -15,6 +15,10 @@
#define SYMMETRIC_LEFT (1u<<8)
#define ALL_REV_FLAGS ((1u<<9)-1)
+
+#define DECORATE_SHORT_REFS 1
+#define DECORATE_FULL_REFS 2
+
struct rev_info;
struct log_info;
@@ -56,7 +60,7 @@ struct rev_info {
rewrite_parents:1,
print_parents:1,
show_source:1,
- show_decorations:1,
+ show_decorations:2,
reverse:1,
reverse_output_stage:1,
cherry_pick:1,
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 8b33321..8e3694e 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -207,6 +207,7 @@ log --root --cc --patch-with-stat --summary master
log -SF master
log -SF -p master
log --decorate --all
+log --decorate=full --all
rev-list --parents HEAD
rev-list --children HEAD
diff --git a/t/t4013/diff.log_--decorate_--all
b/t/t4013/diff.log_--decorate=full_--all
similarity index 72%
copy from t/t4013/diff.log_--decorate_--all
copy to t/t4013/diff.log_--decorate=full_--all
index 954210e..903d9d9 100644
--- a/t/t4013/diff.log_--decorate_--all
+++ b/t/t4013/diff.log_--decorate=full_--all
@@ -1,12 +1,12 @@
-$ git log --decorate --all
-commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (master)
+$ git log --decorate=full --all
+commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (refs/heads/master)
Merge: 9a6d494 c7a2ab9
Author: A U Thor <author@example.com>
Date: Mon Jun 26 00:04:00 2006 +0000
Merge branch 'side'
-commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (side)
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a (refs/heads/side)
Author: A U Thor <author@example.com>
Date: Mon Jun 26 00:03:00 2006 +0000
@@ -26,7 +26,7 @@ Date: Mon Jun 26 00:01:00 2006 +0000
This is the second commit.
-commit 444ac553ac7612cc88969031b02b3767fb8a353a (initial)
+commit 444ac553ac7612cc88969031b02b3767fb8a353a (refs/heads/initial)
Author: A U Thor <author@example.com>
Date: Mon Jun 26 00:00:00 2006 +0000
--
1.6.4.135.g4e5b
^ permalink raw reply related
* Re: [PATCH v5 0/6] {checkout,reset,stash} --patch
From: Thomas Rast @ 2009-08-15 10:04 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, git, Sverre Rabbelier, Nanako Shiraishi,
Nicolas Sebrecht, Pierre Habouzit
In-Reply-To: <20090815065125.GA23068@coredump.intra.peff.net>
Jeff King wrote:
> > reset -p other Apply this hunk to index? (**)
>
> This doesn't make sense to me. For example:
[...]
> The hunk is _already_ in the index. You are really asking to remove it
> from the index. So shouldn't it say something like "Unstage this hunk"
> or "Remove this hunk from the index"?
>
> Or did you intend to reverse the diff, as with "checkout -p" below?
Yes, sorry :-( I apparently managed to forget the reversing here and
never noticed. I'll make a fixed patch 4/6 today.
> > checkout -p HEAD Discard this hunk from index and worktree? (**)
>
> Good. I like how it clarifies what is being touched.
>
> > checkout -p other Apply this hunk to index and worktree? (**)
>
> I really expected this to just be the same as the "HEAD" case. That is,
> with "git checkout -p HEAD", you are saying "I'm not interested in these
> bits, discard to return back to HEAD". So if I do "git checkout -p
> HEAD^", that is conceptually the same thing, except going back further
> in time.
>
> But I guess you are thinking of it as "pull these changes out of
> 'other'", in which case showing the reverse diff makes sense.
>
> I think this may be a situation where the user has one of two mental
> models in issuing the command, and we don't necessarily know which. So I
> guess what you have is fine, but I wanted to register my surprise.
Well, as I said earlier in the thread I'd hate to reverse the
direction of plain "{reset,checkout,stash} -p" because I want them to
show hunks that match visually.
But then my mental model of "checkout other -- file" is already of the
sort "fetch me the contents of 'file' from 'other'", and I think I use
it about as frequently in a forward as in a backward application. And
I just felt it would be easier to mentally go over the consequences if
it shoed the diff the other way.
(I'm sufficiently confused about which way is back that I think I'll
just stop saying "backward"...)
> > stash -p Stash this hunk?
>
> Getting greedy, is there a reason not to have "stash apply -p" as well?
Should be doable, I'll look at it.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* [PATCH 0/6] "git commit --dry-run" updates
From: Junio C Hamano @ 2009-08-15 10:06 UTC (permalink / raw)
To: git; +Cc: Jeff King
The first patch describes --dry-run option, and stops mentioning the
equivalence to "git status" in the documentation for "git commit".
The second one fixes "git commit --dry-run -v", which did not show the
diff text in color when asked with the configuration.
The third and the fourth ones tentatively introduce "git stat". The
former is an incomplete implementation that ignores the pathspec, and the
latter adds pathspec limiting to it. In the final form before it goes to
'next', these two should probably be squashed.
The fifth one introduces "git stat -s" (short output format) that is
designed to be machine readable, especially with -z (NUL termination).
The sixth one renames "git stat" to take over the traditional "git status"
and obviously is a 1.7.0 material.
The tests still have breakages that future updates to the sixth patch need
to fix. t4030 still assumes "git status" is "git commit --dry-run" and
expects it to take "-v" to produce diffs, for example.
^ permalink raw reply
* [PATCH 2/6] git commit --dry-run -v: show diff in color when asked
From: Junio C Hamano @ 2009-08-15 10:06 UTC (permalink / raw)
To: git; +Cc: Jeff King
In-Reply-To: <1250330803-22171-2-git-send-email-gitster@pobox.com>
The earlier implementation of --dry-run didn't duplicate the use of color
"git status -v" set up for diff output.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Fixes what is already queued in 'next'.
builtin-commit.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 1c200eb..200ffda 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -979,9 +979,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
argc = parse_and_validate_options(argc, argv, builtin_commit_usage,
prefix, &s);
- if (dry_run)
+ if (dry_run) {
+ if (diff_use_color_default == -1)
+ diff_use_color_default = git_use_color_default;
return dry_run_commit(argc, argv, prefix, &s);
-
+ }
index_file = prepare_index(argc, argv, prefix, 0);
/* Set up everything for writing the commit object. This includes
--
1.6.4.224.g3be84
^ permalink raw reply related
* [PATCH 1/6] Documentation/git-commit.txt: describe --dry-run
From: Junio C Hamano @ 2009-08-15 10:06 UTC (permalink / raw)
To: git; +Cc: Jeff King
In-Reply-To: <1250330803-22171-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* The --dry-run option is already in 'next', but haven't been described.
Documentation/git-commit.txt | 13 +++++++++----
1 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index d01ff5a..64f94cf 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -9,7 +9,7 @@ SYNOPSIS
--------
[verse]
'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
- [(-c | -C) <commit>] [-F <file> | -m <msg>]
+ [(-c | -C) <commit>] [-F <file> | -m <msg>] [--dry-run]
[--allow-empty] [--no-verify] [-e] [--author=<author>]
[--cleanup=<mode>] [--] [[-i | -o ]<file>...]
@@ -42,10 +42,9 @@ The content to be added can be specified in several ways:
by one which files should be part of the commit, before finalizing the
operation. Currently, this is done by invoking 'git-add --interactive'.
-The 'git-status' command can be used to obtain a
+The `--dry-run` option can be used to obtain a
summary of what is included by any of the above for the next
-commit by giving the same set of parameters you would give to
-this command.
+commit by giving the same set of parameters (options and paths).
If you make a commit and then find a mistake immediately after
that, you can recover from it with 'git-reset'.
@@ -70,6 +69,12 @@ OPTIONS
Like '-C', but with '-c' the editor is invoked, so that
the user can further edit the commit message.
+--dry-run::
+ Do not actually make a commit, but show the list of paths
+ with updates in the index, paths with changes in the work tree,
+ and paths that are untracked, similar to the one that is given
+ in the commit log editor.
+
-F <file>::
--file=<file>::
Take the commit message from the given file. Use '-' to
--
1.6.4.224.g3be84
^ permalink raw reply related
* [PATCH 3/6] git stat: the beginning
From: Junio C Hamano @ 2009-08-15 10:06 UTC (permalink / raw)
To: git; +Cc: Jeff King
In-Reply-To: <1250330803-22171-3-git-send-email-gitster@pobox.com>
Tentatively add "git stat" as a new command. This does not munge the
index with paths parameters before showing the status like "git status"
does. In later rounds, we will take path parameters as pathspec to limit
the output.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* The change since the version cooking in 'pu' is that the long format
honors the color (both "status" colors and "diff" colors).
Makefile | 1 +
builtin-commit.c | 64 +++++++++++++++++++++++++++++++++++++++++++++--------
builtin.h | 1 +
git.c | 1 +
wt-status.c | 23 +++++++++++-------
wt-status.h | 1 +
6 files changed, 72 insertions(+), 19 deletions(-)
diff --git a/Makefile b/Makefile
index daf4296..39dd334 100644
--- a/Makefile
+++ b/Makefile
@@ -378,6 +378,7 @@ BUILT_INS += git-init$X
BUILT_INS += git-merge-subtree$X
BUILT_INS += git-peek-remote$X
BUILT_INS += git-repo-config$X
+BUILT_INS += git-stat$X
BUILT_INS += git-show$X
BUILT_INS += git-stage$X
BUILT_INS += git-status$X
diff --git a/builtin-commit.c b/builtin-commit.c
index 200ffda..0ef7c8f 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -24,6 +24,7 @@
#include "string-list.h"
#include "rerere.h"
#include "unpack-trees.h"
+#include "quote.h"
static const char * const builtin_commit_usage[] = {
"git commit [options] [--] <filepattern>...",
@@ -35,6 +36,11 @@ static const char * const builtin_status_usage[] = {
NULL
};
+static const char * const builtin_stat_usage[] = {
+ "git stat [options]",
+ NULL
+};
+
static unsigned char head_sha1[20], merge_head_sha1[20];
static char *use_message_buffer;
static const char commit_editmsg[] = "COMMIT_EDITMSG";
@@ -691,6 +697,21 @@ static const char *find_author_by_nickname(const char *name)
die("No existing author found with '%s'", name);
}
+
+static void handle_untracked_files_arg(struct wt_status *s)
+{
+ if (!untracked_files_arg)
+ ; /* default already initialized */
+ else if (!strcmp(untracked_files_arg, "no"))
+ s->show_untracked_files = SHOW_NO_UNTRACKED_FILES;
+ else if (!strcmp(untracked_files_arg, "normal"))
+ s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
+ else if (!strcmp(untracked_files_arg, "all"))
+ s->show_untracked_files = SHOW_ALL_UNTRACKED_FILES;
+ else
+ die("Invalid untracked files mode '%s'", untracked_files_arg);
+}
+
static int parse_and_validate_options(int argc, const char *argv[],
const char * const usage[],
const char *prefix,
@@ -794,16 +815,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
else
die("Invalid cleanup mode %s", cleanup_arg);
- if (!untracked_files_arg)
- ; /* default already initialized */
- else if (!strcmp(untracked_files_arg, "no"))
- s->show_untracked_files = SHOW_NO_UNTRACKED_FILES;
- else if (!strcmp(untracked_files_arg, "normal"))
- s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
- else if (!strcmp(untracked_files_arg, "all"))
- s->show_untracked_files = SHOW_ALL_UNTRACKED_FILES;
- else
- die("Invalid untracked files mode '%s'", untracked_files_arg);
+ handle_untracked_files_arg(s);
if (all && argc > 0)
die("Paths with -a does not make sense.");
@@ -886,6 +898,38 @@ static int git_status_config(const char *k, const char *v, void *cb)
return git_diff_ui_config(k, v, NULL);
}
+int cmd_stat(int argc, const char **argv, const char *prefix)
+{
+ struct wt_status s;
+ unsigned char sha1[20];
+ static struct option builtin_stat_options[] = {
+ { OPTION_STRING, 'u', "untracked-files", &untracked_files_arg,
+ "mode",
+ "show untracked files, optional modes: all, normal, no. (Default: all)",
+ PARSE_OPT_OPTARG, NULL, (intptr_t)"all" },
+ OPT_END(),
+ };
+
+ wt_status_prepare(&s);
+ git_config(git_status_config, &s);
+ argc = parse_options(argc, argv, prefix,
+ builtin_stat_options,
+ builtin_stat_usage, 0);
+ handle_untracked_files_arg(&s);
+
+ read_cache();
+ refresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);
+ s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
+ wt_status_collect(&s);
+
+ if (s.use_color == -1)
+ s.use_color = git_use_color_default;
+ if (diff_use_color_default == -1)
+ diff_use_color_default = git_use_color_default;
+ wt_status_print_body(&s);
+ return 0;
+}
+
int cmd_status(int argc, const char **argv, const char *prefix)
{
struct wt_status s;
diff --git a/builtin.h b/builtin.h
index 20427d2..eeaf0b6 100644
--- a/builtin.h
+++ b/builtin.h
@@ -95,6 +95,7 @@ extern int cmd_send_pack(int argc, const char **argv, const char *prefix);
extern int cmd_shortlog(int argc, const char **argv, const char *prefix);
extern int cmd_show(int argc, const char **argv, const char *prefix);
extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
+extern int cmd_stat(int argc, const char **argv, const char *prefix);
extern int cmd_status(int argc, const char **argv, const char *prefix);
extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
diff --git a/git.c b/git.c
index 807d875..de7fcf6 100644
--- a/git.c
+++ b/git.c
@@ -350,6 +350,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "shortlog", cmd_shortlog, USE_PAGER },
{ "show-branch", cmd_show_branch, RUN_SETUP },
{ "show", cmd_show, RUN_SETUP | USE_PAGER },
+ { "stat", cmd_stat, RUN_SETUP | NEED_WORK_TREE },
{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
{ "stripspace", cmd_stripspace },
{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
diff --git a/wt-status.c b/wt-status.c
index 63598ce..c887a90 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -531,6 +531,19 @@ static void wt_status_print_tracking(struct wt_status *s)
color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
}
+void wt_status_print_body(struct wt_status *s)
+{
+ wt_status_print_unmerged(s);
+ wt_status_print_updated(s);
+ wt_status_print_changed(s);
+ if (s->submodule_summary)
+ wt_status_print_submodule_summary(s);
+ if (s->show_untracked_files)
+ wt_status_print_untracked(s);
+ else if (s->commitable)
+ fprintf(s->fp, "# Untracked files not listed (use -u option to show untracked files)\n");
+}
+
void wt_status_print(struct wt_status *s)
{
unsigned char sha1[20];
@@ -561,15 +574,7 @@ void wt_status_print(struct wt_status *s)
color_fprintf_ln(s->fp, color(WT_STATUS_HEADER, s), "#");
}
- wt_status_print_unmerged(s);
- wt_status_print_updated(s);
- wt_status_print_changed(s);
- if (s->submodule_summary)
- wt_status_print_submodule_summary(s);
- if (s->show_untracked_files)
- wt_status_print_untracked(s);
- else if (s->commitable)
- fprintf(s->fp, "# Untracked files not listed (use -u option to show untracked files)\n");
+ wt_status_print_body(s);
if (s->verbose)
wt_status_print_verbose(s);
diff --git a/wt-status.h b/wt-status.h
index a0e7517..ab52ce1 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -54,5 +54,6 @@ struct wt_status {
void wt_status_prepare(struct wt_status *s);
void wt_status_print(struct wt_status *s);
void wt_status_collect(struct wt_status *s);
+void wt_status_print_body(struct wt_status *s);
#endif /* STATUS_H */
--
1.6.4.224.g3be84
^ permalink raw reply related
* [PATCH 4/6] git stat: pathspec limits, unlike traditional "git status"
From: Junio C Hamano @ 2009-08-15 10:06 UTC (permalink / raw)
To: git; +Cc: Jeff King
In-Reply-To: <1250330803-22171-4-git-send-email-gitster@pobox.com>
The "git stat" command is not "preview of commit with the same arguments";
the path parameters are not paths to be added to the pristine index (aka
"--only" option), but are taken as pathspecs to limit the output.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Unchanged since what has been queued in 'pu'. This should probably be
squashed to the previous one in the final form after review.
builtin-commit.c | 3 +++
wt-status.c | 6 ++++++
wt-status.h | 1 +
3 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 0ef7c8f..64ae45f 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -917,6 +917,9 @@ int cmd_stat(int argc, const char **argv, const char *prefix)
builtin_stat_usage, 0);
handle_untracked_files_arg(&s);
+ if (*argv)
+ s.pathspec = get_pathspec(prefix, argv);
+
read_cache();
refresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);
s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
diff --git a/wt-status.c b/wt-status.c
index c887a90..11db07e 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -269,6 +269,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)
rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
rev.diffopt.format_callback = wt_status_collect_changed_cb;
rev.diffopt.format_callback_data = s;
+ rev.prune_data = s->pathspec;
run_diff_files(&rev, 0);
}
@@ -285,6 +286,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)
rev.diffopt.detect_rename = 1;
rev.diffopt.rename_limit = 200;
rev.diffopt.break_opt = 0;
+ rev.prune_data = s->pathspec;
run_diff_index(&rev, 1);
}
@@ -297,6 +299,8 @@ static void wt_status_collect_changes_initial(struct wt_status *s)
struct wt_status_change_data *d;
struct cache_entry *ce = active_cache[i];
+ if (!ce_path_match(ce, s->pathspec))
+ continue;
it = string_list_insert(ce->name, &s->change);
d = it->util;
if (!d) {
@@ -330,6 +334,8 @@ static void wt_status_collect_untracked(struct wt_status *s)
struct dir_entry *ent = dir.entries[i];
if (!cache_name_is_other(ent->name, ent->len))
continue;
+ if (!match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL))
+ continue;
s->workdir_untracked = 1;
string_list_insert(ent->name, &s->untracked);
}
diff --git a/wt-status.h b/wt-status.h
index ab52ce1..4bc1a59 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -31,6 +31,7 @@ struct wt_status {
int is_initial;
char *branch;
const char *reference;
+ const char **pathspec;
int verbose;
int amend;
int nowarn;
--
1.6.4.224.g3be84
^ permalink raw reply related
* [PATCH 6/6] git status: not "commit --dry-run" anymore
From: Junio C Hamano @ 2009-08-15 10:06 UTC (permalink / raw)
To: git; +Cc: Jeff King
In-Reply-To: <1250330803-22171-6-git-send-email-gitster@pobox.com>
This removes tentative "git stat" and make it take over "git status".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-status.txt | 79 ++++++++++++++++++++++++++++++++++++-----
builtin-commit.c | 29 ++-------------
builtin.h | 1 -
git.c | 1 -
4 files changed, 73 insertions(+), 37 deletions(-)
diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 84f60f3..b5939d6 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -8,7 +8,7 @@ git-status - Show the working tree status
SYNOPSIS
--------
-'git status' <options>...
+'git status' [<options>...] [--] [<pathspec>...]
DESCRIPTION
-----------
@@ -20,25 +20,85 @@ are what you _would_ commit by running `git commit`; the second and
third are what you _could_ commit by running 'git-add' before running
`git commit`.
-The command takes the same set of options as 'git-commit'; it
-shows what would be committed if the same options are given to
-'git-commit'.
-
-If there is no path that is different between the index file and
-the current HEAD commit (i.e., there is nothing to commit by running
-`git commit`), the command exits with non-zero status.
+OPTIONS
+-------
+
+-s::
+--short::
+ Give the output in the short-format.
+
+-u[<mode>]::
+--untracked-files[=<mode>]::
+ Show untracked files (Default: 'all').
++
+The mode parameter is optional, and is used to specify
+the handling of untracked files. The possible options are:
++
+--
+ - 'no' - Show no untracked files
+ - 'normal' - Shows untracked files and directories
+ - 'all' - Also shows individual files in untracked directories.
+--
++
+See linkgit:git-config[1] for configuration variable
+used to change the default for when the option is not
+specified.
+
+-z::
+ Terminate entries with NUL, instead of LF. This implies `-s`
+ (short status) output format.
OUTPUT
------
The output from this command is designed to be used as a commit
template comment, and all the output lines are prefixed with '#'.
+The default, long format, is designed to be human readable,
+verbose and descriptive. They are subject to change in any time.
The paths mentioned in the output, unlike many other git commands, are
made relative to the current directory if you are working in a
subdirectory (this is on purpose, to help cutting and pasting). See
the status.relativePaths config option below.
+In short-format, the status of each path is shown as
+
+ XY PATH1 -> PATH2
+
+where `PATH1` is the path in the `HEAD`, and ` -> PATH2` part is
+shown only when `PATH1` corresponds to a different path in the
+index/worktree (i.e. renamed).
+
+For unmerged entries, `X` shows the status of stage #2 (i.e. ours) and `Y`
+shows the status of stage #3 (i.e. theirs).
+
+For entries that do not have conflicts, `X` shows the status of the index,
+and `Y` shows the status of the work tree. For untracked paths, `XY` are
+`??`.
+
+ X Y Meaning
+ -------------------------------------------------
+ [MD] not updated
+ M [ MD] updated in index
+ A [ MD] added to index
+ D [ MD] deleted from index
+ R [ MD] renamed in index
+ C [ MD] copied in index
+ [MARC] index and work tree matches
+ [ MARC] M work tree changed since index
+ [ MARC] D deleted in work tree
+ -------------------------------------------------
+ D D unmerged, both deleted
+ A U unmerged, added by us
+ U D unmerged, deleted by them
+ U A unmerged, added by them
+ D U unmerged, deleted by us
+ A A unmerged, both added
+ U U unmerged, both modified
+ -------------------------------------------------
+ ? ? untracked
+ -------------------------------------------------
+
CONFIGURATION
-------------
@@ -63,8 +123,7 @@ linkgit:gitignore[5]
Author
------
-Written by Linus Torvalds <torvalds@osdl.org> and
-Junio C Hamano <gitster@pobox.com>.
+Written by Junio C Hamano <gitster@pobox.com>.
Documentation
--------------
diff --git a/builtin-commit.c b/builtin-commit.c
index 6d9bd84..efbcd16 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -36,11 +36,6 @@ static const char * const builtin_status_usage[] = {
NULL
};
-static const char * const builtin_stat_usage[] = {
- "git stat [options]",
- NULL
-};
-
static unsigned char head_sha1[20], merge_head_sha1[20];
static char *use_message_buffer;
static const char commit_editmsg[] = "COMMIT_EDITMSG";
@@ -967,13 +962,13 @@ static void short_untracked(int null_termination, struct string_list_item *it,
}
}
-int cmd_stat(int argc, const char **argv, const char *prefix)
+int cmd_status(int argc, const char **argv, const char *prefix)
{
struct wt_status s;
static int null_termination, shortstatus;
int i;
unsigned char sha1[20];
- static struct option builtin_stat_options[] = {
+ static struct option builtin_status_options[] = {
OPT_BOOLEAN('s', "short", &shortstatus,
"show status concicely"),
OPT_BOOLEAN('z', "null", &null_termination,
@@ -991,8 +986,8 @@ int cmd_stat(int argc, const char **argv, const char *prefix)
wt_status_prepare(&s);
git_config(git_status_config, &s);
argc = parse_options(argc, argv, prefix,
- builtin_stat_options,
- builtin_stat_usage, 0);
+ builtin_status_options,
+ builtin_status_usage, 0);
handle_untracked_files_arg(&s);
if (*argv)
@@ -1031,22 +1026,6 @@ int cmd_stat(int argc, const char **argv, const char *prefix)
return 0;
}
-int cmd_status(int argc, const char **argv, const char *prefix)
-{
- struct wt_status s;
-
- wt_status_prepare(&s);
- git_config(git_status_config, &s);
- if (s.use_color == -1)
- s.use_color = git_use_color_default;
- if (diff_use_color_default == -1)
- diff_use_color_default = git_use_color_default;
-
- argc = parse_and_validate_options(argc, argv, builtin_status_usage,
- prefix, &s);
- return dry_run_commit(argc, argv, prefix, &s);
-}
-
static void print_summary(const char *prefix, const unsigned char *sha1)
{
struct rev_info rev;
diff --git a/builtin.h b/builtin.h
index eeaf0b6..20427d2 100644
--- a/builtin.h
+++ b/builtin.h
@@ -95,7 +95,6 @@ extern int cmd_send_pack(int argc, const char **argv, const char *prefix);
extern int cmd_shortlog(int argc, const char **argv, const char *prefix);
extern int cmd_show(int argc, const char **argv, const char *prefix);
extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
-extern int cmd_stat(int argc, const char **argv, const char *prefix);
extern int cmd_status(int argc, const char **argv, const char *prefix);
extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
diff --git a/git.c b/git.c
index de7fcf6..807d875 100644
--- a/git.c
+++ b/git.c
@@ -350,7 +350,6 @@ static void handle_internal_command(int argc, const char **argv)
{ "shortlog", cmd_shortlog, USE_PAGER },
{ "show-branch", cmd_show_branch, RUN_SETUP },
{ "show", cmd_show, RUN_SETUP | USE_PAGER },
- { "stat", cmd_stat, RUN_SETUP | NEED_WORK_TREE },
{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
{ "stripspace", cmd_stripspace },
{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
--
1.6.4.224.g3be84
^ permalink raw reply related
* [PATCH 5/6] git stat -s: short status output
From: Junio C Hamano @ 2009-08-15 10:06 UTC (permalink / raw)
To: git; +Cc: Jeff King
In-Reply-To: <1250330803-22171-5-git-send-email-gitster@pobox.com>
Give -s(hort) option to "git stat" that shows the status of paths in a
more concise way.
XY PATH1 -> PATH2
format to be more machine readable than output from "git status", which is
about previewing of "git commit" with the same arguments.
PATH1 is the path in the HEAD, and " -> PATH2" part is shown only when
PATH1 corresponds to a different path in the index/worktree.
For unmerged entries, X shows the status of stage #2 (i.e. ours) and Y
shows the status of stage #3 (i.e. theirs). For entries that do not have
conflicts, X shows the status of the index, and Y shows the status of the
work tree. For untracked paths, XY are "??".
X Y Meaning
-------------------------------------------------
[MD] not updated
M [ MD] updated in index
A [ MD] added to index
D [ MD] deleted from index
R [ MD] renamed in index
C [ MD] copied in index
[MARC] index and work tree matches
[ MARC] M work tree changed since index
[ MARC] D deleted in work tree
D D unmerged, both deleted
A U unmerged, added by us
U D unmerged, deleted by them
U A unmerged, added by them
D U unmerged, deleted by us
A A unmerged, both added
U U unmerged, both modified
? ? untracked
When given -z option, the records are terminated by NUL characters for
better machine readability. Because the traditional long format is
designed for human consumption, NUL termination does not make sense.
For this reason, -z option implies -s (short output).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Unlike what was in 'pu', -z now implies -s. I do not think the machine
readble -s format should use color, but I _could_ be talked into
coloring the output when -z is not used.
builtin-commit.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 103 insertions(+), 5 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 64ae45f..6d9bd84 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -898,11 +898,86 @@ static int git_status_config(const char *k, const char *v, void *cb)
return git_diff_ui_config(k, v, NULL);
}
+#define quote_path quote_path_relative
+
+static void short_unmerged(int null_termination, struct string_list_item *it,
+ struct wt_status *s)
+{
+ struct wt_status_change_data *d = it->util;
+ const char *how = "??";
+
+ switch (d->stagemask) {
+ case 1: how = "DD"; break; /* both deleted */
+ case 2: how = "AU"; break; /* added by us */
+ case 3: how = "UD"; break; /* deleted by them */
+ case 4: how = "UA"; break; /* added by them */
+ case 5: how = "DU"; break; /* deleted by us */
+ case 6: how = "AA"; break; /* both added */
+ case 7: how = "UU"; break; /* both modified */
+ }
+ printf("%s ", how);
+ if (null_termination) {
+ fprintf(stdout, "%s%c", it->string, 0);
+ } else {
+ struct strbuf onebuf = STRBUF_INIT;
+ const char *one;
+ one = quote_path(it->string, -1, &onebuf, s->prefix);
+ printf("%s\n", one);
+ strbuf_release(&onebuf);
+ }
+}
+
+static void short_status(int null_termination, struct string_list_item *it,
+ struct wt_status *s)
+{
+ struct wt_status_change_data *d = it->util;
+
+ printf("%c%c ",
+ !d->index_status ? ' ' : d->index_status,
+ !d->worktree_status ? ' ' : d->worktree_status);
+ if (null_termination) {
+ fprintf(stdout, "%s%c", it->string, 0);
+ if (d->head_path)
+ fprintf(stdout, "%s%c", d->head_path, 0);
+ } else {
+ struct strbuf onebuf = STRBUF_INIT;
+ const char *one;
+ if (d->head_path) {
+ one = quote_path(d->head_path, -1, &onebuf, s->prefix);
+ printf("%s -> ", one);
+ strbuf_release(&onebuf);
+ }
+ one = quote_path(it->string, -1, &onebuf, s->prefix);
+ printf("%s\n", one);
+ strbuf_release(&onebuf);
+ }
+}
+
+static void short_untracked(int null_termination, struct string_list_item *it,
+ struct wt_status *s)
+{
+ if (null_termination) {
+ fprintf(stdout, "?? %s%c", it->string, 0);
+ } else {
+ struct strbuf onebuf = STRBUF_INIT;
+ const char *one;
+ one = quote_path(it->string, -1, &onebuf, s->prefix);
+ printf("?? %s\n", one);
+ strbuf_release(&onebuf);
+ }
+}
+
int cmd_stat(int argc, const char **argv, const char *prefix)
{
struct wt_status s;
+ static int null_termination, shortstatus;
+ int i;
unsigned char sha1[20];
static struct option builtin_stat_options[] = {
+ OPT_BOOLEAN('s', "short", &shortstatus,
+ "show status concicely"),
+ OPT_BOOLEAN('z', "null", &null_termination,
+ "terminate entries with NUL"),
{ OPTION_STRING, 'u', "untracked-files", &untracked_files_arg,
"mode",
"show untracked files, optional modes: all, normal, no. (Default: all)",
@@ -910,6 +985,9 @@ int cmd_stat(int argc, const char **argv, const char *prefix)
OPT_END(),
};
+ if (null_termination)
+ shortstatus = 1;
+
wt_status_prepare(&s);
git_config(git_status_config, &s);
argc = parse_options(argc, argv, prefix,
@@ -925,11 +1003,31 @@ int cmd_stat(int argc, const char **argv, const char *prefix)
s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
wt_status_collect(&s);
- if (s.use_color == -1)
- s.use_color = git_use_color_default;
- if (diff_use_color_default == -1)
- diff_use_color_default = git_use_color_default;
- wt_status_print_body(&s);
+ if (shortstatus) {
+ for (i = 0; i < s.change.nr; i++) {
+ struct wt_status_change_data *d;
+ struct string_list_item *it;
+
+ it = &(s.change.items[i]);
+ d = it->util;
+ if (d->stagemask)
+ short_unmerged(null_termination, it, &s);
+ else
+ short_status(null_termination, it, &s);
+ }
+ for (i = 0; i < s.untracked.nr; i++) {
+ struct string_list_item *it;
+
+ it = &(s.untracked.items[i]);
+ short_untracked(null_termination, it, &s);
+ }
+ } else {
+ if (s.use_color == -1)
+ s.use_color = git_use_color_default;
+ if (diff_use_color_default == -1)
+ diff_use_color_default = git_use_color_default;
+ wt_status_print_body(&s);
+ }
return 0;
}
--
1.6.4.224.g3be84
^ permalink raw reply related
* Re: [PATCH v5 0/6] {checkout,reset,stash} --patch
From: Thomas Rast @ 2009-08-15 10:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, git, Sverre Rabbelier, Nanako Shiraishi,
Nicolas Sebrecht, Pierre Habouzit
In-Reply-To: <7v4os9v7al.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> >> reset -p [HEAD] Reset this hunk? (**)
> >> reset -p other Apply this hunk to index? (**)
> >
> > This doesn't make sense to me.
>
> Not to me, either.
>
> Let's say you have modified $path and ran "git add $path" earlier.
>
> "reset -p -- $path" and "reset -p HEAD -- $path" both show what your index
> has relative to the commit you are resetting your index to and offer to
> "Unstage" [*1*]. This is consistent and feels natural.
>
> "reset -p HEAD^ -- $path" however shows the same forward diff (i.e. how
> your index is different compared to the commit HEAD^ you are resetting
> to), but offers to "Apply".
[...]
> Perhaps you meant to show a reverse diff and use the word
> "Apply".
Indeed.
> However, that would break down rather badly when HEAD did not change $path
> since HEAD^. Logically what the "reset -p" would do to $path is the same,
> but the patch shown and the operation offered to the user are opposite.
>
> You could compare HEAD and the commit you are resetting the index to and
> see if the path in question is different between the two commits, and
> switch the direction---if there is no change, you show forward diff and
> offer to "Remove this change out of the index", if there is a change, you
> show reverse diff and offer to "Apply this change to the index". But if
> the difference between HEAD and the commit you are resetting to does not
> overlap with the change you staged to the index earlier from your work
> tree, it is unclear such heuristics would yield a natural feel.
>
> So I actually think you may be better off if you consistently showed a
> forward diff (i.e. what patch would have been applied to the commit in
> question to bring the index into its current shape), and always offer
> "Remove this change out of the index?"
HEAD^ is not special. What do we do if the user resets to something
that is logically further progressed in time, perhaps another branch?
I just have a much better mental model of it as "apply <something> to
index" than if it said: here's some diff between whatever commit you
gave me, and your index; but I'm going to apply it reverse!
(v4 actually did it this way and I found it a bit confusing...)
> The same comment applies to "checkout -p HEAD" vs "checkout -p HEAD^".
> I think the latter shouldn't show a reverse diff and offer "Apply?";
> instead both should consitently show a forward diff (i.e. what patch would
> have been applied to the commit to bring your work tree into its current
> shape), and offer "Remove this change out of the index and the work tree?".
Again (and unlike in the reset case, I can actually see myself doing
this at times) the user could pass in a commit that is logically
newer than HEAD.
> *1* I actually have a slight problem with the use of word "Unstage" in
> this context; "to stage", at least to me, means "adding _from the work
> tree_ to the index", not just "modifying the index" from a random source.
> The command is resetting the index in this case from a tree-ish and there
> is no work tree involved, and the word "stage/unstage" feels out of place.
It's not using "unstage" any more if the commit is not HEAD. If it
is, then we're doing the opposite of 'add -p', so doesn't the term
apply then?
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: [PATCH] git-log: allow --decorate[=short|full]
From: Lars Hjemli @ 2009-08-15 10:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <8c5c35580908150250y62b1042cmf6071016bac98a48@mail.gmail.com>
On Sat, Aug 15, 2009 at 11:50, Lars Hjemli<hjemli@gmail.com> wrote:
> This extension to --decorate makes it possible to generate decorations
> similar to pre-1.6.4 git, which is nice when the output from git-log
> is used by external tools.
BTW: the patch was made on top of current master (b2139dbd) - if
accepted, it might be considered for maint.
--
larsh
^ permalink raw reply
* Re: [PATCH] git submodule summary: add --files option
From: Jens Lehmann @ 2009-08-15 11:40 UTC (permalink / raw)
To: Lars Hjemli; +Cc: Junio C Hamano, git
In-Reply-To: <8c5c35580908150140q1d209664ic5e3816609365e24@mail.gmail.com>
Lars Hjemli schrieb:
> On Fri, Aug 14, 2009 at 21:52, Junio C Hamano<gitster@pobox.com> wrote:
>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>>
>>> git submodule summary is providing similar functionality for submodules as
>>> git diff-index does for a git project (including the meaning of --cached).
>>> But the analogon to git diff-files is missing, so add a --files option to
>>> summarize the differences between the index of the super project and the
>>> last commit checked out in the working tree of the submodule.
>>>
>>> Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
>> Makes sense to me. Comments?
>
> Acked-by: Lars Hjemli <hjemli@gmail.com> with a tiny fixup:
>
> --- a/Documentation/git-submodule.txt
> +++ b/Documentation/git-submodule.txt
> @@ -129,7 +129,7 @@ summary::
> in the submodule between the given super project commit and the
> index or working tree (switched by --cached) are shown. If the option
> --files is given, show the series of commits in the submodule between
> - the index of super project the and the working tree of the submodule
> + the index of the super project and the working tree of the submodule
> (this option doesn't allow to use the --cached option or to provide an
> explicit commit).
Yup, sentence this makes much more sense now ... :-)
Shall i send an updated patch?
Jens
^ permalink raw reply
* [PATCH v5.1 5/6] Implement 'git checkout --patch'
From: Thomas Rast @ 2009-08-15 11:48 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Sverre Rabbelier, Nanako Shiraishi,
Nicolas Sebrecht, Pierre Habouzit
In-Reply-To: <b75f4adebf7f417b8c46746202933e8f7ff80331.1250164190.git.trast@student.ethz.ch>
This introduces a --patch mode for git-checkout. In the index usage
git checkout --patch -- [files...]
it lets the user discard edits from the <files> at the granularity of
hunks (by selecting hunks from 'git diff' and then reverse applying
them to the worktree).
We also accept a revision argument. In the case
git checkout --patch HEAD -- [files...]
we offer hunks from the difference between HEAD and the worktree, and
reverse applies them to both index and worktree, allowing you to
discard staged changes completely. In the non-HEAD usage
git checkout --patch <revision> -- [files...]
it offers hunks from the difference between the worktree and
<revision>. The chosen hunks are then applied to both index and
worktree.
The application to worktree and index is done "atomically" in the
sense that we first check if the patch applies to the index (it should
always apply to the worktree). If it does not, we give the user a
choice to either abort or apply to the worktree anyway.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
I found another mismatch of the -R flags during double checking, as
follows:
diff --git c/git-add--interactive.perl i/git-add--interactive.perl
index 2363a77..21746d5 100755
--- c/git-add--interactive.perl
+++ i/git-add--interactive.perl
@@ -117,7 +117,7 @@
'checkout_head' => {
DIFF => 'diff-index -p',
APPLY => sub { apply_patch_for_checkout_commit '-R', @_ },
- APPLY_CHECK => 'apply',
+ APPLY_CHECK => 'apply -R',
VERB => 'Discard',
TARGET => ' from index and worktree',
PARTICIPLE => 'discarding',
Documentation/git-checkout.txt | 13 +++++-
builtin-checkout.c | 19 +++++++
git-add--interactive.perl | 61 +++++++++++++++++++++++
t/t2015-checkout-patch.sh | 107 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 199 insertions(+), 1 deletions(-)
create mode 100755 t/t2015-checkout-patch.sh
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index ad4b31e..26a5447 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -11,6 +11,7 @@ SYNOPSIS
'git checkout' [-q] [-f] [-m] [<branch>]
'git checkout' [-q] [-f] [-m] [-b <new_branch>] [<start_point>]
'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...
+'git checkout' --patch [<tree-ish>] [--] [<paths>...]
DESCRIPTION
-----------
@@ -25,7 +26,7 @@ use the --track or --no-track options, which will be passed to `git
branch`. As a convenience, --track without `-b` implies branch
creation; see the description of --track below.
-When <paths> are given, this command does *not* switch
+When <paths> or --patch are given, this command does *not* switch
branches. It updates the named paths in the working tree from
the index file, or from a named <tree-ish> (most often a commit). In
this case, the `-b` and `--track` options are meaningless and giving
@@ -113,6 +114,16 @@ the conflicted merge in the specified paths.
"merge" (default) and "diff3" (in addition to what is shown by
"merge" style, shows the original contents).
+-p::
+--patch::
+ Interactively select hunks in the difference between the
+ <tree-ish> (or the index, if unspecified) and the working
+ tree. The chosen hunks are then applied in reverse to the
+ working tree (and if a <tree-ish> was specified, the index).
++
+This means that you can use `git checkout -p` to selectively discard
+edits from your current working tree.
+
<branch>::
Branch to checkout; if it refers to a branch (i.e., a name that,
when prepended with "refs/heads/", is a valid ref), then that
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 8a9a474..8b942ba 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -572,6 +572,13 @@ static int git_checkout_config(const char *var, const char *value, void *cb)
return git_xmerge_config(var, value, cb);
}
+static int interactive_checkout(const char *revision, const char **pathspec,
+ struct checkout_opts *opts)
+{
+ return run_add_interactive(revision, "--patch=checkout", pathspec);
+}
+
+
int cmd_checkout(int argc, const char **argv, const char *prefix)
{
struct checkout_opts opts;
@@ -580,6 +587,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
struct branch_info new;
struct tree *source_tree = NULL;
char *conflict_style = NULL;
+ int patch_mode = 0;
struct option options[] = {
OPT__QUIET(&opts.quiet),
OPT_STRING('b', NULL, &opts.new_branch, "new branch", "branch"),
@@ -594,6 +602,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
OPT_BOOLEAN('m', "merge", &opts.merge, "merge"),
OPT_STRING(0, "conflict", &conflict_style, "style",
"conflict style (merge or diff3)"),
+ OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"),
OPT_END(),
};
int has_dash_dash;
@@ -608,6 +617,10 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, options, checkout_usage,
PARSE_OPT_KEEP_DASHDASH);
+ if (patch_mode && (opts.track > 0 || opts.new_branch
+ || opts.new_branch_log || opts.merge || opts.force))
+ die ("--patch is incompatible with all other options");
+
/* --track without -b should DWIM */
if (0 < opts.track && !opts.new_branch) {
const char *argv0 = argv[0];
@@ -714,6 +727,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
if (!pathspec)
die("invalid path specification");
+ if (patch_mode)
+ return interactive_checkout(new.name, pathspec, &opts);
+
/* Checkout paths */
if (opts.new_branch) {
if (argc == 1) {
@@ -729,6 +745,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
return checkout_paths(source_tree, pathspec, &opts);
}
+ if (patch_mode)
+ return interactive_checkout(new.name, NULL, &opts);
+
if (opts.new_branch) {
struct strbuf buf = STRBUF_INIT;
if (strbuf_check_branch_ref(&buf, opts.new_branch))
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index d14f48c..21746d5 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -75,6 +75,7 @@
my $patch_mode_revision;
sub apply_patch;
+sub apply_patch_for_checkout_commit;
my %patch_modes = (
'stage' => {
@@ -104,6 +105,33 @@
PARTICIPLE => 'applying',
FILTER => 'index-only',
},
+ 'checkout_index' => {
+ DIFF => 'diff-files -p',
+ APPLY => sub { apply_patch 'apply -R', @_; },
+ APPLY_CHECK => 'apply -R',
+ VERB => 'Discard',
+ TARGET => ' from worktree',
+ PARTICIPLE => 'discarding',
+ FILTER => 'file-only',
+ },
+ 'checkout_head' => {
+ DIFF => 'diff-index -p',
+ APPLY => sub { apply_patch_for_checkout_commit '-R', @_ },
+ APPLY_CHECK => 'apply -R',
+ VERB => 'Discard',
+ TARGET => ' from index and worktree',
+ PARTICIPLE => 'discarding',
+ FILTER => undef,
+ },
+ 'checkout_nothead' => {
+ DIFF => 'diff-index -R -p',
+ APPLY => sub { apply_patch_for_checkout_commit '', @_ },
+ APPLY_CHECK => 'apply',
+ VERB => 'Apply',
+ TARGET => ' to index and worktree',
+ PARTICIPLE => 'applying',
+ FILTER => undef,
+ },
);
my %patch_mode_flavour = %{$patch_modes{stage}};
@@ -1069,6 +1097,29 @@
return $ret;
}
+sub apply_patch_for_checkout_commit {
+ my $reverse = shift;
+ my $applies_index = run_git_apply 'apply '.$reverse.' --cached --recount --check', @_;
+ my $applies_worktree = run_git_apply 'apply '.$reverse.' --recount --check', @_;
+
+ if ($applies_worktree && $applies_index) {
+ run_git_apply 'apply '.$reverse.' --cached --recount', @_;
+ run_git_apply 'apply '.$reverse.' --recount', @_;
+ return 1;
+ } elsif (!$applies_index) {
+ print colored $error_color, "The selected hunks do not apply to the index!\n";
+ if (prompt_yesno "Apply them to the worktree anyway? ") {
+ return run_git_apply 'apply '.$reverse.' --recount', @_;
+ } else {
+ print colored $error_color, "Nothing was applied.\n";
+ return 0;
+ }
+ } else {
+ print STDERR @_;
+ return 0;
+ }
+}
+
sub patch_update_cmd {
my @all_mods = list_modified($patch_mode_flavour{FILTER});
my @mods = grep { !($_->{BINARY}) } @all_mods;
@@ -1432,6 +1483,16 @@
'reset_head' : 'reset_nothead');
$arg = shift @ARGV or die "missing --";
}
+ } elsif ($1 eq 'checkout') {
+ $arg = shift @ARGV or die "missing --";
+ if ($arg eq '--') {
+ $patch_mode = 'checkout_index';
+ } else {
+ $patch_mode_revision = $arg;
+ $patch_mode = ($arg eq 'HEAD' ?
+ 'checkout_head' : 'checkout_nothead');
+ $arg = shift @ARGV or die "missing --";
+ }
} elsif ($1 eq 'stage') {
$patch_mode = 'stage';
$arg = shift @ARGV or die "missing --";
diff --git a/t/t2015-checkout-patch.sh b/t/t2015-checkout-patch.sh
new file mode 100755
index 0000000..4d1c2e9
--- /dev/null
+++ b/t/t2015-checkout-patch.sh
@@ -0,0 +1,107 @@
+#!/bin/sh
+
+test_description='git checkout --patch'
+
+. ./lib-patch-mode.sh
+
+test_expect_success 'setup' '
+ mkdir dir &&
+ echo parent > dir/foo &&
+ echo dummy > bar &&
+ git add bar dir/foo &&
+ git commit -m initial &&
+ test_tick &&
+ test_commit second dir/foo head &&
+ set_and_save_state bar bar_work bar_index &&
+ save_head
+'
+
+# note: bar sorts before dir/foo, so the first 'n' is always to skip 'bar'
+
+test_expect_success 'saying "n" does nothing' '
+ set_and_save_state dir/foo work head &&
+ (echo n; echo n) | git checkout -p &&
+ verify_saved_state bar &&
+ verify_saved_state dir/foo
+'
+
+test_expect_success 'git checkout -p' '
+ (echo n; echo y) | git checkout -p &&
+ verify_saved_state bar &&
+ verify_state dir/foo head head
+'
+
+test_expect_success 'git checkout -p with staged changes' '
+ set_state dir/foo work index
+ (echo n; echo y) | git checkout -p &&
+ verify_saved_state bar &&
+ verify_state dir/foo index index
+'
+
+test_expect_success 'git checkout -p HEAD with NO staged changes: abort' '
+ set_and_save_state dir/foo work head &&
+ (echo n; echo y; echo n) | git checkout -p HEAD &&
+ verify_saved_state bar &&
+ verify_saved_state dir/foo
+'
+
+test_expect_success 'git checkout -p HEAD with NO staged changes: apply' '
+ (echo n; echo y; echo y) | git checkout -p HEAD &&
+ verify_saved_state bar &&
+ verify_state dir/foo head head
+'
+
+test_expect_success 'git checkout -p HEAD with change already staged' '
+ set_state dir/foo index index
+ # the third n is to get out in case it mistakenly does not apply
+ (echo n; echo y; echo n) | git checkout -p HEAD &&
+ verify_saved_state bar &&
+ verify_state dir/foo head head
+'
+
+test_expect_success 'git checkout -p HEAD^' '
+ # the third n is to get out in case it mistakenly does not apply
+ (echo n; echo y; echo n) | git checkout -p HEAD^ &&
+ verify_saved_state bar &&
+ verify_state dir/foo parent parent
+'
+
+# The idea in the rest is that bar sorts first, so we always say 'y'
+# first and if the path limiter fails it'll apply to bar instead of
+# dir/foo. There's always an extra 'n' to reject edits to dir/foo in
+# the failure case (and thus get out of the loop).
+
+test_expect_success 'path limiting works: dir' '
+ set_state dir/foo work head &&
+ (echo y; echo n) | git checkout -p dir &&
+ verify_saved_state bar &&
+ verify_state dir/foo head head
+'
+
+test_expect_success 'path limiting works: -- dir' '
+ set_state dir/foo work head &&
+ (echo y; echo n) | git checkout -p -- dir &&
+ verify_saved_state bar &&
+ verify_state dir/foo head head
+'
+
+test_expect_success 'path limiting works: HEAD^ -- dir' '
+ # the third n is to get out in case it mistakenly does not apply
+ (echo y; echo n; echo n) | git checkout -p HEAD^ -- dir &&
+ verify_saved_state bar &&
+ verify_state dir/foo parent parent
+'
+
+test_expect_success 'path limiting works: foo inside dir' '
+ set_state dir/foo work head &&
+ # the third n is to get out in case it mistakenly does not apply
+ (echo y; echo n; echo n) | (cd dir && git checkout -p foo) &&
+ verify_saved_state bar &&
+ verify_state dir/foo head head
+'
+
+test_expect_success 'none of this moved HEAD' '
+ verify_saved_head
+'
+
+test_done
--
1.6.4.287.g3e02d
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox