* Re: [PATCH 04/10] Migrate git-clone to use git-rev-parse --parseopt
From: Pierre Habouzit @ 2007-11-04 14:49 UTC (permalink / raw)
To: gitster; +Cc: git
In-Reply-To: <1194172262-1563-5-git-send-email-madcoder@debian.org>
[-- Attachment #1: Type: text/plain, Size: 406 bytes --]
Note: this patch now conflicts with a recent patch to make git clone
grok `--`. As git rev-parse --parseopt does that as a side effect, you
can force the update to the parseopt version without functionality loss.
Cheers,
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 5/5] pretty describe: add %ds, %dn, %dd placeholders
From: René Scharfe @ 2007-11-04 14:42 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0711041356330.4362@racer.site>
Johannes Schindelin schrieb:
> Hi,
>
> On Sun, 4 Nov 2007, Ren? Scharfe wrote:
>
>> The new placeholders %ds (description string, git-describe style), %dn
>> (the name part) and %dd (the depth part) are added.
>>
>> To avoid imposing the significant cost of running describe_commit() on
>> every format string, even if none of the new placeholders are used, a
>> new function, interp_count(), is introduced. It is a stripped down
>> version of interpolate(), that simply counts the placeholders in a
>> format string. If the describe placeholders are not found, setting up
>> the corresponding replacements is skipped.
>
> The way I read this, those are two really quite independent patches
> squashed into one.
Busted! I didn't want to introduce a performance regression with the
%ds parsing code, but I also didn't want to add a function without
users. Patch 6 was then glued on as an afterthought -- the rest of the
series was ready when I saw Paul's mail.
So, a better way might be to move patch 6 to the head of the series and
introduce interp_count() in it, too.
>> + unsigned long occurs[ARRAY_SIZE(table)];
>
> You do not ever use the counts. So, longs are overkill. Even ints might
> be overkill, but probably the most convenient. I would have gone with
> chars. If I knew how to memset() an array of unsigned:1 entries to all
> zero, I would even have gone with that, but the runtime cost of this is
> probably higher than the chars.
Well, it isn't used in format_commit_message() currently, but it could
be. Multiply the count and and the length of each substitution (minus
the length of the placeholder) and you get the number of bytes you need
to allocate. interpolate() wouldn't need to be called twice anymore.
> But the even more fundamental problem is that you count the needed
> interpolations _every_ single time you output a commit message.
>
> A much better place would be get_commit_format(). Yes that means
> restructuring the code a bit more, but I would say that this definitely
> would help. My preference would even be introducing a new source file for
> the user format handling (commit-format.[ch]).
Counting the interpolations is easier than actually interpolating.
Wherever the code goes, the calls to interpolate() and interp_count()
should stay together.
>> +
>> +/*
>> + * interp_count - count occurences of placeholders
>> + */
>> +void interp_count(unsigned long *result, const char *orig,
>> + const struct interp *interps, int ninterps)
>> +{
>> + const char *src = orig;
>
> You do not ever use orig again. So why not just use that variable instead
> of introducing a new one?
I simply copied interpolate() and then chopped off the parts not needed
for counting, to make it easy to see that this is the smaller brother.
>
>> + const char *name;
>> + unsigned long namelen;
>> + int i;
>> + char c;
>> +
>> + for (i = 0; i < ninterps; i++)
>> + result[i] = 0;
>
> memset()?
In earlier versions there was a memset() there. I replaced it to make
the intent even more clear, but I guess not using memset() is simply
superstition..
>
>> +
>> + while ((c = *src)) {
>> + if (c == '%') {
>> + /* Try to match an interpolation string. */
>> + for (i = 0; i < ninterps; i++) {
>> + name = interps[i].name;
>> + namelen = strlen(name);
>> + if (strncmp(src, name, namelen) == 0)
>
> prefixcmp()?
Copied from interpolate()..
>> + break;
>> + }
>> +
>> + /* Check for valid interpolation. */
>> + if (i < ninterps) {
>
> This is ugly. You already had a successful if() for that case earlier.
Dito..
>
>> + result[i]++;
>> + src += namelen;
>> + continue;
>> + }
>> + }
>> + src++;
>> + }
>> +}
>
> I'd rewrite this whole loop as
>
> while ((c = *(orig++)))
> if (c == '%')
> /* Try to match an interpolation string. */
> for (i = 0; i < ninterps; i++)
> if (prefixcmp(orig, interps[i].name)) {
> result[i] = 1;
> orig += strlen(interps[i].name);
> break;
> }
Cleanups are sure possible, but they should be done on top, and to both
interpolate() and interp_count(). Let's first see how far we get with
dumb code-copying and reusing the result in new ways. :)
Thanks,
René
^ permalink raw reply
* Re: [PATCH 3/5] pretty describe: move library functions to the new file describe.c
From: Johannes Schindelin @ 2007-11-04 14:36 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <472DB199.2040305@lsrfire.ath.cx>
Hi,
On Sun, 4 Nov 2007, Ren? Scharfe wrote:
> Makefile | 2 +-
> builtin-describe.c | 202 ---------------------------------------------------
> cache.h | 5 ++
> describe.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 210 insertions(+), 203 deletions(-)
> create mode 100644 describe.c
Would not "format-patch -C -C" have given a nicer output?
Ciao,
Dscho
^ permalink raw reply
* [PATCH] Remove a couple of duplicated include
From: Marco Costalba @ 2007-11-04 14:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
Signed-off-by: Marco Costalba <mcostalba@gmail.com>
---
compat/inet_ntop.c | 1 -
compat/inet_pton.c | 1 -
2 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/compat/inet_ntop.c b/compat/inet_ntop.c
index 4d7ab9d..f444982 100644
--- a/compat/inet_ntop.c
+++ b/compat/inet_ntop.c
@@ -18,7 +18,6 @@
#include <errno.h>
#include <sys/types.h>
#include <sys/socket.h>
-#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <stdio.h>
diff --git a/compat/inet_pton.c b/compat/inet_pton.c
index 5704e0d..4078fc0 100644
--- a/compat/inet_pton.c
+++ b/compat/inet_pton.c
@@ -18,7 +18,6 @@
#include <errno.h>
#include <sys/types.h>
#include <sys/socket.h>
-#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <stdio.h>
--
1.5.3.5.532.g5c38-dirty
^ permalink raw reply related
* Re: [PATCH 1/5] pretty describe: add name info to struct commit
From: Johannes Schindelin @ 2007-11-04 14:22 UTC (permalink / raw)
To: Alex Riesen; +Cc: René Scharfe, Junio C Hamano, Git Mailing List
In-Reply-To: <20071104140700.GB3078@steel.home>
Hi,
On Sun, 4 Nov 2007, Alex Riesen wrote:
> Ren? Scharfe, Sun, Nov 04, 2007 12:48:22 +0100:
> > diff --git a/commit.h b/commit.h
> > index b661503..80e94b9 100644
> > --- a/commit.h
> > +++ b/commit.h
> > @@ -18,6 +18,9 @@ struct commit {
> > struct commit_list *parents;
> > struct tree *tree;
> > char *buffer;
> > + char *name;
> > + unsigned int name_flags;
> > + char name_prio;
> > };
>
> It increases size of struct commit by ~12 bytes (assuming 4byte
> allignment), and this is a popular structure.
I was just about to say something similar.
But there is a wonderful solution: Have a look at decorate.[ch]. This is
the structure you should use IMHO.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 9/5] Migrate git-checkout.sh to use git-rev-parse --parseopt --keep-dashdash
From: Pierre Habouzit @ 2007-11-04 14:18 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711041343470.4362@racer.site>
[-- Attachment #1: Type: text/plain, Size: 2481 bytes --]
On Sun, Nov 04, 2007 at 01:48:36PM +0000, Johannes Schindelin wrote:
> > Pierre Habouzit <madcoder@debian.org> writes:
> > It takes on the standard input the specification of the options to parse
> > and understand, and echoes on the standard ouput a line suitable for
> > `sh(1)` `eval` to replace the arguments with normalized ones.
>
> Why not go the full nine yards and output something which when eval'ed
> sets the variables correctly (taking the variable names from the option
> names; long name if available, otherwise short one)? It can also set the
> command line arguments to what's left after option parsing, with a "set"
> call.
We could do that, though it's not as great as it looks like at the first
glance. If you want -vvv to work like an accumulator, then you need a
really more complex approach in the C code. To enter the gory details,
git-rev-parse --parseopt uses a callback that deals with options and
their arguments one by one, then appends a delimiter to tell the shell
script that only arguments follow, and then appends the arguments the
option parser left alone.
It does not deal with the semantics that the C has available at all,
it's up to the shell script to decide which is better.
My goal with this is not really to do all the work for the shell script
author, but rather to the user: it's not because a porcelain is new (or
not a builtin yet) that it should have a creepy interface. If it helps
the programmer as a side effect, then it's great, but this series really
is about usability to me.
Of course we can do what you propose, but it will probably be quite
sophisticated and looks to me like an overkill to what shell builtins
really are used for: prototyping a new porcelain until it becomes a new
full blown C-builtin. I do believe in simplicity after all :)
> And to prevent funny games with "PARSEOPT_OPTS=blabla git xyz", why not
> provide a function in git-sh-setup which takes the string as argument, and
> is called _after_ sourcing git-sh-setup?
This one is quite a non issue, it's only used for keep-dashdash, and I
see no other need in the near future. And if the need arise, it'll still
be doable any time. So for now I've taken a non-generic way to do that,
and I believe it's fine as it is.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 5/5] pretty describe: add %ds, %dn, %dd placeholders
From: Johannes Schindelin @ 2007-11-04 14:11 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <472DB1B0.1050904@lsrfire.ath.cx>
Hi,
On Sun, 4 Nov 2007, Ren? Scharfe wrote:
> The new placeholders %ds (description string, git-describe style), %dn
> (the name part) and %dd (the depth part) are added.
>
> To avoid imposing the significant cost of running describe_commit() on
> every format string, even if none of the new placeholders are used, a
> new function, interp_count(), is introduced. It is a stripped down
> version of interpolate(), that simply counts the placeholders in a
> format string. If the describe placeholders are not found, setting up
> the corresponding replacements is skipped.
The way I read this, those are two really quite independent patches
squashed into one.
> + unsigned long occurs[ARRAY_SIZE(table)];
You do not ever use the counts. So, longs are overkill. Even ints might
be overkill, but probably the most convenient. I would have gone with
chars. If I knew how to memset() an array of unsigned:1 entries to all
zero, I would even have gone with that, but the runtime cost of this is
probably higher than the chars.
But the even more fundamental problem is that you count the needed
interpolations _every_ single time you output a commit message.
A much better place would be get_commit_format(). Yes that means
restructuring the code a bit more, but I would say that this definitely
would help. My preference would even be introducing a new source file for
the user format handling (commit-format.[ch]).
> +
> +/*
> + * interp_count - count occurences of placeholders
> + */
> +void interp_count(unsigned long *result, const char *orig,
> + const struct interp *interps, int ninterps)
> +{
> + const char *src = orig;
You do not ever use orig again. So why not just use that variable instead
of introducing a new one?
> + const char *name;
> + unsigned long namelen;
> + int i;
> + char c;
> +
> + for (i = 0; i < ninterps; i++)
> + result[i] = 0;
memset()?
> +
> + while ((c = *src)) {
> + if (c == '%') {
> + /* Try to match an interpolation string. */
> + for (i = 0; i < ninterps; i++) {
> + name = interps[i].name;
> + namelen = strlen(name);
> + if (strncmp(src, name, namelen) == 0)
prefixcmp()?
> + break;
> + }
> +
> + /* Check for valid interpolation. */
> + if (i < ninterps) {
This is ugly. You already had a successful if() for that case earlier.
> + result[i]++;
> + src += namelen;
> + continue;
> + }
> + }
> + src++;
> + }
> +}
I'd rewrite this whole loop as
while ((c = *(orig++)))
if (c == '%')
/* Try to match an interpolation string. */
for (i = 0; i < ninterps; i++)
if (prefixcmp(orig, interps[i].name)) {
result[i] = 1;
orig += strlen(interps[i].name);
break;
}
Ciao,
Dscho
^ permalink raw reply
* [PATCH 7/5] pretty describe: add min_prio parameter to describe_commit()
From: René Scharfe @ 2007-11-04 14:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <472DB1B0.1050904@lsrfire.ath.cx>
If load_commit_names() is called with a certain priority and later with
a higher priority, all the old, low-priority names are still kept and
describe_commit() will happily take them into account. It can thus
report heads, even if the user asked for annotated tags.
In the current code, this can't happen, because builtin-describe.c calls
load_commit_names() only once, and commit.c always asks for the highest
priority (annotated tags). This patch fixes the problem anyway, for the
benefit of future users.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
builtin-describe.c | 6 ++++--
cache.h | 2 +-
commit.c | 2 +-
describe.c | 7 ++++---
4 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/builtin-describe.c b/builtin-describe.c
index fcd93f4..481d92f 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -14,6 +14,7 @@ static int all; /* Default to annotated tags only */
static int tags; /* But allow any tags if --tags is specified */
static int abbrev = DEFAULT_ABBREV;
static int max_candidates = 10;
+static int min_prio;
static void describe(const char *arg, int last_one)
{
@@ -28,7 +29,7 @@ static void describe(const char *arg, int last_one)
if (!cmit)
die("%s is not a valid '%s' object", arg, commit_type);
- name = describe_commit(cmit, max_candidates, debug, &depth);
+ name = describe_commit(cmit, max_candidates, min_prio, debug, &depth);
if (!name)
die("cannot describe '%s'", sha1_to_hex(cmit->object.sha1));
@@ -74,7 +75,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
* If --tags, then any tags are used.
* Otherwise only annotated tags are used.
*/
- load_commit_names(all ? 0 : (tags ? 1 : 2));
+ min_prio = all ? 0 : (tags ? 1 : 2);
+ load_commit_names(min_prio);
if (argc == 0) {
describe("HEAD", 1);
diff --git a/cache.h b/cache.h
index 84423a3..703d6a9 100644
--- a/cache.h
+++ b/cache.h
@@ -605,6 +605,6 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i
/* describe.c */
struct commit;
extern void load_commit_names(int min_prio);
-extern char *describe_commit(struct commit *cmit, int max_candidates, int debug, int *depthp);
+extern char *describe_commit(struct commit *cmit, int max_candidates, int min_prio, int debug, int *depthp);
#endif /* CACHE_H */
diff --git a/commit.c b/commit.c
index 9ff4735..624005b 100644
--- a/commit.c
+++ b/commit.c
@@ -897,7 +897,7 @@ void format_commit_message(struct commit *commit,
const char *abbr;
load_commit_names(2);
- name = describe_commit(commit, 10, 0, &depth);
+ name = describe_commit(commit, 10, 2, 0, &depth);
clear_commit_name_flags(commit);
abbr = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
diff --git a/describe.c b/describe.c
index 18c7abc..b6de4c1 100644
--- a/describe.c
+++ b/describe.c
@@ -102,7 +102,8 @@ static unsigned long finish_depth_computation(
return seen_commits;
}
-char *describe_commit(struct commit *cmit, int max_candidates, int debug, int *depthp)
+char *describe_commit(struct commit *cmit, int max_candidates, int min_prio,
+ int debug, int *depthp)
{
struct commit *gave_up_on = NULL;
struct commit_list *list;
@@ -110,7 +111,7 @@ char *describe_commit(struct commit *cmit, int max_candidates, int debug, int *d
unsigned int match_cnt = 0, annotated_cnt = 0, cur_match;
unsigned long seen_commits = 0;
- if (cmit->name) {
+ if (cmit->name && cmit->name_prio >= min_prio) {
*depthp = 0;
return cmit->name;
}
@@ -130,7 +131,7 @@ char *describe_commit(struct commit *cmit, int max_candidates, int debug, int *d
struct commit *c = pop_commit(&list);
struct commit_list *parents = c->parents;
seen_commits++;
- if (c->name) {
+ if (c->name && c->name_prio >= min_prio) {
if (match_cnt < max_candidates) {
struct possible_tag *t = &all_matches[match_cnt++];
t->name = c->name;
^ permalink raw reply related
* Re: [PATCH 1/5] pretty describe: add name info to struct commit
From: Alex Riesen @ 2007-11-04 14:07 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <472DB186.9030909@lsrfire.ath.cx>
René Scharfe, Sun, Nov 04, 2007 12:48:22 +0100:
> diff --git a/commit.h b/commit.h
> index b661503..80e94b9 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -18,6 +18,9 @@ struct commit {
> struct commit_list *parents;
> struct tree *tree;
> char *buffer;
> + char *name;
> + unsigned int name_flags;
> + char name_prio;
> };
It increases size of struct commit by ~12 bytes (assuming 4byte
allignment), and this is a popular structure. Besides, the three new
fields used by only git-describe, which nobody has in their top-ten
used commands (see "best git practices" thread). If the fields are so
badly needed (and the information can't (really?) be stored somewhere
else), maybe they could be at least compressed when not used:
struct commit_name_info {
unsigned int name_flags;
char name_prio;
char name[FLEX_ARRAY];
};
struct commit {
struct commit_list *parents;
struct tree *tree;
char *buffer;
struct commit_name_info *name_info;
};
BTW, do we still have some bits left in struct object->flags?
To, for example, switch between normal and expanded structure of
second-level object (i.e. commit and commit-with-names).
^ permalink raw reply
* Re: [PATCH] git-fetch: more terse fetch output
From: Pierre Habouzit @ 2007-11-04 14:01 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Jeff King, Nicolas Pitre, Junio C Hamano, git, Shawn O. Pearce
In-Reply-To: <Pine.LNX.4.64.0711041313020.4362@racer.site>
[-- Attachment #1: Type: text/plain, Size: 6548 bytes --]
On Sun, Nov 04, 2007 at 01:13:40PM +0000, Johannes Schindelin wrote:
> Hi,
>
> On Sun, 4 Nov 2007, Jeff King wrote:
>
> > On Sat, Nov 03, 2007 at 01:32:48AM -0400, Nicolas Pitre wrote:
> >
> > > This makes the fetch output much more terse and prettier on a 80 column
> > > display, based on a consensus reached on the mailing list. Here's an
> > > example output:
> >
> > Thank you for this; it was at the end of a very long todo list for me.
> >
> > > Receiving objects: 100% (5439/5439), 1.60 MiB | 636 KiB/s, done.
> > > Resolving deltas: 100% (4604/4604), done.
> > > From git://git.kernel.org/pub/scm/git/git
> > > ! [rejected] html -> origin/html (non fast forward)
> > > 136e631..f45e867 maint -> origin/maint (fast forward)
> > > 9850e2e..44dd7e0 man -> origin/man (fast forward)
> > > 3e4bb08..e3d6d56 master -> origin/master (fast forward)
> > > fa3665c..536f64a next -> origin/next (fast forward)
> > > + 4f6d9d6...768326f pu -> origin/pu (forced update)
> > > * [new branch] todo -> origin/todo
> >
> > One nice thing about this format is that it works equally well for
> > "push" (changing "From" to "To" and reversing the order of the
> > branches). Comments?
>
> I would like that, too.
Seconded.
btw I believe that an even best attempt to align columns could be done
Receiving objects: 100% (5439/5439), 1.60 MiB | 636 KiB/s, done.
Resolving deltas: 100% (4604/4604), done.
From git://git.kernel.org/pub/scm/git/git
! [rejected] html -> origin/html (non fast forward)
136e631..f45e867 maint -> origin/maint (fast forward)
9850e2e..44dd7e0 man -> origin/man (fast forward)
3e4bb08..e3d6d56 master -> origin/master (fast forward)
fa3665c..536f64a next -> origin/next (fast forward)
+ 4f6d9d6...768326f pu -> origin/pu (forced update)
* [new branch] todo -> origin/todo
This is way easier to read (for me at least). Of course, perfect
alignment of the first column needs to know the lengths of refnames
prior to the fetch, which is not the case, and would be an overkill. A
10 char column looks quite ok to me.
I'd also argue that (fast forward) does not need to be mentioned after
the "if things work, say nothing"-unixish-philosophy. those
parenthesized words catch my attention, to read that it was a fast
forward after all. So my even preferred output would be:
Receiving objects: 100% (5439/5439), 1.60 MiB | 636 KiB/s, done.
Resolving deltas: 100% (4604/4604), done.
From git://git.kernel.org/pub/scm/git/git
! [rejected] html -> origin/html (non fast forward)
136e631..f45e867 maint -> origin/maint
9850e2e..44dd7e0 man -> origin/man
3e4bb08..e3d6d56 master -> origin/master
fa3665c..536f64a next -> origin/next
+ 4f6d9d6...768326f pu -> origin/pu (forced update)
* [new branch] todo -> origin/todo
Something like the following should do it.
Author: Pierre Habouzit <madcoder@debian.org>
Date: Sun Nov 4 14:59:28 2007 +0100
builtin-fetch: be even quieter, more column-formatting
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 12b1c4b..92d789f 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -156,6 +156,7 @@ static int s_update_ref(const char *action,
}
#define SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
+#define REFCOL_WIDTH 10
static int update_local_ref(struct ref *ref,
const char *remote,
@@ -185,8 +186,9 @@ static int update_local_ref(struct ref *ref,
if (!hashcmp(ref->old_sha1, ref->new_sha1)) {
if (verbose)
- sprintf(display, "= %-*s %s -> %s", SUMMARY_WIDTH,
- "[up to date]", remote, pretty_ref);
+ sprintf(display, "= %-*s %-*s -> %s", SUMMARY_WIDTH,
+ "[up to date]", REFCOL_WIDTH, remote,
+ pretty_ref);
return 0;
}
@@ -198,15 +200,17 @@ static int update_local_ref(struct ref *ref,
* If this is the head, and it's not okay to update
* the head, and the old value of the head isn't empty...
*/
- sprintf(display, "! %-*s %s -> %s (can't fetch in current branch)",
- SUMMARY_WIDTH, "[rejected]", remote, pretty_ref);
+ sprintf(display, "! %-*s %-*s -> %s (can't fetch in current branch)",
+ SUMMARY_WIDTH, "[rejected]", REFCOL_WIDTH, remote,
+ pretty_ref);
return 1;
}
if (!is_null_sha1(ref->old_sha1) &&
!prefixcmp(ref->name, "refs/tags/")) {
- sprintf(display, "- %-*s %s -> %s",
- SUMMARY_WIDTH, "[tag update]", remote, pretty_ref);
+ sprintf(display, "- %-*s %-*s -> %s",
+ SUMMARY_WIDTH, "[tag update]", REFCOL_WIDTH, remote,
+ pretty_ref);
return s_update_ref("updating tag", ref, 0);
}
@@ -224,8 +228,8 @@ static int update_local_ref(struct ref *ref,
what = "[new branch]";
}
- sprintf(display, "* %-*s %s -> %s",
- SUMMARY_WIDTH, what, remote, pretty_ref);
+ sprintf(display, "* %-*s %-*s -> %s", SUMMARY_WIDTH, what,
+ REFCOL_WIDTH, remote, pretty_ref);
return s_update_ref(msg, ref, 0);
}
@@ -234,20 +238,21 @@ static int update_local_ref(struct ref *ref,
strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
strcat(quickref, "..");
strcat(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
- sprintf(display, " %-*s %s -> %s (fast forward)",
- SUMMARY_WIDTH, quickref, remote, pretty_ref);
+ sprintf(display, " %-*s %-*s -> %s", SUMMARY_WIDTH, quickref,
+ REFCOL_WIDTH, remote, pretty_ref);
return s_update_ref("fast forward", ref, 1);
} else if (force || ref->force) {
char quickref[84];
strcpy(quickref, find_unique_abbrev(current->object.sha1, DEFAULT_ABBREV));
strcat(quickref, "...");
strcat(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));
- sprintf(display, "+ %-*s %s -> %s (forced update)",
- SUMMARY_WIDTH, quickref, remote, pretty_ref);
+ sprintf(display, "+ %-*s %-*s -> %s (forced update)",
+ SUMMARY_WIDTH, quickref, REFCOL_WIDTH, remote, pretty_ref);
return s_update_ref("forced-update", ref, 1);
} else {
- sprintf(display, "! %-*s %s -> %s (non fast forward)",
- SUMMARY_WIDTH, "[rejected]", remote, pretty_ref);
+ sprintf(display, "! %-*s %-*s -> %s (non fast forward)",
+ SUMMARY_WIDTH, "[rejected]", REFCOL_WIDTH, remote,
+ pretty_ref);
return 1;
}
}
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply related
* Re: Why is --pretty=format: so slow?
From: René Scharfe @ 2007-11-04 13:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Paul Mackerras, git, Johannes Schindelin
In-Reply-To: <7vhck2s02z.fsf@gitster.siamese.dyndns.org>
Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
>> Incidentally, I'm finishing a patch series to add git-describe
>> style placeholders. It needs such an optimization even more badly
>> (and has it). Speeding up other slow placeholder expansions falls
>> out quite naturally. Please wait just a few minutes..
>
> Thanks, and take your time, as I am just going to bed, dreaming for a
> perfect series from one of the few people on this list whose patches
> I do not have to read to reject crap, but I do read to get impressed
> and admire ;-)
*blushes*
Well, it's certainly not perfect -- I just noticed I've sent the patches
to your old address. Would you like me to resend them?
And, of course, I found a conceptual bug (which is impossible to trigger
with the current code, fortunately), while looking over the changes a
last time. *sigh*. I'll send a fix right away.
René
^ permalink raw reply
* Re: [PATCH] user-manual: add advanced topic "bisecting merges"
From: Benoit SIGOURE @ 2007-11-04 13:50 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: git
In-Reply-To: <11941677732664-git-send-email-prohaska@zib.de>
[-- Attachment #1: Type: text/plain, Size: 1257 bytes --]
On Nov 4, 2007, at 10:16 AM, Steffen Prohaska wrote:
> diff --git a/Documentation/user-manual.txt b/Documentation/user-
> manual.txt
> index d99adc6..480e7c1 100644
> --- a/Documentation/user-manual.txt
> +++ b/Documentation/user-manual.txt
> @@ -934,6 +934,95 @@ Figuring out why this works is left as an
> exercise to the (advanced)
[...]
> +
> +Imagine this history.
> +
> +................................................
> + ---Z---o---X---...---o---A---C---D
> + \ /
> + o---o---Y---...---o---B
> +................................................
> +
I don't know how you chose these letters, but I don't find them
particularly intuitive to remember.
................................................
---B---o---X---...---o---Y---M---H
\ /
o---o---T---...---o---Z
................................................
would be better (IMO): B stands for "Branch point"
M stands for "Merge"
H stands for "HEAD"
T stands for "Topic"
Otherwise thanks for documenting this, the patch looks fine to me.
--
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]
^ permalink raw reply
* Re: [PATCH 9/5] Migrate git-checkout.sh to use git-rev-parse --parseopt --keep-dashdash
From: Johannes Schindelin @ 2007-11-04 13:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pierre Habouzit, git
In-Reply-To: <7v7ikytpz0.fsf@gitster.siamese.dyndns.org>
Hi,
On Sun, 4 Nov 2007, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
>
> > Also fix some space versus tabs issues.
> > ---
> > git-checkout.sh | 99 +++++++++++++++++++++++++++----------------------------
> > 1 files changed, 49 insertions(+), 50 deletions(-)
> >
> > diff --git a/git-checkout.sh b/git-checkout.sh
> > index 8993920..5424745 100755
> > --- a/git-checkout.sh
> > +++ b/git-checkout.sh
> > @@ -1,6 +1,16 @@
> > #!/bin/sh
> >
> > -USAGE='[-q] [-f] [-b <new_branch>] [-m] [<branch>] [<paths>...]'
> > +PARSEOPT_OPTS=--keep-dashdash
> > +OPTIONS_SPEC="\
> > +git-branch [options] [<branch>] [<paths>...]
> > +--
> > +b= create a new branch started at <branch>
> > +l create the new branchs reflog
> > +track tells if the new branch should track the remote branch
> > +f proceed even if the index or working tree is not HEAD
> > +m performa three-way merge on local modifications if needed
> > +q,quiet be quiet
> > +"
>
> Ok, so this is how PARSEOPT_OPTS gets used.
I also read in the docs:
> It takes on the standard input the specification of the options to parse
> and understand, and echoes on the standard ouput a line suitable for
> `sh(1)` `eval` to replace the arguments with normalized ones.
Why not go the full nine yards and output something which when eval'ed
sets the variables correctly (taking the variable names from the option
names; long name if available, otherwise short one)? It can also set the
command line arguments to what's left after option parsing, with a "set"
call.
And to prevent funny games with "PARSEOPT_OPTS=blabla git xyz", why not
provide a function in git-sh-setup which takes the string as argument, and
is called _after_ sourcing git-sh-setup?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] fix display overlap between remote and local progress
From: Johannes Schindelin @ 2007-11-04 13:33 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.0.9999.0711032328490.21255@xanadu.home>
Hi,
On Sun, 4 Nov 2007, Nicolas Pitre wrote:
> +#define SUFFIX "\e[K" /* change to " " if ANSI sequences don't work */
I am almost certain (without even testing) that cmd.exe has problems with
that. It does not even understand colorisation.
My vote is to let it be for the moment, and us msysGit people will
probably add something like NO_ANSI_TERM=YesPlease later.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] Make git-mailsplit strip whitespace from the start of the mailbox file.
From: Simon Sasburg @ 2007-11-04 13:32 UTC (permalink / raw)
To: gitster; +Cc: git, Simon Sasburg
In-Reply-To: <7v8x5h58qj.fsf@gitster.siamese.dyndns.org>
This will allow it to handle the files gotten through gmail's web interface via its 'Show original' option.
These files contain the mail headers and the mail body, but start with some whitespace.
Now you can give these files to git-am without having to remove the whitespace yourself.
Signed-off-by: Simon Sasburg <Simon.Sasburg@gmail.com>
---
On Nov 2, 2007 9:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I wonder why this is not using isspace(peek).
Fixed.
On Nov 1, 2007 11:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ah, I meant "git-mailsplit", which is the command internally run
> by "git-am" to preprocess the file and to split it into
> individual mail pieces to be fed to "git-mailinfo".
>
> That may suggest the change is better done in git-mailsplit not
> git-mailinfo.
The files from gmail only contain 1 mail per file, but having git-mailspit
massage these into a proper file that git-mailinfo can parse seems like
a sane solution to me.
builtin-mailsplit.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
index 43fc373..74b0470 100644
--- a/builtin-mailsplit.c
+++ b/builtin-mailsplit.c
@@ -164,6 +164,7 @@ static int split_mbox(const char *file, const char *dir, int allow_bare,
{
char name[PATH_MAX];
int ret = -1;
+ int peek;
FILE *f = !strcmp(file, "-") ? stdin : fopen(file, "r");
int file_done = 0;
@@ -173,6 +174,11 @@ static int split_mbox(const char *file, const char *dir, int allow_bare,
goto out;
}
+ do {
+ peek = fgetc(f);
+ } while (isspace(peek));
+ ungetc(peek, f);
+
if (fgets(buf, sizeof(buf), f) == NULL) {
/* empty stdin is OK */
if (f != stdin) {
--
1.5.3.4.504.gdf75-dirty
^ permalink raw reply related
* Re: [PATCH] status&commit: Teach them to show commits of modified submodules.
From: Yin Ping @ 2007-11-04 13:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmytus0ff.fsf@gitster.siamese.dyndns.org>
On 11/4/07, Junio C Hamano <gitster@pobox.com> wrote:
> "Yin Ping" <pkufranky@gmail.com> writes:
>
> > In both case, i think the user should be notified about the inconsistence.
> > My patch given in the first letter handles this by two warning messages as
> > follows (where $name is module name)
> >
> > + cd $name >&/dev/null || { echo " Warning: fail to
> > chdir to $name" && exit; }
>
> In that situation, all he needs to know, with respect to the
> submodule, is that the submodule has been updated since his HEAD
> (and that is given by the runstatus output). He does not _care_
> about what the individual commits in the submodule were. It is
> not an error that the information from the submodule cannot be
> shown to him. He _chose_ to ignore the details of that
> submodule by not checking it out to begin with.
>
> Something like this, to be totally quiet, would be more
> appropriate.
>
> for name in $modules
> do
> (
> ... do the range, indexone, headone stuff
> cd "$name" 2>/dev/null || exit 0
> echo "* $name $headone...$indexone:"
> ... whatever log you show
> ) | sed ...
> done
>
Your point is in some cases people think the warning messages are
annoying because they don't care the change details for submodules.
However, in some cases these messages are helpful. And a third kind of
cases is that people care about only part of all submodules.
So, maybe some an switch can be used to turn this on or off (default
off)? For example
# only sm1 and sm2 are cared
git commit --submodule=sm1,sm2
# all submodules are cared
git commit --submodule='*'
> By the way, I do not know about the quoting issues with $modules
> variable in the above illustration, as I am not (yet) discussing
> about the implementation level of details.
>
--
franky
^ permalink raw reply
* Re: [PATCH] git-fetch: more terse fetch output
From: Johannes Schindelin @ 2007-11-04 13:13 UTC (permalink / raw)
To: Jeff King; +Cc: Nicolas Pitre, Junio C Hamano, git, Shawn O. Pearce
In-Reply-To: <20071104045800.GB12359@segfault.peff.net>
Hi,
On Sun, 4 Nov 2007, Jeff King wrote:
> On Sat, Nov 03, 2007 at 01:32:48AM -0400, Nicolas Pitre wrote:
>
> > This makes the fetch output much more terse and prettier on a 80 column
> > display, based on a consensus reached on the mailing list. Here's an
> > example output:
>
> Thank you for this; it was at the end of a very long todo list for me.
>
> > Receiving objects: 100% (5439/5439), 1.60 MiB | 636 KiB/s, done.
> > Resolving deltas: 100% (4604/4604), done.
> > From git://git.kernel.org/pub/scm/git/git
> > ! [rejected] html -> origin/html (non fast forward)
> > 136e631..f45e867 maint -> origin/maint (fast forward)
> > 9850e2e..44dd7e0 man -> origin/man (fast forward)
> > 3e4bb08..e3d6d56 master -> origin/master (fast forward)
> > fa3665c..536f64a next -> origin/next (fast forward)
> > + 4f6d9d6...768326f pu -> origin/pu (forced update)
> > * [new branch] todo -> origin/todo
>
> One nice thing about this format is that it works equally well for
> "push" (changing "From" to "To" and reversing the order of the
> branches). Comments?
I would like that, too.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] RelNotes-1.5.3.5: fix another typo
From: David D Kilzer @ 2007-11-04 12:45 UTC (permalink / raw)
To: git; +Cc: David D Kilzer
In-Reply-To: <1194175990-76335-1-git-send-email-ddkilzer@kilzer.net>
Signed-off-by: David D Kilzer <ddkilzer@kilzer.net>
---
Documentation/RelNotes-1.5.3.5.txt | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/Documentation/RelNotes-1.5.3.5.txt b/Documentation/RelNotes-1.5.3.5.txt
index f99a2cd..7ff1d5d 100644
--- a/Documentation/RelNotes-1.5.3.5.txt
+++ b/Documentation/RelNotes-1.5.3.5.txt
@@ -90,5 +90,5 @@ Fixes since v1.5.3.4
* "git-send-pack $remote frotz" segfaulted when there is nothing
named 'frotz' on the local end.
- * "git-rebase -interactive" did not handle its "--strategy" option
+ * "git-rebase --interactive" did not handle its "--strategy" option
properly.
--
1.5.3.5
^ permalink raw reply related
* Re: [PATCH] RelNotes-1.5.3.5: fix typos
From: David D. Kilzer @ 2007-11-04 12:35 UTC (permalink / raw)
To: git; +Cc: David D Kilzer
In-Reply-To: <1194175990-76335-1-git-send-email-ddkilzer@kilzer.net>
David D Kilzer <ddkilzer@kilzer.net> wrote:
> On Sat, 3 Nov 2007 at 06:56:36 -0700, David D. Kilzer wrote:
> > Documentation/RelNotes-1.5.3.5.txt | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
>
> Oops. Found another typo.
>
> Documentation/RelNotes-1.5.3.5.txt | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
Please ignore this patch; didn't realize the other one had been applied. Will
resend a new patch.
Dave
^ permalink raw reply
* Re: [RFC PATCH] Make gitk use --early-output
From: Marco Costalba @ 2007-11-04 11:57 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Linus Torvalds, git
In-Reply-To: <18221.42793.38389.359621@cargo.ozlabs.ibm.com>
On 11/4/07, Paul Mackerras <paulus@samba.org> wrote:
>
> So yes, --early-output does imply --topo-order.
>
Thanks, I should have checked myself.
> > P.S: Why did you choose not let git log (i.e. Linus) to handle the
> > default number of commits?
> >
> > "--early-output=50" instead of just "--early-output"
>
> Because I was thinking of adding a control in the edit/preferences
> window for it later on.
>
I see. Perhaps this default number could become obsolete very quickly
if Linus implements what he has suggested in a similar thread:
"One other thing I was thinking of was also to perhaps allow multiple
partial early-output things, in case we get just 5 commits in the first
0.1 seconds, then 50 in the first second, and 200 after 2 seconds.. I can
well imagine getting the full list taking a long time over a network
filesystem (somebody mentioned samba), and maybe having just a single
trigger is too inflexible."
One thing I see playing with this new --early-output feature in qgit
is that for small /warm cache repos the list of revisions is already
the final one, i.e. the line
"Final output"
appears as the first (and useless in this case) line of the git-log
output stream.
If my proposal to teach git-log to check the final output revisions
against the already outputted one is accepted then the handling of the
above case would come free.
The proposal is that in case early-output has already streamed out 'n'
revisions, when the final ones are ready git-log checks the firsts 'n'
final output revisions and if they exactly match with the already
outputted ones then "Final output" line is skipped and final output
stream starts directly from revisions 'n+1'.
Given the statistically very low number of out of order revisions in
big repos the above could end up being the common case.
Marco
^ permalink raw reply
* [PATCH 6/5] pretty describe: avoid calling find_unique_abbrev() if not needed
From: René Scharfe @ 2007-11-04 11:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Paul Mackerras, Johannes Schindelin
As suggested by Junio, --pretty=format can be sped up by avoiding to
call find_unique_abbrev() if it's not needed. This is quite easy
after the "pretty describe" series, as interp_count() exists and has
already been called to avoid the (even bigger) overhead of the
describe placeholders.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
commit.c | 35 +++++++++++++++++++++--------------
1 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/commit.c b/commit.c
index 06d5cec..9ff4735 100644
--- a/commit.c
+++ b/commit.c
@@ -851,13 +851,17 @@ void format_commit_message(struct commit *commit,
if (!commit->object.parsed)
parse_object(commit->object.sha1);
interp_set_entry(table, IHASH, sha1_to_hex(commit->object.sha1));
- interp_set_entry(table, IHASH_ABBREV,
- find_unique_abbrev(commit->object.sha1,
- DEFAULT_ABBREV));
+ if (occurs[IHASH_ABBREV]) {
+ interp_set_entry(table, IHASH_ABBREV,
+ find_unique_abbrev(commit->object.sha1,
+ DEFAULT_ABBREV));
+ }
interp_set_entry(table, ITREE, sha1_to_hex(commit->tree->object.sha1));
- interp_set_entry(table, ITREE_ABBREV,
- find_unique_abbrev(commit->tree->object.sha1,
- DEFAULT_ABBREV));
+ if (occurs[ITREE_ABBREV]) {
+ interp_set_entry(table, ITREE_ABBREV,
+ find_unique_abbrev(commit->tree->object.sha1,
+ DEFAULT_ABBREV));
+ }
interp_set_entry(table, ILEFT_RIGHT,
(commit->object.flags & BOUNDARY)
? "-"
@@ -873,14 +877,17 @@ void format_commit_message(struct commit *commit,
sha1_to_hex(p->item->object.sha1));
interp_set_entry(table, IPARENTS, parents + 1);
- parents[1] = 0;
- for (i = 0, p = commit->parents;
- p && i < sizeof(parents) - 1;
- p = p->next)
- i += snprintf(parents + i, sizeof(parents) - i - 1, " %s",
- find_unique_abbrev(p->item->object.sha1,
- DEFAULT_ABBREV));
- interp_set_entry(table, IPARENTS_ABBREV, parents + 1);
+ if (occurs[IPARENTS_ABBREV]) {
+ parents[1] = 0;
+ for (i = 0, p = commit->parents;
+ p && i < sizeof(parents) - 1;
+ p = p->next)
+ i += snprintf(parents + i, sizeof(parents) - i - 1,
+ " %s",
+ find_unique_abbrev(p->item->object.sha1,
+ DEFAULT_ABBREV));
+ interp_set_entry(table, IPARENTS_ABBREV, parents + 1);
+ }
if (occurs[IDESC] || occurs[IDESC_DEPTH] || occurs[IDESC_NAME]) {
struct strbuf desc;
--
1.5.3.5.529.ge3d6d
^ permalink raw reply related
* [PATCH 5/5] pretty describe: add %ds, %dn, %dd placeholders
From: René Scharfe @ 2007-11-04 11:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
The new placeholders %ds (description string, git-describe style), %dn
(the name part) and %dd (the depth part) are added.
To avoid imposing the significant cost of running describe_commit() on
every format string, even if none of the new placeholders are used, a
new function, interp_count(), is introduced. It is a stripped down
version of interpolate(), that simply counts the placeholders in a
format string. If the describe placeholders are not found, setting up
the corresponding replacements is skipped.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
Documentation/pretty-formats.txt | 3 +++
commit.c | 38 ++++++++++++++++++++++++++++++++++++++
interpolate.c | 36 ++++++++++++++++++++++++++++++++++++
interpolate.h | 2 ++
t/t6120-describe.sh | 22 ++++++++++++++++++++++
5 files changed, 101 insertions(+), 0 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 0193c3c..ec86415 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -100,6 +100,9 @@ The placeholders are:
- '%t': abbreviated tree hash
- '%P': parent hashes
- '%p': abbreviated parent hashes
+- '%ds': description
+- '%dn': tag name part of the description
+- '%dd': depth part of the description
- '%an': author name
- '%ae': author email
- '%ad': author date
diff --git a/commit.c b/commit.c
index 2e52a2f..06d5cec 100644
--- a/commit.c
+++ b/commit.c
@@ -781,6 +781,9 @@ void format_commit_message(struct commit *commit,
{ "%t" }, /* abbreviated tree hash */
{ "%P" }, /* parent hashes */
{ "%p" }, /* abbreviated parent hashes */
+ { "%ds" }, /* description */
+ { "%dn" }, /* tag name part of the description */
+ { "%dd" }, /* depth part of the description */
{ "%an" }, /* author name */
{ "%ae" }, /* author email */
{ "%ad" }, /* author date */
@@ -809,6 +812,7 @@ void format_commit_message(struct commit *commit,
IHASH = 0, IHASH_ABBREV,
ITREE, ITREE_ABBREV,
IPARENTS, IPARENTS_ABBREV,
+ IDESC, IDESC_NAME, IDESC_DEPTH,
IAUTHOR_NAME, IAUTHOR_EMAIL,
IAUTHOR_DATE, IAUTHOR_DATE_RFC2822, IAUTHOR_DATE_RELATIVE,
IAUTHOR_TIMESTAMP, IAUTHOR_ISO8601,
@@ -829,10 +833,13 @@ void format_commit_message(struct commit *commit,
int i;
enum { HEADER, SUBJECT, BODY } state;
const char *msg = commit->buffer;
+ unsigned long occurs[ARRAY_SIZE(table)];
if (ILEFT_RIGHT + 1 != ARRAY_SIZE(table))
die("invalid interp table!");
+ interp_count(occurs, format, table, ARRAY_SIZE(table));
+
/* these are independent of the commit */
interp_set_entry(table, IRED, "\033[31m");
interp_set_entry(table, IGREEN, "\033[32m");
@@ -875,6 +882,37 @@ void format_commit_message(struct commit *commit,
DEFAULT_ABBREV));
interp_set_entry(table, IPARENTS_ABBREV, parents + 1);
+ if (occurs[IDESC] || occurs[IDESC_DEPTH] || occurs[IDESC_NAME]) {
+ struct strbuf desc;
+ char *name;
+ int depth = 0;
+ char *depthstr;
+ const char *abbr;
+
+ load_commit_names(2);
+ name = describe_commit(commit, 10, 0, &depth);
+ clear_commit_name_flags(commit);
+ abbr = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV);
+
+ strbuf_init(&desc, 32);
+ strbuf_addstr(&desc, name ? name + 5 : abbr);
+ interp_set_entry(table, IDESC_NAME, desc.buf);
+ if (name) {
+ if (depth) {
+ strbuf_addch(&desc, '-');
+ depthstr = desc.buf + desc.len;
+ strbuf_addf(&desc, "%d", depth);
+ interp_set_entry(table, IDESC_DEPTH, depthstr);
+ strbuf_addstr(&desc, "-g");
+ strbuf_addstr(&desc, abbr);
+ } else {
+ interp_set_entry(table, IDESC_DEPTH, "0");
+ }
+ }
+ interp_set_entry(table, IDESC, desc.buf);
+ strbuf_release(&desc);
+ }
+
for (i = 0, state = HEADER; msg[i] && state < BODY; i++) {
int eol;
for (eol = i; msg[eol] && msg[eol] != '\n'; eol++)
diff --git a/interpolate.c b/interpolate.c
index 6ef53f2..8f51649 100644
--- a/interpolate.c
+++ b/interpolate.c
@@ -102,3 +102,39 @@ unsigned long interpolate(char *result, unsigned long reslen,
*dest = '\0';
return newlen;
}
+
+/*
+ * interp_count - count occurences of placeholders
+ */
+void interp_count(unsigned long *result, const char *orig,
+ const struct interp *interps, int ninterps)
+{
+ const char *src = orig;
+ const char *name;
+ unsigned long namelen;
+ int i;
+ char c;
+
+ for (i = 0; i < ninterps; i++)
+ result[i] = 0;
+
+ while ((c = *src)) {
+ if (c == '%') {
+ /* Try to match an interpolation string. */
+ for (i = 0; i < ninterps; i++) {
+ name = interps[i].name;
+ namelen = strlen(name);
+ if (strncmp(src, name, namelen) == 0)
+ break;
+ }
+
+ /* Check for valid interpolation. */
+ if (i < ninterps) {
+ result[i]++;
+ src += namelen;
+ continue;
+ }
+ }
+ src++;
+ }
+}
diff --git a/interpolate.h b/interpolate.h
index 77407e6..c41ea18 100644
--- a/interpolate.h
+++ b/interpolate.h
@@ -22,5 +22,7 @@ extern void interp_clear_table(struct interp *table, int ninterps);
extern unsigned long interpolate(char *result, unsigned long reslen,
const char *orig,
const struct interp *interps, int ninterps);
+extern void interp_count(unsigned long *result, const char *orig,
+ const struct interp *interps, int ninterps);
#endif /* INTERPOLATE_H */
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index ae8ee11..3be43c4 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -23,6 +23,28 @@ check_describe () {
false ;;
esac
'
+
+ # %ds, %dn and %dd don't work with --tags or --all
+ case "$@" in
+ *--tags*|*--all*) return ;;
+ esac
+
+ R=$(git log -n 1 --pretty=format:%ds "$@") &&
+ test_expect_success "log --pretty=format:%ds $*" '
+ case "$R" in
+ $expect) echo happy ;;
+ *) echo "Oops - $R is not $expect";
+ false ;;
+ esac
+ '
+ R=$(git log -n 1 --pretty=format:%dn-%dd-g%h "$@" | sed 's/-0-g.*//') &&
+ test_expect_success "log --pretty=format:%dn-%dd-g%h $*" '
+ case "$R" in
+ $expect) echo happy ;;
+ *) echo "Oops - $R is not $expect";
+ false ;;
+ esac
+ '
}
test_expect_success setup '
--
1.5.3.5.529.ge3d6d
^ permalink raw reply related
* [PATCH 4/5] pretty describe: de-const'ify struct commit arg of format_commit_message()
From: René Scharfe @ 2007-11-04 11:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
As a preparation to use describe_commit() in format_commit_message(),
the const attribute of the struct commit parameter must go, as
describe_commit() needs to change the name related members in order
to do its work.
This change requires changes in other places, too -- those that call
format_commit_message() with a const struct commit, and the places
calling those places.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
archive-tar.c | 2 +-
archive-zip.c | 2 +-
archive.h | 4 ++--
builtin-archive.c | 8 ++++----
commit.c | 4 ++--
commit.h | 4 ++--
6 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index e1bced5..792462c 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -16,7 +16,7 @@ static unsigned long offset;
static time_t archive_time;
static int tar_umask = 002;
static int verbose;
-static const struct commit *commit;
+static struct commit *commit;
/* writes out the whole block, but only if it is full */
static void write_if_needed(void)
diff --git a/archive-zip.c b/archive-zip.c
index 74e30f6..ed0918e 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -12,7 +12,7 @@
static int verbose;
static int zip_date;
static int zip_time;
-static const struct commit *commit;
+static struct commit *commit;
static unsigned char *zip_dir;
static unsigned int zip_dir_size;
diff --git a/archive.h b/archive.h
index 5791e65..7fb69c4 100644
--- a/archive.h
+++ b/archive.h
@@ -8,7 +8,7 @@ struct archiver_args {
const char *base;
struct tree *tree;
const unsigned char *commit_sha1;
- const struct commit *commit;
+ struct commit *commit;
time_t time;
const char **pathspec;
unsigned int verbose : 1;
@@ -43,6 +43,6 @@ extern int write_tar_archive(struct archiver_args *);
extern int write_zip_archive(struct archiver_args *);
extern void *parse_extra_zip_args(int argc, const char **argv);
-extern void *sha1_file_to_archive(const char *path, const unsigned char *sha1, unsigned int mode, enum object_type *type, unsigned long *size, const struct commit *commit);
+extern void *sha1_file_to_archive(const char *path, const unsigned char *sha1, unsigned int mode, enum object_type *type, unsigned long *size, struct commit *commit);
#endif /* ARCHIVE_H */
diff --git a/builtin-archive.c b/builtin-archive.c
index 14a1b30..759f265 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -79,7 +79,7 @@ static int run_remote_archiver(const char *remote, int argc,
return !!rv;
}
-static void format_subst(const struct commit *commit,
+static void format_subst(struct commit *commit,
const char *src, size_t len,
struct strbuf *buf)
{
@@ -115,7 +115,7 @@ static void format_subst(const struct commit *commit,
static int convert_to_archive(const char *path,
const void *src, size_t len,
struct strbuf *buf,
- const struct commit *commit)
+ struct commit *commit)
{
static struct git_attr *attr_export_subst;
struct git_attr_check check[1];
@@ -139,7 +139,7 @@ static int convert_to_archive(const char *path,
void *sha1_file_to_archive(const char *path, const unsigned char *sha1,
unsigned int mode, enum object_type *type,
unsigned long *sizep,
- const struct commit *commit)
+ struct commit *commit)
{
void *buffer;
@@ -188,7 +188,7 @@ void parse_treeish_arg(const char **argv, struct archiver_args *ar_args,
const unsigned char *commit_sha1;
time_t archive_time;
struct tree *tree;
- const struct commit *commit;
+ struct commit *commit;
unsigned char sha1[20];
if (get_sha1(name, sha1))
diff --git a/commit.c b/commit.c
index 24b7268..2e52a2f 100644
--- a/commit.c
+++ b/commit.c
@@ -771,7 +771,7 @@ static void fill_person(struct interp *table, const char *msg, int len)
interp_set_entry(table, 6, show_date(date, tz, DATE_ISO8601));
}
-void format_commit_message(const struct commit *commit,
+void format_commit_message(struct commit *commit,
const void *format, struct strbuf *sb)
{
struct interp table[] = {
@@ -1064,7 +1064,7 @@ static void pp_remainder(enum cmit_fmt fmt,
}
}
-void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
+void pretty_print_commit(enum cmit_fmt fmt, struct commit *commit,
struct strbuf *sb, int abbrev,
const char *subject, const char *after_subject,
enum date_mode dmode)
diff --git a/commit.h b/commit.h
index 80e94b9..d74859e 100644
--- a/commit.h
+++ b/commit.h
@@ -65,9 +65,9 @@ enum cmit_fmt {
};
extern enum cmit_fmt get_commit_format(const char *arg);
-extern void format_commit_message(const struct commit *commit,
+extern void format_commit_message(struct commit *commit,
const void *format, struct strbuf *sb);
-extern void pretty_print_commit(enum cmit_fmt fmt, const struct commit*,
+extern void pretty_print_commit(enum cmit_fmt fmt, struct commit*,
struct strbuf *,
int abbrev, const char *subject,
const char *after_subject, enum date_mode);
--
1.5.3.5.529.ge3d6d
^ permalink raw reply related
* [PATCH 3/5] pretty describe: move library functions to the new file describe.c
From: René Scharfe @ 2007-11-04 11:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
load_commit_names(), describe_commit() et. al. are moved out to their
own file and properly exported. There are no code changes inside the
functions.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
Makefile | 2 +-
builtin-describe.c | 202 ---------------------------------------------------
cache.h | 5 ++
describe.c | 204 ++++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 210 insertions(+), 203 deletions(-)
create mode 100644 describe.c
diff --git a/Makefile b/Makefile
index 042f79e..afdd847 100644
--- a/Makefile
+++ b/Makefile
@@ -312,7 +312,7 @@ LIB_OBJS = \
alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
- transport.o bundle.o walker.o parse-options.o
+ transport.o bundle.o walker.o parse-options.o describe.o
BUILTIN_OBJS = \
builtin-add.o \
diff --git a/builtin-describe.c b/builtin-describe.c
index e68a3d0..fcd93f4 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -1,14 +1,9 @@
#include "cache.h"
#include "commit.h"
-#include "tag.h"
-#include "refs.h"
#include "builtin.h"
#include "exec_cmd.h"
#include "parse-options.h"
-#define SEEN (1u<<0)
-#define MAX_TAGS (FLAG_BITS - 1)
-
static const char * const describe_usage[] = {
"git-describe [options] <committish>*",
NULL
@@ -20,203 +15,6 @@ static int tags; /* But allow any tags if --tags is specified */
static int abbrev = DEFAULT_ABBREV;
static int max_candidates = 10;
-static const char *prio_names[] = {
- "head", "lightweight", "annotated",
-};
-
-static int get_name(const char *path, const unsigned char *sha1, int flag, void *cb_data)
-{
- struct commit *commit = lookup_commit_reference_gently(sha1, 1);
- struct object *object;
- int min_prio = *((int *)cb_data);
- int prio;
-
- if (!commit)
- return 0;
- object = parse_object(sha1);
-
- if (!prefixcmp(path, "refs/tags/")) {
- if (object->type == OBJ_TAG)
- prio = 2;
- else
- prio = 1;
- }
- else
- prio = 0;
- if (prio < min_prio)
- return 0;
-
- if (commit->name) {
- if (commit->name_prio >= prio)
- return 0;
- free(commit->name);
- }
- commit->name = xstrdup(path + 5);
- commit->name_prio = prio;
-
- return 0;
-}
-
-void load_commit_names(int min_prio)
-{
- for_each_ref(get_name, &min_prio);
-}
-
-struct possible_tag {
- char *name;
- int prio; /* annotated tag = 2, tag = 1, head = 0 */
- int depth;
- int found_order;
- unsigned flag_within;
-};
-
-static int compare_pt(const void *a_, const void *b_)
-{
- struct possible_tag *a = (struct possible_tag *)a_;
- struct possible_tag *b = (struct possible_tag *)b_;
- if (a->prio != b->prio)
- return b->prio - a->prio;
- if (a->depth != b->depth)
- return a->depth - b->depth;
- if (a->found_order != b->found_order)
- return a->found_order - b->found_order;
- return 0;
-}
-
-static unsigned long finish_depth_computation(
- struct commit_list **list,
- struct possible_tag *best)
-{
- unsigned long seen_commits = 0;
- while (*list) {
- struct commit *c = pop_commit(list);
- struct commit_list *parents = c->parents;
- seen_commits++;
- if (c->name_flags & best->flag_within) {
- struct commit_list *a = *list;
- while (a) {
- struct commit *i = a->item;
- if (!(i->name_flags & best->flag_within))
- break;
- a = a->next;
- }
- if (!a)
- break;
- } else
- best->depth++;
- while (parents) {
- struct commit *p = parents->item;
- parse_commit(p);
- if (!(p->name_flags & SEEN))
- insert_by_date(p, list);
- p->name_flags |= c->name_flags;
- parents = parents->next;
- }
- }
- return seen_commits;
-}
-
-char *describe_commit(struct commit *cmit, int max_candidates, int debug, int *depthp)
-{
- struct commit *gave_up_on = NULL;
- struct commit_list *list;
- struct possible_tag all_matches[MAX_TAGS];
- unsigned int match_cnt = 0, annotated_cnt = 0, cur_match;
- unsigned long seen_commits = 0;
-
- if (cmit->name) {
- *depthp = 0;
- return cmit->name;
- }
-
- if (debug)
- fprintf(stderr, "searching...\n");
-
- if (max_candidates < 1)
- max_candidates = 1;
- else if (max_candidates > MAX_TAGS)
- max_candidates = MAX_TAGS;
-
- list = NULL;
- cmit->name_flags = SEEN;
- commit_list_insert(cmit, &list);
- while (list) {
- struct commit *c = pop_commit(&list);
- struct commit_list *parents = c->parents;
- seen_commits++;
- if (c->name) {
- if (match_cnt < max_candidates) {
- struct possible_tag *t = &all_matches[match_cnt++];
- t->name = c->name;
- t->prio = c->name_prio;
- t->depth = seen_commits - 1;
- t->flag_within = 1u << match_cnt;
- t->found_order = match_cnt;
- c->name_flags |= t->flag_within;
- if (c->name_prio == 2)
- annotated_cnt++;
- }
- else {
- gave_up_on = c;
- break;
- }
- }
- for (cur_match = 0; cur_match < match_cnt; cur_match++) {
- struct possible_tag *t = &all_matches[cur_match];
- if (!(c->name_flags & t->flag_within))
- t->depth++;
- }
- if (annotated_cnt && !list) {
- if (debug)
- fprintf(stderr, "finished search at %s\n",
- sha1_to_hex(c->object.sha1));
- break;
- }
- while (parents) {
- struct commit *p = parents->item;
- parse_commit(p);
- if (!(p->name_flags & SEEN))
- insert_by_date(p, &list);
- p->name_flags |= c->name_flags;
- parents = parents->next;
- }
- }
-
- if (!match_cnt) {
- free_commit_list(list);
- *depthp = -1;
- return NULL;
- }
-
- qsort(all_matches, match_cnt, sizeof(all_matches[0]), compare_pt);
-
- if (gave_up_on) {
- insert_by_date(gave_up_on, &list);
- seen_commits--;
- }
- seen_commits += finish_depth_computation(&list, &all_matches[0]);
- free_commit_list(list);
-
- if (debug) {
- for (cur_match = 0; cur_match < match_cnt; cur_match++) {
- struct possible_tag *t = &all_matches[cur_match];
- fprintf(stderr, " %-11s %8d %s\n",
- prio_names[t->prio], t->depth, t->name);
- }
- fprintf(stderr, "traversed %lu commits\n", seen_commits);
- if (gave_up_on) {
- fprintf(stderr,
- "more than %i tags found; listed %i most recent\n"
- "gave up search at %s\n",
- max_candidates, max_candidates,
- sha1_to_hex(gave_up_on->object.sha1));
- }
- }
-
- *depthp = all_matches[0].depth;
- return all_matches[0].name;
-}
-
static void describe(const char *arg, int last_one)
{
unsigned char sha1[20];
diff --git a/cache.h b/cache.h
index bfffa05..84423a3 100644
--- a/cache.h
+++ b/cache.h
@@ -602,4 +602,9 @@ extern int diff_auto_refresh_index;
/* match-trees.c */
void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, int);
+/* describe.c */
+struct commit;
+extern void load_commit_names(int min_prio);
+extern char *describe_commit(struct commit *cmit, int max_candidates, int debug, int *depthp);
+
#endif /* CACHE_H */
diff --git a/describe.c b/describe.c
new file mode 100644
index 0000000..18c7abc
--- /dev/null
+++ b/describe.c
@@ -0,0 +1,204 @@
+#include "cache.h"
+#include "commit.h"
+#include "tag.h"
+#include "refs.h"
+
+#define SEEN (1u<<0)
+#define MAX_TAGS (FLAG_BITS - 1)
+
+static const char *prio_names[] = {
+ "head", "lightweight", "annotated",
+};
+
+static int get_name(const char *path, const unsigned char *sha1, int flag, void *cb_data)
+{
+ struct commit *commit = lookup_commit_reference_gently(sha1, 1);
+ struct object *object;
+ int min_prio = *((int *)cb_data);
+ int prio;
+
+ if (!commit)
+ return 0;
+ object = parse_object(sha1);
+
+ if (!prefixcmp(path, "refs/tags/")) {
+ if (object->type == OBJ_TAG)
+ prio = 2;
+ else
+ prio = 1;
+ }
+ else
+ prio = 0;
+ if (prio < min_prio)
+ return 0;
+
+ if (commit->name) {
+ if (commit->name_prio >= prio)
+ return 0;
+ free(commit->name);
+ }
+ commit->name = xstrdup(path + 5);
+ commit->name_prio = prio;
+
+ return 0;
+}
+
+void load_commit_names(int min_prio)
+{
+ for_each_ref(get_name, &min_prio);
+}
+
+struct possible_tag {
+ char *name;
+ int prio; /* annotated tag = 2, tag = 1, head = 0 */
+ int depth;
+ int found_order;
+ unsigned flag_within;
+};
+
+static int compare_pt(const void *a_, const void *b_)
+{
+ struct possible_tag *a = (struct possible_tag *)a_;
+ struct possible_tag *b = (struct possible_tag *)b_;
+ if (a->prio != b->prio)
+ return b->prio - a->prio;
+ if (a->depth != b->depth)
+ return a->depth - b->depth;
+ if (a->found_order != b->found_order)
+ return a->found_order - b->found_order;
+ return 0;
+}
+
+static unsigned long finish_depth_computation(
+ struct commit_list **list,
+ struct possible_tag *best)
+{
+ unsigned long seen_commits = 0;
+ while (*list) {
+ struct commit *c = pop_commit(list);
+ struct commit_list *parents = c->parents;
+ seen_commits++;
+ if (c->name_flags & best->flag_within) {
+ struct commit_list *a = *list;
+ while (a) {
+ struct commit *i = a->item;
+ if (!(i->name_flags & best->flag_within))
+ break;
+ a = a->next;
+ }
+ if (!a)
+ break;
+ } else
+ best->depth++;
+ while (parents) {
+ struct commit *p = parents->item;
+ parse_commit(p);
+ if (!(p->name_flags & SEEN))
+ insert_by_date(p, list);
+ p->name_flags |= c->name_flags;
+ parents = parents->next;
+ }
+ }
+ return seen_commits;
+}
+
+char *describe_commit(struct commit *cmit, int max_candidates, int debug, int *depthp)
+{
+ struct commit *gave_up_on = NULL;
+ struct commit_list *list;
+ struct possible_tag all_matches[MAX_TAGS];
+ unsigned int match_cnt = 0, annotated_cnt = 0, cur_match;
+ unsigned long seen_commits = 0;
+
+ if (cmit->name) {
+ *depthp = 0;
+ return cmit->name;
+ }
+
+ if (debug)
+ fprintf(stderr, "searching...\n");
+
+ if (max_candidates < 1)
+ max_candidates = 1;
+ else if (max_candidates > MAX_TAGS)
+ max_candidates = MAX_TAGS;
+
+ list = NULL;
+ cmit->name_flags = SEEN;
+ commit_list_insert(cmit, &list);
+ while (list) {
+ struct commit *c = pop_commit(&list);
+ struct commit_list *parents = c->parents;
+ seen_commits++;
+ if (c->name) {
+ if (match_cnt < max_candidates) {
+ struct possible_tag *t = &all_matches[match_cnt++];
+ t->name = c->name;
+ t->prio = c->name_prio;
+ t->depth = seen_commits - 1;
+ t->flag_within = 1u << match_cnt;
+ t->found_order = match_cnt;
+ c->name_flags |= t->flag_within;
+ if (c->name_prio == 2)
+ annotated_cnt++;
+ }
+ else {
+ gave_up_on = c;
+ break;
+ }
+ }
+ for (cur_match = 0; cur_match < match_cnt; cur_match++) {
+ struct possible_tag *t = &all_matches[cur_match];
+ if (!(c->name_flags & t->flag_within))
+ t->depth++;
+ }
+ if (annotated_cnt && !list) {
+ if (debug)
+ fprintf(stderr, "finished search at %s\n",
+ sha1_to_hex(c->object.sha1));
+ break;
+ }
+ while (parents) {
+ struct commit *p = parents->item;
+ parse_commit(p);
+ if (!(p->name_flags & SEEN))
+ insert_by_date(p, &list);
+ p->name_flags |= c->name_flags;
+ parents = parents->next;
+ }
+ }
+
+ if (!match_cnt) {
+ free_commit_list(list);
+ *depthp = -1;
+ return NULL;
+ }
+
+ qsort(all_matches, match_cnt, sizeof(all_matches[0]), compare_pt);
+
+ if (gave_up_on) {
+ insert_by_date(gave_up_on, &list);
+ seen_commits--;
+ }
+ seen_commits += finish_depth_computation(&list, &all_matches[0]);
+ free_commit_list(list);
+
+ if (debug) {
+ for (cur_match = 0; cur_match < match_cnt; cur_match++) {
+ struct possible_tag *t = &all_matches[cur_match];
+ fprintf(stderr, " %-11s %8d %s\n",
+ prio_names[t->prio], t->depth, t->name);
+ }
+ fprintf(stderr, "traversed %lu commits\n", seen_commits);
+ if (gave_up_on) {
+ fprintf(stderr,
+ "more than %i tags found; listed %i most recent\n"
+ "gave up search at %s\n",
+ max_candidates, max_candidates,
+ sha1_to_hex(gave_up_on->object.sha1));
+ }
+ }
+
+ *depthp = all_matches[0].depth;
+ return all_matches[0].name;
+}
--
1.5.3.5.529.ge3d6d
^ permalink raw reply related
* Re: Why is --pretty=format: so slow?
From: Junio C Hamano @ 2007-11-04 11:48 UTC (permalink / raw)
To: René Scharfe; +Cc: Paul Mackerras, git, Johannes Schindelin
In-Reply-To: <472D9F6E.8070609@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> Incidentally, I'm finishing a patch series to add git-describe style
> placeholders. It needs such an optimization even more badly (and has
> it). Speeding up other slow placeholder expansions falls out quite
> naturally. Please wait just a few minutes..
Thanks, and take your time, as I am just going to bed, dreaming
for a perfect series from one of the few people on this list
whose patches I do not have to read to reject crap, but I do
read to get impressed and admire ;-)
^ permalink raw reply
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