* Re: [PATCH/RFC] git-am: Make it easier to see which patch failed
From: Jeff King @ 2009-01-18 17:49 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Stephan Beyer, Junio C Hamano, Jonas Flodén, git
In-Reply-To: <alpine.DEB.1.00.0901181843230.3586@pacific.mpi-cbg.de>
On Sun, Jan 18, 2009 at 06:44:01PM +0100, Johannes Schindelin wrote:
> Oh. Okay, so I was wrong. But only because dash's echo behaves in a
> strange way: it makes "-e" a noop?
Right. "-e" isn't in POSIX at all, and it is a SysV-ism to allow escapes
in any argument (I don't know if "-e" was introduced by GNU people, or
came from elsewhere).
See here for the gory details:
http://www.opengroup.org/onlinepubs/009695399/utilities/echo.html
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] color: make it easier for non-config to parse color specs
From: Jeff King @ 2009-01-18 18:06 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Markus Heidelberg, git
In-Reply-To: <49736A9C.1040601@lsrfire.ath.cx>
On Sun, Jan 18, 2009 at 06:45:00PM +0100, René Scharfe wrote:
> > need a "memcasecmp" here.
>
> if (!strncasecmp(value, "reset", len)) {
Thanks. I knew there must be some stock function to do this, but for
some reason I just could not think of strncasecmp.
> die("bad color value '%.*s' for variable '%s', len, value, var);
Except that we've been shortening "len" during the course of the
function, so it is generally 0 at this point.
> > $ time ./git log --pretty=tformat:'%Credfoo%Creset' >/dev/null
> > real 0m0.673s
> > user 0m0.652s
> > sys 0m0.016s
> > $ time ./git log --pretty=tformat:'%C(red)foo%C(reset)' >/dev/null
> > real 0m0.692s
> > user 0m0.660s
> > sys 0m0.032s
> >
> > That's about 1 microsecond per commit.
>
> Hmm, not too much overhead, agreed, but it would still be nice to avoid it.
Here are the numbers with your fixes:
$ time ./git log --pretty=tformat:'%C(red)foo%C(reset)' >/dev/null
real 0m0.677s
user 0m0.668s
sys 0m0.008s
which puts the difference well into the noise region (I actually did get
one run with your patch that was just as fast as the original).
Here is an updated version of patch 1/2, squashing in your
color_mem_parse update, your follow-on fix, and a fix for the die().
-- >8 --
color: make it easier for non-config to parse color specs
We have very featureful color-parsing routines which are
used for color.diff.* and other options. Let's make it
easier to use those routines from other parts of the code.
This patch converts color_parse to color_parse_mem, taking a
length-bounded string instead of a NUL-terminated one. We
keep color_parse as a wrapper which takes a normal string.
Thanks to René Scharfe for rewriting the color_parse
implementation.
This also changes the error string for an invalid color not
to mention the word "config", since it is not always
appropriate (and when it is, the context is obvious since
the offending config variable is given).
Finally, while we are in the area, we clean up the parameter
names in the declaration of color_parse; the var and value
parameters were reversed from the actual implementation.
---
color.c | 31 +++++++++++++++++++++----------
color.h | 3 ++-
2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/color.c b/color.c
index fc0b72a..915d7a9 100644
--- a/color.c
+++ b/color.c
@@ -41,29 +41,40 @@ static int parse_attr(const char *name, int len)
void color_parse(const char *value, const char *var, char *dst)
{
+ color_parse_mem(value, strlen(value), var, dst);
+}
+
+void color_parse_mem(const char *value, int value_len, const char *var,
+ char *dst)
+{
const char *ptr = value;
+ int len = value_len;
int attr = -1;
int fg = -2;
int bg = -2;
- if (!strcasecmp(value, "reset")) {
+ if (!strncasecmp(value, "reset", len)) {
strcpy(dst, "\033[m");
return;
}
/* [fg [bg]] [attr] */
- while (*ptr) {
+ while (len > 0) {
const char *word = ptr;
- int val, len = 0;
+ int val, wordlen = 0;
- while (word[len] && !isspace(word[len]))
- len++;
+ while (len > 0 && !isspace(word[wordlen])) {
+ wordlen++;
+ len--;
+ }
- ptr = word + len;
- while (*ptr && isspace(*ptr))
+ ptr = word + wordlen;
+ while (len > 0 && isspace(*ptr)) {
ptr++;
+ len--;
+ }
- val = parse_color(word, len);
+ val = parse_color(word, wordlen);
if (val >= -1) {
if (fg == -2) {
fg = val;
@@ -75,7 +86,7 @@ void color_parse(const char *value, const char *var, char *dst)
}
goto bad;
}
- val = parse_attr(word, len);
+ val = parse_attr(word, wordlen);
if (val < 0 || attr != -1)
goto bad;
attr = val;
@@ -115,7 +126,7 @@ void color_parse(const char *value, const char *var, char *dst)
*dst = 0;
return;
bad:
- die("bad config value '%s' for variable '%s'", value, var);
+ die("bad color value '%.*s' for variable '%s'", value_len, value, var);
}
int git_config_colorbool(const char *var, const char *value, int stdout_is_tty)
diff --git a/color.h b/color.h
index 6cf5c88..7066099 100644
--- a/color.h
+++ b/color.h
@@ -16,7 +16,8 @@ extern int git_use_color_default;
int git_color_default_config(const char *var, const char *value, void *cb);
int git_config_colorbool(const char *var, const char *value, int stdout_is_tty);
-void color_parse(const char *var, const char *value, char *dst);
+void color_parse(const char *value, const char *var, char *dst);
+void color_parse_mem(const char *value, int len, const char *var, char *dst);
int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);
--
1.6.1.266.g3b9d0.dirty
^ permalink raw reply related
* Re: [PATCH 2/3] Teach read_tree_recursive() how to traverse into submodules
From: Johannes Schindelin @ 2009-01-18 18:33 UTC (permalink / raw)
To: Lars Hjemli; +Cc: Junio C Hamano, git
In-Reply-To: <8c5c35580901180945u17a69140vff2736765ee6073@mail.gmail.com>
Hi,
On Sun, 18 Jan 2009, Lars Hjemli wrote:
> On Sun, Jan 18, 2009 at 16:48, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> > On Sun, 18 Jan 2009, Lars Hjemli wrote:
> >
> >> + struct tree **subtree)
> >> +{
> >> + unsigned char sha1[20];
> >> + int linked_odb = 0;
> >> + struct commit *commit;
> >> + void *buffer;
> >> + enum object_type type;
> >> + unsigned long size;
> >> +
> >> + hashcpy(sha1, commit_sha1);
> >> + if (!add_gitlink_odb(path)) {
> >> + linked_odb = 1;
> >> + if (resolve_gitlink_ref(path, "HEAD", sha1))
> >> + die("Unable to lookup HEAD in %s", path);
> >> + }
> >
> > Why would you want to continue if add_gitlink_odb() did not find a checked
> > out submodule?
> >
> > Seems you want to fall back to look in the superproject's object database.
> > But I think that is wrong, as I have a superproject with many platform
> > dependent submodules, only one of which is checked out, and for
> > convenience, the submodules all live in the superproject's repository.
>
> Actually, I want this to work for bare repositories by specifying the
> submodule odbs in the alternates file. So if the current submodule odb
> wasn't found my plan was to check if the commit object was accessible
> anyways but don't die() if it wasn't.
Please make that an explicit option (cannot think of a good name, though),
otherwise I will not be able to use your feature. Making it the default
would be inconsistent with the rest of our submodules framework.
> >> + commit = lookup_commit(sha1);
> >> + if (!commit)
> >> + die("traverse_gitlink(): internal error");
> >
> > s/internal error/could not access commit '%s' of submodule '%s'",
> > sha1_to_hex(sha1), path);/
>
> Ok (I belive this codepath is virtually impossible to hit, hence the
> "internal error", but I could of course be mistaken).
You make it a function that is exported to other parts of Git in cache.h.
So you might just as well expect it to be used by other parts at some
stage.
Ciao,
Dscho
^ permalink raw reply
* John (zzz) Doe <john.doe@xz> (Comment)
From: Junio C Hamano @ 2009-01-18 18:50 UTC (permalink / raw)
To: Kirill Smelkov; +Cc: Johannes Schindelin, git
In-Reply-To: <20090118145429.GA27522@roro3.zxlink>
Kirill Smelkov <kirr@landau.phys.spbu.ru> writes:
> On Fri, Jan 16, 2009 at 12:54:28PM +0100, Johannes Schindelin wrote:
> ...
>> > From: A U Thor (MonikeR) <a.u@thor.xz>
>>
>> It is Philippe Bruhat (BooK), who sometimes forgets the closing
>> parenthesis, and who is listed in .mailmap without the moniker.
It was not "forgets", but is an artifact that older mailinfo removed
parenthesis incorrectly (see below).
> So now I don't understand what to do.
>
> From one hand RFC822 says '(...)' is a comment, and from the other hand,
> we have a use case where one guy wants this to stay.
> ...
> commit 49bebfbe18dac296e5e246884bd98c1f90be9676
> Author: Kirill Smelkov <kirr@landau.phys.spbu.ru>
> Date: Tue Jan 13 01:21:04 2009 +0300
>
> mailinfo: more smarter removal of rfc822 comments from 'From'
>
> As described in RFC822 (3.4.3 COMMENTS, and A.1.4.), comments, as e.g.
>
> John (zzz) Doe <john.doe@xz> (Comment)
>
> should "NOT [be] included in the destination mailbox"
The above quote from the RFC is irrelevant. Note that it is only about
how you extract the e-mail address, discarding everything else.
What mailinfo wants to do is to separate the human-readable name and the
e-mail address, and we want to use _both_ results from it.
We separate a few example From: lines like this:
Kirill Smelkov <kirr@smelkov.xz>
==> AUTHOR_EMAIL="kirr@smelkov.xz" AUTHOR_NAME="Kirill Smelkov"
kirr@smelkov.xz (Kirill Smelkov)
==> AUTHOR_EMAIL="kirr@smelkov.xz" AUTHOR_NAME="Kirill Smelkov"
Traditionally, the way people spelled their name on From: line has been
either one of the above form. Typically comment form (i.e. the second
one) adds the name at the end, while "Name <addr>" form has the name at
the front. But I do not think RFC requires that, primarily because it is
all about discarding non-address part to find the e-mail address aka
"destination mailbox". It does not specify how humans should interpret
the human readable name and the comment.
Now, why is the name not AUTHOR_NAME="(Kirill Smelkov)" in the latter
form?
It is just common sense transformation. Otherwise it looks simply ugly,
and it is obvious that the parentheses is not part of the name of the
person who used "kirr@smelkov.xz (Kirill Smelkov)" on his From: line.
So we can separate "John (zzz) Doe <john.doe@xz> (Comment)" into:
AUTHOR_EMAIL=john.doe@xz
AUTHOR_NAME="John (zzz) Doe (Comment)"
and leave it like so, I think.
^ permalink raw reply
* Re: [PATCH 2/3] Teach read_tree_recursive() how to traverse into submodules
From: Junio C Hamano @ 2009-01-18 19:00 UTC (permalink / raw)
To: Lars Hjemli; +Cc: René Scharfe, git
In-Reply-To: <8c5c35580901180837i6e835d98ob8875ce1b8ad3011@mail.gmail.com>
Lars Hjemli <hjemli@gmail.com> writes:
> I like the idea, but it will require thorough review of all
> read_tree_recursive() consumers. So now we've got three different
> approaches:
> * me: global setting
> * dscho: parameter to read_tree_recursive()
> * you: accept the return value from the callback function
>
> Junio, what would you prefer?
As usual René has the best taste in designing things ;-)
^ permalink raw reply
* Re: [WIP Patch 02/12] Some cleanup in get_refs_via_curl()
From: Johannes Schindelin @ 2009-01-18 19:06 UTC (permalink / raw)
To: Mike Hommey; +Cc: git, gitster
In-Reply-To: <1232265877-3649-3-git-send-email-mh@glandium.org>
Hi,
On Sun, 18 Jan 2009, Mike Hommey wrote:
> diff --git a/transport.c b/transport.c
> index 56831c5..6919ff1 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -508,6 +508,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)
> free(ref);
> }
>
> + http_cleanup();
> + free(refs_url);
> return refs;
> }
You cannot http_cleanup() here, as http-push calls that function, but
continues to want to use curl.
Ciao,
Dscho
^ permalink raw reply
* Re: [WIP Patch 02/12] Some cleanup in get_refs_via_curl()
From: Johannes Schindelin @ 2009-01-18 19:11 UTC (permalink / raw)
To: Mike Hommey; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901182005230.3586@pacific.mpi-cbg.de>
Hi,
On Sun, 18 Jan 2009, Johannes Schindelin wrote:
> On Sun, 18 Jan 2009, Mike Hommey wrote:
>
> > diff --git a/transport.c b/transport.c
> > index 56831c5..6919ff1 100644
> > --- a/transport.c
> > +++ b/transport.c
> > @@ -508,6 +508,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)
> > free(ref);
> > }
> >
> > + http_cleanup();
> > + free(refs_url);
> > return refs;
> > }
>
> You cannot http_cleanup() here, as http-push calls that function, but
> continues to want to use curl.
Worse, a http clone will hit the same issue.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 3/7 v2] git_extract_argv0_path(): Move check for valid argv0 from caller to callee
From: Johannes Sixt @ 2009-01-18 19:16 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <1232280015-8144-4-git-send-email-prohaska@zib.de>
On Sonntag, 18. Januar 2009, Steffen Prohaska wrote:
> This simplifies the calling code.
But it could really be squashed into the previous patch, after fixing...
> @@ -23,6 +23,9 @@ const char *system_path(const char *path)
>
> const char *git_extract_argv0_path(const char *argv0)
> {
> + if (!argv0 || !*argv0)
> + return 0;
> +
> const char *slash = argv0 + strlen(argv0);
... this declaration after statement.
-- Hannes
^ permalink raw reply
* Re: [WIP Patch 02/12] Some cleanup in get_refs_via_curl()
From: Mike Hommey @ 2009-01-18 19:19 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901182005230.3586@pacific.mpi-cbg.de>
On Sun, Jan 18, 2009 at 08:06:17PM +0100, Johannes Schindelin wrote:
> Hi,
>
> On Sun, 18 Jan 2009, Mike Hommey wrote:
>
> > diff --git a/transport.c b/transport.c
> > index 56831c5..6919ff1 100644
> > --- a/transport.c
> > +++ b/transport.c
> > @@ -508,6 +508,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)
> > free(ref);
> > }
> >
> > + http_cleanup();
> > + free(refs_url);
> > return refs;
> > }
>
> You cannot http_cleanup() here, as http-push calls that function, but
> continues to want to use curl.
Are you really sure? It doesn't seem so, to me.
Mike
^ permalink raw reply
* Re: [WIP Patch 04/12] Use the new http API in http_fetch_ref()
From: Mike Hommey @ 2009-01-18 19:21 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901181607210.3586@pacific.mpi-cbg.de>
On Sun, Jan 18, 2009 at 04:10:38PM +0100, Johannes Schindelin wrote:
> Hi,
>
> On Sun, 18 Jan 2009, Mike Hommey wrote:
>
> > diff --git a/http.c b/http.c
> > index 82534cf..0c9504b 100644
> > --- a/http.c
> > +++ b/http.c
> > @@ -604,34 +604,17 @@ int http_fetch_ref(const char *base, struct ref *ref)
> > {
> > char *url;
> > struct strbuf buffer = STRBUF_INIT;
> > - struct active_request_slot *slot;
> > - struct slot_results results;
> > - int ret;
> > + int ret = -1;
> >
> > url = quote_ref_url(base, ref->name);
> > - slot = get_active_slot();
> > - slot->results = &results;
> > - curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
> > - curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
> > - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL);
> > - curl_easy_setopt(slot->curl, CURLOPT_URL, url);
> > - if (start_active_slot(slot)) {
> > - run_active_slot(slot);
> > - if (results.curl_result == CURLE_OK) {
> > - strbuf_rtrim(&buffer);
> > - if (buffer.len == 40)
> > - ret = get_sha1_hex(buffer.buf, ref->old_sha1);
> > - else if (!prefixcmp(buffer.buf, "ref: ")) {
> > - ref->symref = xstrdup(buffer.buf + 5);
> > - ret = 0;
> > - } else
> > - ret = 1;
> > - } else {
> > - ret = error("Couldn't get %s for %s\n%s",
> > - url, ref->name, curl_errorstr);
> > + if (http_get_strbuf(url, &buffer, HTTP_NO_CACHE) == HTTP_OK) {
> > + strbuf_rtrim(&buffer);
> > + if (buffer.len == 40)
> > + ret = get_sha1_hex(buffer.buf, ref->old_sha1);
> > + else if (!prefixcmp(buffer.buf, "ref: ")) {
> > + ref->symref = xstrdup(buffer.buf + 5);
> > + ret = 0;
> > }
> > - } else {
> > - ret = error("Unable to start request");
> > }
>
> Why not keep that error?
It should be handled in http_request, I'd say...
Mike
^ permalink raw reply
* Re: [WIP Patch 08/12] Use the new http API in update_remote_info_refs()
From: Mike Hommey @ 2009-01-18 19:23 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901181615070.3586@pacific.mpi-cbg.de>
On Sun, Jan 18, 2009 at 04:18:16PM +0100, Johannes Schindelin wrote:
> Hi,
>
> On Sun, 18 Jan 2009, Mike Hommey wrote:
>
> >
> > Signed-off-by: Mike Hommey <mh@glandium.org>
> > ---
> > http-push.c | 29 ++++++++++-------------------
> > 1 files changed, 10 insertions(+), 19 deletions(-)
> >
> > diff --git a/http-push.c b/http-push.c
> > index e0b4f5a..7627860 100644
> > --- a/http-push.c
> > +++ b/http-push.c
> > @@ -1960,29 +1960,20 @@ static void update_remote_info_refs(struct remote_lock *lock)
> > static int remote_exists(const char *path)
> > {
>
> Heh, I see where your commit subject comes from, but it should rather
> mention the function "remote_exists()"...
>
> > char *url = xmalloc(strlen(remote->url) + strlen(path) + 1);
> > - struct active_request_slot *slot;
> > - struct slot_results results;
> > - int ret = -1;
> > + int ret;
> >
> > sprintf(url, "%s%s", remote->url, path);
> >
> > - slot = get_active_slot();
> > - slot->results = &results;
> > - curl_easy_setopt(slot->curl, CURLOPT_URL, url);
> > - curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1);
> > -
> > - if (start_active_slot(slot)) {
> > - run_active_slot(slot);
> > - if (results.http_code == 404)
> > - ret = 0;
> > - else if (results.curl_result == CURLE_OK)
> > - ret = 1;
> > - else
> > - fprintf(stderr, "HEAD HTTP error %ld\n", results.http_code);
> > - } else {
> > - fprintf(stderr, "Unable to start HEAD request\n");
> > + switch (http_get_strbuf(url, NULL, 0)) {
> > + case HTTP_OK:
> > + ret = 1;
> > + break;
> > + case HTTP_MISSING_TARGET:
> > + ret = 0;
> > + break;
> > + default:
> > + ret = -1;
> > }
>
> Does http_get_strbuf() already show the error? Not as far as I can see,
> even if it would make sense, no? At least you'll have to "return
> error(...)".
As I said, it has some error handling regressions ;)
Mike
^ permalink raw reply
* Re: [PATCH/RFC v1 1/1] +5 cases (4 fail), diff whitespace tests
From: Keith Cascio @ 2009-01-18 19:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd4elys21.fsf@gitster.siamese.dyndns.org>
On Sat, 17 Jan 2009, Junio C Hamano wrote:
> Hmm. Are these three supposed to be orthogonal?
The semantics of those 3 flags are not orthogonal, no. Their relationship
amongst each other is one of transitive implication:
-w implies the other two
-b implies --ignore-space-at-eol
--ignore-space-at-eol implies only itself
Therefore, it is never *necessary* to specify more than one of these flags on
the command line. However, it is not hard to imagine scenarios where software
wrappers around git (e.g. GUIs, etc), generate command lines with more than one
of these flags. I thought about it, and it seems unreasonable to make it an
error to specify more than one, since a new user might not immediately grasp the
way they imply each other. I think Git could and should treat it as a legal
case. I contacted Dscho about fixing it, but he is busy so I will submit a fix
patch myself.
^ permalink raw reply
* Re: [PATCH 3/7 v2] git_extract_argv0_path(): Move check for valid argv0 from caller to callee
From: Johannes Sixt @ 2009-01-18 19:28 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: Junio C Hamano, git, Johannes Schindelin
In-Reply-To: <200901182016.56395.j6t@kdbg.org>
On Sonntag, 18. Januar 2009, Johannes Sixt wrote:
> On Sonntag, 18. Januar 2009, Steffen Prohaska wrote:
> > This simplifies the calling code.
>
> But it could really be squashed into the previous patch, after fixing...
>
> > @@ -23,6 +23,9 @@ const char *system_path(const char *path)
> >
> > const char *git_extract_argv0_path(const char *argv0)
> > {
> > + if (!argv0 || !*argv0)
> > + return 0;
> > +
> > const char *slash = argv0 + strlen(argv0);
>
> ... this declaration after statement.
And we prefer NULL over 0 for the null pointer.
The series is nicely done, thank you! I am using it (the previous round)
without problems so far. I hope we can get this in RSN.
-- Hannes
^ permalink raw reply
* Re: [PATCH] contrib: add 'git difftool' for launching common merge tools
From: Markus Heidelberg @ 2009-01-18 19:25 UTC (permalink / raw)
To: David Aguilar; +Cc: git, gitster
In-Reply-To: <1232092802-30838-1-git-send-email-davvid@gmail.com>
David Aguilar, 16.01.2009:
> diff --git a/contrib/difftool/git-difftool b/contrib/difftool/git-difftool
> new file mode 100755
> index 0000000..1fc087c
> --- /dev/null
> +++ b/contrib/difftool/git-difftool
> @@ -0,0 +1,74 @@
> +#!/usr/bin/env perl
> +# Copyright (c) 2009 David Aguilar
> +#
> +# This is a wrapper around the GIT_EXTERNAL_DIFF-compatible
> +# git-difftool-helper script. This script exports
> +# GIT_EXTERNAL_DIFF and GIT_PAGER for use by git, and
> +# GIT_NO_PROMPT and GIT_MERGE_TOOL for use by git-difftool-helper.
GIT_DIFFTOOL_NO_PROMPT
> +sub usage
> +{
> + print << 'USAGE';
> +
Why the leading empty line?
> +usage: git difftool [--no-prompt] [--tool=tool] ["git diff" options]
--tool=<tool>
Swap the order of --no-prompt and --tool for consistency with
git-difftool.txt and git-mergetool.
> diff --git a/contrib/difftool/git-difftool-helper b/contrib/difftool/git-difftool-helper
> new file mode 100755
> index 0000000..0b266e3
> --- /dev/null
> +++ b/contrib/difftool/git-difftool-helper
> + meld|vimdiff)
> + "$merge_tool_path" "$LOCAL" "$REMOTE"
> + ;;
> +
> + gvimdiff)
> + "$merge_tool_path" -f "$LOCAL" "$REMOTE"
> + ;;
Maybe use '-c "wincmd l"' for Vim as in my patch for git-mergetool to
automatically place the cursor in the editable file? Useful for editing,
if git-difftool is used to diff a file from the working tree.
See http://thread.gmane.org/gmane.comp.version-control.git/106109
> diff --git a/contrib/difftool/git-difftool.txt b/contrib/difftool/git-difftool.txt
> new file mode 100644
> index 0000000..3940c70
> --- /dev/null
> +++ b/contrib/difftool/git-difftool.txt
You have deleted all the '-' chars from git-command, but when using it as the
name I think it's the preferred method, only when used as command then without
slash.
> +DESCRIPTION
> +-----------
> +'git difftool' is a git command that allows you to compare and edit files
If for example 'git difftool' is used as the name of the tool and not
the command, I think the dash should be kept.
> +between revisions using common merge tools. At its most basic level,
> +'git difftool' does what 'git mergetool' does but its use is for non-merge
Keep the dash.
> +situations such as when preparing commits or comparing changes against
> +the index.
> +
> +'git difftool' is a frontend to 'git diff' and accepts the same
Keep the dash.
> +arguments and options.
> +
> +See linkgit:git-diff[7] for the full list of supported options.
[1]
> +
> +OPTIONS
> +-------
> +-t <tool>::
> +--tool=<tool>::
> + Use the merge resolution program specified by <tool>.
> + Valid merge tools are:
> + kdiff3, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff, ecmerge, and opendiff
> ++
> +If a merge resolution program is not specified, 'git difftool'
Keep the dash.
> +situations such as when preparing commits or comparing changes against
> +will use the configuration variable `merge.tool`. If the
> +configuration variable `merge.tool` is not set, 'git difftool'
Keep the dash.
> +situations such as when preparing commits or comparing changes against
> +will pick a suitable default.
> ++
> +You can explicitly provide a full path to the tool by setting the
> +configuration variable `mergetool.<tool>.path`. For example, you
> +can configure the absolute path to kdiff3 by setting
> +`mergetool.kdiff3.path`. Otherwise, 'git difftool' assumes the
Keep the dash.
> +situations such as when preparing commits or comparing changes against
> +tool is available in PATH.
> ++
> +Instead of running one of the known merge tool programs,
> +'git difftool' can be customized to run an alternative program
Keep the dash.
> +situations such as when preparing commits or comparing changes against
> +by specifying the command line to invoke in a configuration
> +variable `mergetool.<tool>.cmd`.
> ++
> +When 'git difftool' is invoked with this tool (either through the
Keep the dash.
> +--no-prompt::
> + Do not prompt before launching a merge tool.
the diff tool
> +SEE ALSO
> +--------
> +linkgit:git-diff[7]::
[1]
> + Show changes between commits, commit and working tree, etc
> +
> +linkgit:git-mergetool[1]::
> + Run merge conflict resolution tools to resolve merge conflicts
> +
> +linkgit:git-config[7]::
[1]
Works fine for me, thanks.
Markus
^ permalink raw reply
* Re: [WIP Patch 02/12] Some cleanup in get_refs_via_curl()
From: Mike Hommey @ 2009-01-18 19:30 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901182010380.3586@pacific.mpi-cbg.de>
On Sun, Jan 18, 2009 at 08:11:07PM +0100, Johannes Schindelin wrote:
> Hi,
>
> On Sun, 18 Jan 2009, Johannes Schindelin wrote:
>
> > On Sun, 18 Jan 2009, Mike Hommey wrote:
> >
> > > diff --git a/transport.c b/transport.c
> > > index 56831c5..6919ff1 100644
> > > --- a/transport.c
> > > +++ b/transport.c
> > > @@ -508,6 +508,8 @@ static struct ref *get_refs_via_curl(struct transport *transport)
> > > free(ref);
> > > }
> > >
> > > + http_cleanup();
> > > + free(refs_url);
> > > return refs;
> > > }
> >
> > You cannot http_cleanup() here, as http-push calls that function, but
> > continues to want to use curl.
>
> Worse, a http clone will hit the same issue.
IIRC, it doesn't break anything because getting refs is going to happen
at the beginning, and the following http request is going to
reinitialize the whole thing.
It is suboptimal, I agree.
Mike
^ permalink raw reply
* Re: [PATCH 2/2] expand --pretty=format color options
From: Jeff King @ 2009-01-18 19:43 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Markus Heidelberg, git
In-Reply-To: <20090118173753.GB17434@coredump.intra.peff.net>
On Sun, Jan 18, 2009 at 12:37:53PM -0500, Jeff King wrote:
> On Sun, Jan 18, 2009 at 06:13:21PM +0100, René Scharfe wrote:
> > Another step would be for --pretty=format to respect the color config,
> > i.e. it shouldn't print any colour codes if colouring is turned off or
> > if set to auto while writing to file or pipe.
>
> That makes sense to me. In theory, we could offer an "always use this
> color" and a "conditionally use this color" substitution. But I don't
> really see why anyone would want the "always use this color" one (they
> could just say --color, then).
Here is a patch that seems to work. It predicates pretty format colors
on diff colors, which is the same way the yellow commit header works in
log-tree. I don't know if it makes more sense to introduce yet another
color config option.
And I say "seems to work" because I remember there being some trickery
with color flags sometimes not getting set properly. However, since this
is the same flag as the yellow commit header, and called around the same
time, I think it should be fine.
One final note: if you are writing "generic" format strings, you
probably don't want to say "yellow". You want to say "whatever the user
has configured for color.diff.commit". I don't know if %C should be
overloaded for that or not (i.e., %C(commit) would be a valid color).
-- >8 --
respect color settings for --pretty=format colors
Previously, we would unconditionally print any colors the
user put into the --pretty=format specifier, regardless of
the setting of color configuration. This is not a problem if
the user writes a custom string each time git-log is
invoked. However, it makes use in scripts unnecessarily
complex, since they would have to change the format string
based on user preferences (and whether output is going to a
tty).
This patch will ignore any colors in --pretty=format if diff
colors are not turned on (they will still be parsed and
treated as placeholders, but no color will be output). This
matches the color.diff.commit colorization, which depends on
diff coloring even though it is not technically part of a
diff.
---
pretty.c | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/pretty.c b/pretty.c
index b1b8620..fe606c5 100644
--- a/pretty.c
+++ b/pretty.c
@@ -563,20 +563,25 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
color_parse_mem(placeholder + 2,
end - (placeholder + 2),
"--pretty format", color);
- strbuf_addstr(sb, color);
+ if (diff_use_color_default)
+ strbuf_addstr(sb, color);
return end - placeholder + 1;
}
if (!prefixcmp(placeholder + 1, "red")) {
- strbuf_addstr(sb, "\033[31m");
+ if (diff_use_color_default)
+ strbuf_addstr(sb, "\033[31m");
return 4;
} else if (!prefixcmp(placeholder + 1, "green")) {
- strbuf_addstr(sb, "\033[32m");
+ if (diff_use_color_default)
+ strbuf_addstr(sb, "\033[32m");
return 6;
} else if (!prefixcmp(placeholder + 1, "blue")) {
- strbuf_addstr(sb, "\033[34m");
+ if (diff_use_color_default)
+ strbuf_addstr(sb, "\033[34m");
return 5;
} else if (!prefixcmp(placeholder + 1, "reset")) {
- strbuf_addstr(sb, "\033[m");
+ if (diff_use_color_default)
+ strbuf_addstr(sb, "\033[m");
return 6;
} else
return 0;
--
1.6.1.151.g5a7da.dirty
^ permalink raw reply related
* Re: [PATCH 2/3] Teach read_tree_recursive() how to traverse into submodules
From: Lars Hjemli @ 2009-01-18 19:45 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901181929220.3586@pacific.mpi-cbg.de>
On Sun, Jan 18, 2009 at 19:33, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Sun, 18 Jan 2009, Lars Hjemli wrote:
>> Actually, I want this to work for bare repositories by specifying the
>> submodule odbs in the alternates file. So if the current submodule odb
>> wasn't found my plan was to check if the commit object was accessible
>> anyways but don't die() if it wasn't.
>
> Please make that an explicit option (cannot think of a good name, though),
> otherwise I will not be able to use your feature. Making it the default
> would be inconsistent with the rest of our submodules framework.
Would a test on is_bare_repository() suffice for your use-case? That
is, something like this:
if (!add_gitlink_odb(path->buf)) {
linked_odb = 1;
if (resolve_gitlink_ref(path->buf, "HEAD", sha1))
die("Unable to lookup HEAD in %s", path->buf);
} else if (!is_bare_repository())
return 0;
If this isn't good enough, how do you propose it be solved?
>
>> >> + commit = lookup_commit(sha1);
>> >> + if (!commit)
>> >> + die("traverse_gitlink(): internal error");
>> >
>> > s/internal error/could not access commit '%s' of submodule '%s'",
>> > sha1_to_hex(sha1), path);/
>>
>> Ok (I belive this codepath is virtually impossible to hit, hence the
>> "internal error", but I could of course be mistaken).
>
> You make it a function that is exported to other parts of Git in cache.h.
> So you might just as well expect it to be used by other parts at some
> stage.
This function is local to tree.c, but your point is still valid.
--
larsh
^ permalink raw reply
* Re: [PATCH 2/3] Teach read_tree_recursive() how to traverse into submodules
From: Lars Hjemli @ 2009-01-18 19:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: René Scharfe, git
In-Reply-To: <7vfxjgxwv7.fsf@gitster.siamese.dyndns.org>
On Sun, Jan 18, 2009 at 20:00, Junio C Hamano <gitster@pobox.com> wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
>
>> I like the idea, but it will require thorough review of all
>> read_tree_recursive() consumers. So now we've got three different
>> approaches:
>> * me: global setting
>> * dscho: parameter to read_tree_recursive()
>> * you: accept the return value from the callback function
>>
>> Junio, what would you prefer?
>
> As usual René has the best taste in designing things ;-)
Agreed. I've pushed an updated series using his approach to the
lh/traverse-gitlinks branch in git://hjemli.net/pub/git/git
(http://hjemli.net/git/git/log/?h=lh/traverse-gitlinks), but without a
fix for Dscho's concern about the behaviour in
tree.c::traverse_gitlink(). Hopefully I'll get to send a reworked
series later tonight.
--
larsh
^ permalink raw reply
* Re: [PATCH 2/2] expand --pretty=format color options
From: Jeff King @ 2009-01-18 19:53 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Markus Heidelberg, git
In-Reply-To: <20090118194328.GA31180@coredump.intra.peff.net>
On Sun, Jan 18, 2009 at 02:43:28PM -0500, Jeff King wrote:
> Here is a patch that seems to work. It predicates pretty format colors
> on diff colors, which is the same way the yellow commit header works in
> log-tree. I don't know if it makes more sense to introduce yet another
> color config option.
>
> And I say "seems to work" because I remember there being some trickery
> with color flags sometimes not getting set properly. However, since this
> is the same flag as the yellow commit header, and called around the same
> time, I think it should be fine.
Hrm. OK, it doesn't actually work always. It does for git-log, but not
for rev-list, which leaves diff_use_color_default as -1. I don't know if
there are any other ways you can get to this code path without having
set diff_use_color_default.
Maybe it is time to do a cleanup on the color handling, which has
provided no end of these bugs. I will have to leave that for another
day, though.
-Peff
^ permalink raw reply
* Re: [PATCH] contrib/workdir: create logs/refs and rr-cache in the origin repository
From: Junio C Hamano @ 2009-01-18 19:59 UTC (permalink / raw)
To: Adeodato Simó; +Cc: git
In-Reply-To: <20090118113830.GA1394@chistera.yi.org>
Adeodato Simó <dato@net.com.org.es> writes:
> However, I've as of late directly created bare repositories knowing that
> I wanted to work just with workdirs against it. In this case, the logs
> for each checkout'ed branch will be stored in the workdirs and not the
> repo, so deleting the workdir will make you lose those logs. Which is
> bad, since workdirs should always be safe to delete.
I had to think about the above for a while, but after realizing that you
have a strict distinction between a "workdir" and a normal "repository
with a work tree" in mind, I can see where you are coming from. A workdir
is transient in nature and you should be able to dismiss it safely as long
as the repository it borrows from is intact.
But "safely" is somewhat relative.
What state would you be discarding when you remove a workdir? I can think
of:
- local uncommitted changes, in the work tree contents and in the index
(obviously);
- reflog for the HEAD (aka branch switching);
- what branch you had checked out when you discarded the workdir;
and everything else (commits created, the tips and histories of refs,
configuration changes) are kept in the .git repository of the original.
But what if the original does _not_ want to keep track of changes of
certain nature? It is nonsensical for the original not to want to keep
the commits nor the tips of the refs, but it is not unreasonable for a
bare repository used as a distribution point not want to keep reflogs, for
example. A workdir could be defined as "a transient work tree created on
an existing repository, the side effects of working in which are saved to
the original repository (except for the ones listed above). The kind of
side effects saved are however limited to the ones that are saved while
working in the original repository."
With such a definition, you can "safely" create a workdir out of a bare
repository, without fear of contaminating it with unwanted reflogs.
I tend to think the definition your patch seems to use would be more
useful in practice, though.
A workdir is a new work area that is not a normal "work tree with a
full repository", but borrows from an existing repository. Any side
effect from the work you do in a workdir will be saved in the original
repository, and removing one would lose only the three kind of
information listed above. Creating a new workdir has the side effect
of enabling reflogs and rerere in the original repository.
But the last sentence somehow feels dirty.
^ permalink raw reply
* Re: [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server
From: Junio C Hamano @ 2009-01-18 20:05 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Ray Chuan, git
In-Reply-To: <alpine.DEB.1.00.0901181425420.3586@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> The point is: the repository inside the document root of the web server is
> still a valid repository.
>
> And the assumption is that whenever you have a file that looks like a
> valid object/pack inside a valid repository, that it does not need
> replacing.
>
> So even when optimizing the uncommon (HTTP push is 2nd class citizen), we
> have to keep the common workflow intact (1st class citizens _are_ push by
> file system, ssh or git://).
The first class citizens are "local use", not "copying out of the area
that looks like a repository only to http:// transport users".
> Which unfortunately means that put && move must stay.
I still do not understand why it is unfortunate.
As far as I understand, Ray's issue was that his filesystem did not like
the temporary file name that is used for the initial "put" phase because
it contained a character it did not like (colon, perhaps). Why isn't the
patch about changing _that_ single issue?
^ permalink raw reply
* Re: [JGIT PATCH 8/8] Define a basic merge API, and a two-way tree merge strategy
From: Robin Rosenberg @ 2009-01-18 20:21 UTC (permalink / raw)
To: tomi.pakarinen; +Cc: Shawn O. Pearce, git
In-Reply-To: <f299b4f30901171116y216835c9jc11df2d424ee0377@mail.gmail.com>
lördag 17 januari 2009 20:16:21 skrev Tomi Pakarinen:
> testTrivialTwoWay_disjointhistories() failed because merge strategy
> didn't handle missing base
> version. Am'i right?
Kyllä
Or so it seems. My test cases pass. I'll see if I can come up with somehing nasty.
If not I'll apply
-- robin
^ permalink raw reply
* Re: [PATCH] mergetool: put the cursor on the editable file for Vim
From: Junio C Hamano @ 2009-01-18 20:29 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: markus.heidelberg, git
In-Reply-To: <20090118141854.GA25155@neumann>
SZEDER Gábor <szeder@ira.uka.de> writes:
> On Sat, Jan 17, 2009 at 05:35:22PM -0800, Junio C Hamano wrote:
>> Markus Heidelberg <markus.heidelberg@web.de> writes:
>>
>> > When resolving conflicts, you only need to edit the $MERGED file. Put
>> > the cursor automatically into its window for vimdiff and gvimdiff to
>> > avoid doing <C-w>l every time.
>>
>> I think this is sensible.
>>
>> I do not use vim, and I do not know if the patch does what it claims to
>> do, though.
>
> It does.
>
> Tested-by: SZEDER Gábor <szeder@ira.uka.de>
Thanks, both.
^ permalink raw reply
* [PATCH 1/2] handle color.ui at a central place
From: Markus Heidelberg @ 2009-01-18 20:37 UTC (permalink / raw)
To: Jeff King; +Cc: René Scharfe, Junio C Hamano, git
In-Reply-To: <20090118195342.GA612@coredump.intra.peff.net>
Jeff King, 18.01.2009:
> On Sun, Jan 18, 2009 at 02:43:28PM -0500, Jeff King wrote:
>
> > Here is a patch that seems to work. It predicates pretty format colors
> > on diff colors, which is the same way the yellow commit header works in
> > log-tree. I don't know if it makes more sense to introduce yet another
> > color config option.
> >
> > And I say "seems to work" because I remember there being some trickery
> > with color flags sometimes not getting set properly. However, since this
> > is the same flag as the yellow commit header, and called around the same
> > time, I think it should be fine.
>
> Hrm. OK, it doesn't actually work always. It does for git-log, but not
> for rev-list, which leaves diff_use_color_default as -1. I don't know if
> there are any other ways you can get to this code path without having
> set diff_use_color_default.
>
> Maybe it is time to do a cleanup on the color handling, which has
> provided no end of these bugs. I will have to leave that for another
> day, though.
Not sure, if you it has something to do with the following, but I had
this in my tree for some days now, waiting for the 2 commits mentioned
in the log message to graduate to master, which happend just an hour or
so ago.
And a good opportunity to test the 8< scissors :)
-- 8< --
The color.ui variable had to be evaluated in the commands that use
colors. This could lead to missing colors, if it was forgotten for
certain git commands. Centralizing the handling of color.ui for branch,
diff and status color avoids this problem and also reduces code
duplication.
See commit 3f4b609 (git-commit: color status output when color.ui is set,
2009-01-08) and commit 38920dd (git-status -v: color diff output when
color.ui is set, 2009-01-08) for fixes of these bugs.
There is a fourth variable color.interactive, but this is currently only
used in git-add-interactive.perl, so there is no need for handling this
as well, so far.
Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
builtin-branch.c | 4 ----
builtin-commit.c | 9 ---------
builtin-diff.c | 3 ---
builtin-log.c | 12 ------------
builtin-merge.c | 4 ----
color.c | 16 ++++++++++++++++
color.h | 2 ++
config.c | 9 +++++++--
8 files changed, 25 insertions(+), 34 deletions(-)
diff --git a/builtin-branch.c b/builtin-branch.c
index 02fa38f..6aa329b 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -30,7 +30,6 @@ static const char * const builtin_branch_usage[] = {
static const char *head;
static unsigned char head_sha1[20];
-static int branch_use_color = -1;
static char branch_colors[][COLOR_MAXLEN] = {
"\033[m", /* reset */
"", /* PLAIN (normal) */
@@ -553,9 +552,6 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
git_config(git_branch_config, NULL);
- if (branch_use_color == -1)
- branch_use_color = git_use_color_default;
-
track = git_branch_track;
head = resolve_ref("HEAD", head_sha1, 0, NULL);
diff --git a/builtin-commit.c b/builtin-commit.c
index 2f0b00a..5cac034 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -862,12 +862,6 @@ int cmd_status(int argc, const char **argv, const char *prefix)
git_config(git_status_config, NULL);
- if (wt_status_use_color == -1)
- wt_status_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);
index_file = prepare_index(argc, argv, prefix);
@@ -947,9 +941,6 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
git_config(git_commit_config, NULL);
- if (wt_status_use_color == -1)
- wt_status_use_color = git_use_color_default;
-
argc = parse_and_validate_options(argc, argv, builtin_commit_usage, prefix);
index_file = prepare_index(argc, argv, prefix);
diff --git a/builtin-diff.c b/builtin-diff.c
index d75d69b..d8645cf 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -279,9 +279,6 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
prefix = setup_git_directory_gently(&nongit);
git_config(git_diff_ui_config, NULL);
- if (diff_use_color_default == -1)
- diff_use_color_default = git_use_color_default;
-
init_revisions(&rev, prefix);
/* If this is a no-index diff, just run it and exit there. */
diff --git a/builtin-log.c b/builtin-log.c
index c7aa48e..84027d4 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -236,9 +236,6 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)
git_config(git_log_config, NULL);
- if (diff_use_color_default == -1)
- diff_use_color_default = git_use_color_default;
-
init_revisions(&rev, prefix);
rev.diff = 1;
rev.simplify_history = 0;
@@ -303,9 +300,6 @@ int cmd_show(int argc, const char **argv, const char *prefix)
git_config(git_log_config, NULL);
- if (diff_use_color_default == -1)
- diff_use_color_default = git_use_color_default;
-
init_revisions(&rev, prefix);
rev.diff = 1;
rev.combine_merges = 1;
@@ -373,9 +367,6 @@ int cmd_log_reflog(int argc, const char **argv, const char *prefix)
git_config(git_log_config, NULL);
- if (diff_use_color_default == -1)
- diff_use_color_default = git_use_color_default;
-
init_revisions(&rev, prefix);
init_reflog_walk(&rev.reflog_info);
rev.abbrev_commit = 1;
@@ -406,9 +397,6 @@ int cmd_log(int argc, const char **argv, const char *prefix)
git_config(git_log_config, NULL);
- if (diff_use_color_default == -1)
- diff_use_color_default = git_use_color_default;
-
init_revisions(&rev, prefix);
rev.always_show_header = 1;
cmd_log_init(argc, argv, prefix, &rev);
diff --git a/builtin-merge.c b/builtin-merge.c
index cf86975..8e726da 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -872,10 +872,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
git_config(git_merge_config, NULL);
- /* for color.ui */
- if (diff_use_color_default == -1)
- diff_use_color_default = git_use_color_default;
-
argc = parse_options(argc, argv, builtin_merge_options,
builtin_merge_usage, 0);
if (verbosity < 0)
diff --git a/color.c b/color.c
index fc0b72a..7a8bf6e 100644
--- a/color.c
+++ b/color.c
@@ -1,9 +1,12 @@
#include "cache.h"
#include "color.h"
+#include "diff.h"
+#include "wt-status.h"
#define COLOR_RESET "\033[m"
int git_use_color_default = 0;
+int branch_use_color = -1;
static int parse_color(const char *name, int len)
{
@@ -155,6 +158,19 @@ int git_color_default_config(const char *var, const char *value, void *cb)
return git_default_config(var, value, cb);
}
+void git_finish_color_config(void)
+{
+ if (git_use_color_default) {
+ /* Enable colors, if undefined in the config files */
+ if (branch_use_color == -1)
+ branch_use_color = git_use_color_default;
+ if (diff_use_color_default == -1)
+ diff_use_color_default = git_use_color_default;
+ if (wt_status_use_color == -1)
+ wt_status_use_color = git_use_color_default;
+ }
+}
+
static int color_vfprintf(FILE *fp, const char *color, const char *fmt,
va_list args, const char *trail)
{
diff --git a/color.h b/color.h
index 6cf5c88..6924848 100644
--- a/color.h
+++ b/color.h
@@ -9,12 +9,14 @@
*/
extern int git_use_color_default;
+extern int branch_use_color;
/*
* Use this instead of git_default_config if you need the value of color.ui.
*/
int git_color_default_config(const char *var, const char *value, void *cb);
+void git_finish_color_config(void);
int git_config_colorbool(const char *var, const char *value, int stdout_is_tty);
void color_parse(const char *var, const char *value, char *dst);
int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);
diff --git a/config.c b/config.c
index 790405a..e35afbc 100644
--- a/config.c
+++ b/config.c
@@ -7,6 +7,7 @@
*/
#include "cache.h"
#include "exec_cmd.h"
+#include "color.h"
#define MAXNAME (256)
@@ -637,8 +638,10 @@ int git_config(config_fn_t fn, void *data)
const char *home = NULL;
/* Setting $GIT_CONFIG makes git read _only_ the given config file. */
- if (config_exclusive_filename)
- return git_config_from_file(fn, config_exclusive_filename, data);
+ if (config_exclusive_filename) {
+ ret += git_config_from_file(fn, config_exclusive_filename, data);
+ goto finish;
+ }
if (git_config_system() && !access(git_etc_gitconfig(), R_OK))
ret += git_config_from_file(fn, git_etc_gitconfig(),
data);
@@ -654,6 +657,8 @@ int git_config(config_fn_t fn, void *data)
repo_config = git_pathdup("config");
ret += git_config_from_file(fn, repo_config, data);
free(repo_config);
+ finish:
+ git_finish_color_config();
return ret;
}
--
1.6.1.208.g3a5f4
^ permalink raw reply related
* [PATCH 2/2] move the color variables to color.c
From: Markus Heidelberg @ 2009-01-18 20:39 UTC (permalink / raw)
To: Jeff King; +Cc: René Scharfe, Junio C Hamano, git
In-Reply-To: <200901182137.16562.markus.heidelberg@web.de>
To be consistent with where the branch color variable is located now,
move the variables for diff and status color to the same place.
Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
---
color.c | 4 ++--
color.h | 2 ++
diff.c | 1 -
diff.h | 1 -
wt-status.c | 1 -
wt-status.h | 1 -
6 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/color.c b/color.c
index 7a8bf6e..b23c39c 100644
--- a/color.c
+++ b/color.c
@@ -1,12 +1,12 @@
#include "cache.h"
#include "color.h"
-#include "diff.h"
-#include "wt-status.h"
#define COLOR_RESET "\033[m"
int git_use_color_default = 0;
int branch_use_color = -1;
+int diff_use_color_default = -1;
+int wt_status_use_color = -1;
static int parse_color(const char *name, int len)
{
diff --git a/color.h b/color.h
index 6924848..3b47d99 100644
--- a/color.h
+++ b/color.h
@@ -10,6 +10,8 @@
extern int git_use_color_default;
extern int branch_use_color;
+extern int diff_use_color_default;
+extern int wt_status_use_color;
/*
* Use this instead of git_default_config if you need the value of color.ui.
diff --git a/diff.c b/diff.c
index d235482..4bd068c 100644
--- a/diff.c
+++ b/diff.c
@@ -22,7 +22,6 @@
static int diff_detect_rename_default;
static int diff_rename_limit_default = 200;
static int diff_suppress_blank_empty;
-int diff_use_color_default = -1;
static const char *external_diff_cmd_cfg;
int diff_auto_refresh_index = 1;
static int diff_mnemonic_prefix;
diff --git a/diff.h b/diff.h
index 4d5a327..f2c9984 100644
--- a/diff.h
+++ b/diff.h
@@ -188,7 +188,6 @@ extern void diff_unmerge(struct diff_options *,
extern int git_diff_basic_config(const char *var, const char *value, void *cb);
extern int git_diff_ui_config(const char *var, const char *value, void *cb);
-extern int diff_use_color_default;
extern void diff_setup(struct diff_options *);
extern int diff_opt_parse(struct diff_options *, const char **, int);
extern int diff_setup_done(struct diff_options *);
diff --git a/wt-status.c b/wt-status.c
index 96ff2f8..5c3742b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -12,7 +12,6 @@
#include "remote.h"
int wt_status_relative_paths = 1;
-int wt_status_use_color = -1;
int wt_status_submodule_summary;
static char wt_status_colors[][COLOR_MAXLEN] = {
"", /* WT_STATUS_HEADER: normal */
diff --git a/wt-status.h b/wt-status.h
index 78add09..6258fd0 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -36,7 +36,6 @@ struct wt_status {
};
int git_status_config(const char *var, const char *value, void *cb);
-extern int wt_status_use_color;
extern int wt_status_relative_paths;
void wt_status_prepare(struct wt_status *s);
void wt_status_print(struct wt_status *s);
--
1.6.1.208.g3a5f4
^ 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