Re: [RFC/PATCH] shortstatus v1

On Tue, Feb 10, 2009 at 2:44 AM, Junio C Hamano [off-list ref] wrote:

Tuncer Ayaz [off-list ref] writes:

quoted

Adding git 'shortstatus --mini' to PS1 is not noticeable or 1sec
maximum in my tree. As a worst case it takes 10secs in a clone
of WebKit.git.

Junio, if I leave out my --mini experiment would you be interested
in merging shortstatus without any additions except maybe
removing the index_score? Is it useful enough in your eyes? If yes
I will resubmit it and decouple the --mini case completely as
possible future work.

Frankly, I think having to spend one second to add only one or two bits to
PS1 is simply spending one second too much.

ACK. it will get worse with time.

quoted

diff --git a/builtin-commit.c b/builtin-commit.c
index d6a3a62..9267d26 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c

@@ -821,6 +827,88 @@ static int parse_and_validate_options(int argc, const char *argv[],
      return argc;
 }

+int cmd_shortstatus(int argc, const char **argv, const char *prefix)
+{
+     struct wt_status s;
+     int i;
+     int c, a, u;
+
+     c = a = u = 0;
+
+     argc = parse_and_validate_options(argc, argv, builtin_shortstatus_usage, prefix);
+     read_cache();
+     refresh_cache(REFRESH_QUIET);
+     wt_status_prepare(&s);
+     wt_status_collect_changes(&s);
+     if (mini) {
+             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;
+                     switch (d->index_status) {
+                             case DIFF_STATUS_ADDED:
+                                     a = 1;
+                                     break;
+                             case 0:
+                             case DIFF_STATUS_COPIED:
+                             case DIFF_STATUS_DELETED:
+                             case DIFF_STATUS_MODIFIED:
+                             case DIFF_STATUS_RENAMED:
+                             case DIFF_STATUS_TYPE_CHANGED:
+                                     c = 1;
+                                     break;

If you at the end discard information by squashing renamed, copied,
deleted and modified into a single "changed" category, I do not think you
would want wt_status_collect_changes() to spend the cost of rename
detection in the first place.  Sure, you can tell between "git mv old new"
and "git add new", because you won't show "+" for "new" if you run rename
detection, but that is about the only thing I think you are getting.

actually I can leave out all but case 0 to get the current behavior.
I am not sure but (presumably) have the suspicion from what I
have read that these extra cases are irrelevant in this case.
I may err.

Is it worth extra 1 second (or 10 seconds)?

1 second is noticeable and therefore bad but it is a definite
improvement compared to what I had before with
'git status|grep' calls. it is slow for PS1, yes.

What are you really trying to achieve?  Do you want to see if you have any
change to the index since you checked out?  Do you want to further tell
the user that the work tree has more changes that are not staged yet
(which --mini does not seem to do)?

Do you really need more than "diff-index --cached --exit-code" in your
$PS1 code, and so why?  Does the added feature your "shortstatus --mini"
offers over "diff-index --cached --exit-code" justify the latency penalty
to the user?

What I and others need - based on the fact that the PS1
enhancement was inspired by someone else's PS1 - is not
diff-index --cached. It should include changes in the
index plus those not.
The feature is there to display that a repo is dirty and if
possible in an instant way also display that there are not
only untrackeds but also modifications and/or additions not
yet committed with separate symbols (+,*,?).
If this is not currently implementable fast enough let's forget
about it for now and tackle it once the future unfolds and shows
us a better path or someone comes up with a bright idea :).