* Re: [PATCH] diff: Support diff.color-words config option
From: Junio C Hamano @ 2009-01-20 6:59 UTC (permalink / raw)
To: Boyd Stephen Smith Jr.
Cc: Johannes Schindelin, Santi Béjar, Thomas Rast, git,
Teemu Likonen
In-Reply-To: <200901192145.21115.bss@iguanasuicide.net>
"Boyd Stephen Smith Jr." <bss@iguanasuicide.net> writes:
> When diff is invoked with --color-words (w/o =regex), use the regular
> expression the user has configured as diff.color-words.
>
> diff drivers configured via attributes take precedence over the
> diff.color-words setting. If the user wants to change them, they have
> their own configuration variables.
This needs an entry in Documentation/config.txt
None of the existing configuration variables defined use hyphens in
multi-word variable names.
Other than that, I think this is a welcome addition to the suite.
Thanks.
^ permalink raw reply
* Re: [PATCH] Provide pessimistic defaults for cross compilation tests.
From: Ralf Wildenhues @ 2009-01-20 6:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Julius Naperkowski, git
In-Reply-To: <7vab9mpu8w.fsf@gitster.siamese.dyndns.org>
* Junio C Hamano wrote on Tue, Jan 20, 2009 at 03:49:03AM CET:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > How do you deal with the hardcoded limitation that uname_S is defined to
> > be the output of "uname -s" on the _build_ system, and that quite a large
> > part of the Makefile sets variables dependent on this?
Oh, up to now I have blissfully ignored cross-compilation issues in git
outside of configure.ac. :-)
> > IOW are you certain that configure (with your patch) will override _all_
> > uname_S dependent settings?
No, I am certain they won't override them at all.
Fixing Makefile will be more (but independent) work. All I did was get
configure.ac in shape to not error out in the face of cross compilation.
> It may be a valid question but it is not limited to cross compilation, is
> it? The matter is if values the Makefile wants to default to can be
> overriden by whatever is placed in config.mak, and as long as that is Ok
> we won't have a problem with or without use of configure (which is a
> second class citizen).
Yeah, I figured that. I assume it makes little sense to suggest adding
AC_CANONICAL_HOST to configure.ac, letting config.{guess,sub} do their
job, and the user to use "./configure --host=some-value" to specify a
host alias, and then using the computed host triple to decide features,
without the need to modify Makefile or other input files.
See, in a way I come from the GNU world here, and that's what I know
best. Since git does its own setup here, I trust you will invent some
way to solve this.
Thanks,
Ralf
^ permalink raw reply
* Re: [PATCH] Provide pessimistic defaults for cross compilation tests.
From: Junio C Hamano @ 2009-01-20 6:50 UTC (permalink / raw)
To: Ralf Wildenhues; +Cc: Julius Naperkowski, git
In-Reply-To: <20090119203400.GA3539@ins.uni-bonn.de>
Ralf Wildenhues <Ralf.Wildenhues@gmx.de> writes:
> In a cross compile setup, configure tests that run programs
> cannot be executed; in that case, provide pessimistic default
> values.
>
> Bug reported by Julius Naperkowski.
> ---
>
>> I can post a patch to add sane default settings for AC_RUN_IFELSE in
>> cross compile setups, this weekend.
>
> configure.ac | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 363547c..4a208d4 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -360,6 +360,7 @@ AC_RUN_IFELSE(
> else if (strcmp(buf, "12345"))
> return 2;]])],
> [ac_cv_c_c99_format=yes],
> + [ac_cv_c_c99_format=no],
> [ac_cv_c_c99_format=no])
> ])
> if test $ac_cv_c_c99_format = no; then
This one probably is Ok, but...
> @@ -380,6 +381,7 @@ AC_RUN_IFELSE(
> FILE *f = fopen(".", "r");
> return f && fread(&c, 1, 1, f)]])],
> [ac_cv_fread_reads_directories=no],
> + [ac_cv_fread_reads_directories=yes],
> [ac_cv_fread_reads_directories=yes])
> ])
> if test $ac_cv_fread_reads_directories = yes; then
I am not quite sure if this is an improvement ...
> @@ -414,6 +416,7 @@ AC_RUN_IFELSE(
> if (snprintf(buf, 3, "%s", "12345") != 5
> || strcmp(buf, "12")) return 1]])],
> [ac_cv_snprintf_returns_bogus=no],
> + [ac_cv_snprintf_returns_bogus=yes],
> [ac_cv_snprintf_returns_bogus=yes])
> ])
> if test $ac_cv_snprintf_returns_bogus = yes; then
... nor this one.
Is there a way to say something like "I'll autodetect as much as I can
without running tests, but please tell me these characteristics of the
target system manually" and leave the resulting config.mak.autogen in a
shape that will guarantee compilation failure until the missing ones are
supplied by config.mak?
The thing is, I am not convinced that it is desirable to be able to build
a possibly suboptimal binary in a cross compilation environment, without
being told in what aspect of the resulting binary is suboptimal. I'd
rather see a build system that honestly tells me what information it needs
but couldn't find, so that I would know I have a chance to help it.
Of course, suggesting a pessimistic default that can result in suboptimal
but correct result would be a good thing to help the user help the build.
I just think it is a good idea to tell the user we are giving such hint a
bit more loudly to draw attention.
^ permalink raw reply
* Re: [PATCH] contrib: add 'git bpush' to push to bundles
From: Mike Hommey @ 2009-01-20 6:47 UTC (permalink / raw)
To: Santi Béjar; +Cc: git
In-Reply-To: <1232408791-16834-1-git-send-email-santi@agolina.net>
On Tue, Jan 20, 2009 at 12:46:31AM +0100, Santi Béjar wrote:
> 'git bpush' updates the branches in a bundle, while adding the objects
> necessary to complete the given branches. Basically, it is a 'git
> push' for bundles.
I think it'd be better to improve git-push to support that, instead of
adding yet another git command.
Mike
^ permalink raw reply
* Re: [PATCH] Fix naming scheme for configure cache variables.
From: Junio C Hamano @ 2009-01-20 6:30 UTC (permalink / raw)
To: Ralf Wildenhues; +Cc: git
In-Reply-To: <20090119203436.GB3539@ins.uni-bonn.de>
Ralf Wildenhues <Ralf.Wildenhues@gmx.de> writes:
> In order to be cached, configure variables need to contain the
> string '_cv_', and they should begin with a package-specific
> prefix in order to avoid interfering with third-party macros.
> Rename ld_dashr, ld_wl_rpath, ld_rpath to git_cv_ld_dashr etc.
> ---
>
> This avoids a warning with newer autoconf versions about the naming of
> the cache variables, and makes the caching work for them.
Thanks.
We require every patch we accept to be accompanied with a sign-off.
Can you certify that ... [see Documentation/SubmittingPatches and look for
(4) Sign your work] ... please?
^ permalink raw reply
* [PATCH] Introduce for_each_recent_reflog_ent().
From: Junio C Hamano @ 2009-01-20 6:22 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Thomas Rast, git, Johannes Sixt, Johan Herland
In-Reply-To: <alpine.DEB.1.00.0901191331590.3586@pacific.mpi-cbg.de>
This can be used to scan only the last few kilobytes of a reflog, as a
cheap optimization when the data you are looking for is likely to be
found near the end of it. The caller is expected to fall back to the
full scan if that is not the case.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
refs.c | 17 ++++++++++++++++-
refs.h | 1 +
sha1_name.c | 8 +++++++-
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index 33ced65..024211d 100644
--- a/refs.c
+++ b/refs.c
@@ -1453,7 +1453,7 @@ int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned char *
return 1;
}
-int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
+int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long ofs, void *cb_data)
{
const char *logfile;
FILE *logfp;
@@ -1464,6 +1464,16 @@ int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
logfp = fopen(logfile, "r");
if (!logfp)
return -1;
+
+ if (ofs) {
+ struct stat statbuf;
+ if (fstat(fileno(logfp), &statbuf) ||
+ statbuf.st_size < ofs ||
+ fseek(logfp, -ofs, SEEK_END) ||
+ fgets(buf, sizeof(buf), logfp))
+ return -1;
+ }
+
while (fgets(buf, sizeof(buf), logfp)) {
unsigned char osha1[20], nsha1[20];
char *email_end, *message;
@@ -1497,6 +1507,11 @@ int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
return ret;
}
+int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data)
+{
+ return for_each_recent_reflog_ent(ref, fn, 0, cb_data);
+}
+
static int do_for_each_reflog(const char *base, each_ref_fn fn, void *cb_data)
{
DIR *dir = opendir(git_path("logs/%s", base));
diff --git a/refs.h b/refs.h
index 06ad260..3bb529d 100644
--- a/refs.h
+++ b/refs.h
@@ -60,6 +60,7 @@ extern int read_ref_at(const char *ref, unsigned long at_time, int cnt, unsigned
/* iterate over reflog entries */
typedef int each_reflog_ent_fn(unsigned char *osha1, unsigned char *nsha1, const char *, unsigned long, int, const char *, void *);
int for_each_reflog_ent(const char *ref, each_reflog_ent_fn fn, void *cb_data);
+int for_each_recent_reflog_ent(const char *ref, each_reflog_ent_fn fn, long, void *cb_data);
/*
* Calls the specified function for each reflog file until it returns nonzero,
diff --git a/sha1_name.c b/sha1_name.c
index 4c0370b..38c9f1b 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -775,7 +775,13 @@ int interpret_nth_last_branch(const char *name, struct strbuf *buf)
strbuf_init(&cb.buf[i], 20);
cb.cnt = 0;
retval = 0;
- for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+ for_each_recent_reflog_ent("HEAD", grab_nth_branch_switch, 40960, &cb);
+ if (cb.cnt < nth) {
+ cb.cnt = 0;
+ for (i = 0; i < nth; i++)
+ strbuf_release(&cb.buf[i]);
+ for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
+ }
if (cb.cnt < nth)
goto release_return;
i = cb.cnt % nth;
--
1.6.1.267.g11c6e
^ permalink raw reply related
* [PATCH] interpret_nth_last_branch(): plug small memleak
From: Junio C Hamano @ 2009-01-20 6:21 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Thomas Rast, git, Johannes Sixt, Johan Herland
In-Reply-To: <alpine.DEB.1.00.0901191331590.3586@pacific.mpi-cbg.de>
The error return path leaked both cb.buf[] strbuf array itself, and the
strings contained in its elements.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
sha1_name.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index f54b6cb..4c0370b 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -754,7 +754,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
int interpret_nth_last_branch(const char *name, struct strbuf *buf)
{
long nth;
- int i;
+ int i, retval;
struct grab_nth_branch_switch_cbdata cb;
const char *brace;
char *num_end;
@@ -774,17 +774,21 @@ int interpret_nth_last_branch(const char *name, struct strbuf *buf)
for (i = 0; i < nth; i++)
strbuf_init(&cb.buf[i], 20);
cb.cnt = 0;
+ retval = 0;
for_each_reflog_ent("HEAD", grab_nth_branch_switch, &cb);
if (cb.cnt < nth)
- return 0;
+ goto release_return;
i = cb.cnt % nth;
strbuf_reset(buf);
strbuf_add(buf, cb.buf[i].buf, cb.buf[i].len);
+ retval = brace-name+1;
+
+release_return:
for (i = 0; i < nth; i++)
strbuf_release(&cb.buf[i]);
free(cb.buf);
- return brace-name+1;
+ return retval;
}
/*
--
1.6.1.267.g11c6e
^ permalink raw reply related
* Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)
From: Boyd Stephen Smith Jr. @ 2009-01-20 5:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbpu3r745.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1462 bytes --]
On Monday 19 January 2009, Junio C Hamano <gitster@pobox.com> wrote
about 'What's cooking in git.git (Jan 2009, #04; Mon, 19)':
>Here are the topics that have been cooking. Commits prefixed with '-'
> are only in 'pu' while commits prefixed with '+' are in 'next'. The
> ones marked with '.' do not appear in any of the branches, but I am
> still holding onto them.
Is there anywhere you are publishing these refs? Of course, I see the
commits in 'pu', but sometimes I would like to merge something you have
in 'next'/'pu' into a branch based on 'master' or one of my local
branches, and I have to go hunting for the commit SHA.
It's not a big deal: qgit, gitk, and 'git log'+grep all solve the issue
quickly enough, and I don't want to add to your workload. I was just
hoping they were already published and I could simply add a remote to my
config to get them.
Currently, I'm just using:
* remote origin
URL: git://git.kernel.org/pub/scm/git/git.git
Remote branch merged with 'git pull' while on branch master
master
Tracked remote branches
html maint man master next pu todo
and I get this:
$ git pull origin jk/color-parse
fatal: Couldn't find remote ref jk/color-parse
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)
From: Jeff King @ 2009-01-20 4:44 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0901191407470.3586@pacific.mpi-cbg.de>
On Mon, Jan 19, 2009 at 02:08:48PM +0100, Johannes Schindelin wrote:
> > * jk/valgrind (Thu Oct 23 04:30:45 2008 +0000) 2 commits
> > . valgrind: ignore ldso errors
> > . add valgrind support in test scripts
>
> Could you put this in pu, at least, please?
I don't think I've really touched this since it was posted. One of the
things I didn't like about it was that the valgrind wrapper directory
was created in the Makefile. I think creating it inside the trash
directory for each test run that wants to use valgrind makes more sense
(probably as .git/valgrind, which is unlikely to hurt anything but will
stay out of the way of most of the tests).
I doubt I will have the chance to look at it anytime soon, so please
feel free to pick up the topic if you are interested.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)
From: Jeff King @ 2009-01-20 4:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbpu3r745.fsf@gitster.siamese.dyndns.org>
On Mon, Jan 19, 2009 at 01:13:30AM -0800, Junio C Hamano wrote:
> * jk/signal-cleanup (Sun Jan 11 06:36:49 2009 -0500) 3 commits
> - pager: do wait_for_pager on signal death
> - refactor signal handling for cleanup functions
> - chain kill signals for cleanup functions
>
> Sorry, I lost track. What is the status of this one?
I need to clean up and re-send. The three improvements needed are:
- there is a related Windows cleanup from JSixt, which I will send
when I re-post
- the test needs a few tweaks to be portable to Windows
- Some of the signal handlers should be guarded from inserting
themselves multiple times. I don't think any are dangerous to run
twice (they generally traverse a list, cleaning up files, and then
remove the list elements), but I'm not sure that you can't get some
stupid behavior, like inserting one handler per diff'd file, which
will unnecessarily allocate memory.
This series fixes pager handling for interrupted git programs. There is
also a related fix that needs to be done for forked git programs. I
posted a "how about this" patch to use run_command for external git
programs, but it has some serious problems ("git bogus" no longer
reports an error!).
I have unfortunately not had very much git time lately, but I'll try to
come up with something for both cases this week.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #04; Mon, 19)
From: Jeff King @ 2009-01-20 4:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbpu3r745.fsf@gitster.siamese.dyndns.org>
On Mon, Jan 19, 2009 at 01:13:30AM -0800, Junio C Hamano wrote:
> * jk/color-parse (Sat Jan 17 10:38:46 2009 -0500) 2 commits
> + expand --pretty=format color options
> + color: make it easier for non-config to parse color specs
I posted a revised version of 1/2 based on René's work, but it looks
like you have the original. So here it is on top of what's in next.
-- >8 --
From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
optimize color_parse_mem
Commit 5ef8d77a implemented color_parse_mem, a function for
parsing colors from a non-NUL-terminated string, by simply
allocating a new NUL-terminated string and calling
color_parse. This had a small but measurable speed impact on
a user format that used the advanced color parsing. E.g.,
# uses quick parsing
$ time ./git log --pretty=tformat:'%Credfoo%Creset' >/dev/null
real 0m0.673s
user 0m0.652s
sys 0m0.016s
# uses color_parse_mem
$ time ./git log --pretty=tformat:'%C(red)foo%C(reset)' >/dev/null
real 0m0.692s
user 0m0.660s
sys 0m0.032s
This patch implements color_parse_mem as the primary
function, with color_parse as a wrapper for strings. This
gives comparable timings to the first case above.
Original patch by René. Commit message and debugging by Jeff
King.
Signed-off-by: Jeff King <peff@peff.net>
---
color.c | 38 +++++++++++++++++++++-----------------
1 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/color.c b/color.c
index 54a3da1..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 color 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)
@@ -191,10 +202,3 @@ int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...)
va_end(args);
return r;
}
-
-void color_parse_mem(const char *value, int len, const char *var, char *dst)
-{
- char *tmp = xmemdupz(value, len);
- color_parse(tmp, var, dst);
- free(tmp);
-}
--
1.6.1.335.g0366b.dirty
^ permalink raw reply related
* Re: [PATCH] commit: more compact summary and without extra quotes
From: Jeff King @ 2009-01-20 4:14 UTC (permalink / raw)
To: Santi Béjar; +Cc: git
In-Reply-To: <1232405116-2359-1-git-send-email-santi@agolina.net>
On Mon, Jan 19, 2009 at 11:45:16PM +0100, Santi Béjar wrote:
> Original:
> [master]: created d9a5491: "foo:bar"
>
> While with the patch it becomes:
> [master d9a5491] foo:bar
>
> As discussed in the git mailing list:
>
> http://marc.info/?l=git&m=122765031208922&w=2
I agree with Junio that the URL is fine, but it should not _replace_ a
summary of the issue. But as for the patch itself, I think it is
sensible (and I remember wondering at some point what had become of your
proposal, since everybody seemed to like it).
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] expand --pretty=format color options
From: Jeff King @ 2009-01-20 4:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: René Scharfe, Markus Heidelberg, git
In-Reply-To: <7vljt6q4cf.fsf@gitster.siamese.dyndns.org>
On Mon, Jan 19, 2009 at 03:10:56PM -0800, Junio C Hamano wrote:
> > 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.
>
> Yuck, no matter what you do please don't contaminate plumbing with the UI
> color options.
Of course. But the problem is that rev-list is _already_ contaminated by
--pretty=format:%Cred. Or do you mean, you really want rev-list to
unconditionally output color in such a case?
-Peff
^ permalink raw reply
* Re: [PATCH 1/2] handle color.ui at a central place
From: Jeff King @ 2009-01-20 4:04 UTC (permalink / raw)
To: Markus Heidelberg; +Cc: René Scharfe, Junio C Hamano, git
In-Reply-To: <200901182137.16562.markus.heidelberg@web.de>
On Sun, Jan 18, 2009 at 09:37:15PM +0100, Markus Heidelberg wrote:
> 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.
I think this is probably an improvement, but I had in mind something a
little more drastic. Right now we keep munging one variable that is our
current idea of "should we do color" based on multiple config values.
Then you end up with (best case) this "finalize color config", which is
a bit ugly, or (worst case) bugs where the value hasn't always been
properly initialized (or finalized).
So I think it makes more sense to record each config value, and then
check a _function_ that does the right thing. I.e., you end up with
something like:
if (use_color(COLOR_DIFF)) /* or COLOR_BRANCH, etc */
...
int use_color(enum color_type t)
{
enum color_preference preference;
preference = color_config[t];
if (preference == COLOR_UNKNOWN)
preference = color_config[COLOR_UI];
if (preference == COLOR_UNKNOWN)
preference = /* some sane default, possibly command-dependent */
if (preference == COLOR_AUTO)
return pager_in_use() || isatty(1);
if (preference == COLOR_ALWAYS)
return 1;
if (preference == COLOR_NEVER)
return 0;
}
which very clearly expresses the policy being used (and you probably
want to memo-ize either that whole thing, or at least the isatty check
to avoid making repeated system calls).
-Peff
^ permalink raw reply
* Re: [PATCH] commit: more compact summary and without extra quotes
From: Junio C Hamano @ 2009-01-20 3:49 UTC (permalink / raw)
To: Santi Béjar; +Cc: Johannes Schindelin, git
In-Reply-To: <adf1fd3d0901191600k6fd7a364h3e55f9600960df03@mail.gmail.com>
Santi Béjar <santi@agolina.net> writes:
>>> As discussed in the git mailing list:
>>>
>>> http://marc.info/?l=git&m=122765031208922&w=2
>>>
>>> Signed-off-by: Santi Béjar <santi@agolina.net>
>>> ---
>>
>> You really want to move the link after the ---. I often read commit
>> messages off-line, and let me tell you: I am not really happy reading
>> links then. Not at all.
>
> OK, less work for me :-)
I do not mind URL in the commit log; it would help me or other forgetful
people three months from now if we need to go back to the original
discussion.
I however agree with Dscho that it is irritating when somebody uses URL to
avoid summarizing the issue himself. If somebody cares enough and
understands the issues discussed at URL to come up with a patch that is
worth applying, he surely should be able to summarize the primary idea and
issues involved in a few paragraphs.
A summary of discussion in addition to URL would save everybody's time.
^ permalink raw reply
* [PATCH] diff: Support diff.color-words config option
From: Boyd Stephen Smith Jr. @ 2009-01-20 3:45 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Santi Béjar, Thomas Rast, git, Junio C Hamano, Teemu Likonen
In-Reply-To: <200901192017.54163.bss@iguanasuicide.net>
When diff is invoked with --color-words (w/o =regex), use the regular
expression the user has configured as diff.color-words.
diff drivers configured via attributes take precedence over the
diff.color-words setting. If the user wants to change them, they have
their own configuration variables.
Signed-off-by: Boyd Stephen Smith Jr <bss@iguanasuicide.net>
---
On Monday 19 January 2009, "Boyd Stephen Smith Jr." <bss@iguanasuicide.net> wrote about '[PATCH] Add
tests for diff.color-words configuration option.':
>Yes, I also think I'll work on the actual implementation, but I'd be glad
>to have someone beat me to it. I'm not sure why the diff is crazy long.
Here's a patch that makes the added test case succeed, but I think it and
the tests themselves should probably be reworked. Hopefully, this doesn't
show up in quoted-printable format (damn you kmail).
While it might be a corner-case, we probably need a test of some sort for
when a user/system has a global diff.color-words configuration wants
to have a single repository (or single run of 'git diff') use the default
algorithm. I.e. run as if no regex had been set.
diff.c | 5 +++++
t/t4034-diff-words.sh | 2 +-
2 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/diff.c b/diff.c
index 9fcde96..c53e1d1 100644
--- a/diff.c
+++ b/diff.c
@@ -23,6 +23,7 @@ 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 *diff_color_words_cfg = NULL;
static const char *external_diff_cmd_cfg;
int diff_auto_refresh_index = 1;
static int diff_mnemonic_prefix;
@@ -92,6 +93,8 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
}
if (!strcmp(var, "diff.external"))
return git_config_string(&external_diff_cmd_cfg, var, value);
+ if (!strcmp(var, "diff.color-words"))
+ return git_config_string(&diff_color_words_cfg, var, value);
return git_diff_basic_config(var, value, cb);
}
@@ -1550,6 +1553,8 @@ static void builtin_diff(const char *name_a,
o->word_regex = userdiff_word_regex(one);
if (!o->word_regex)
o->word_regex = userdiff_word_regex(two);
+ if (!o->word_regex)
+ o->word_regex = diff_color_words_cfg;
if (o->word_regex) {
ecbdata.diff_words->word_regex = (regex_t *)
xmalloc(sizeof(regex_t));
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 6ebce9d..a207d9e 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -105,7 +105,7 @@ a = b + c<RESET>
EOF
cp expect.non-whitespace-is-word expect
-test_expect_failure 'use default supplied by config' '
+test_expect_success 'use default supplied by config' '
word_diff --color-words
--
1.5.6.5
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
^ permalink raw reply related
* Re: how to track multiple upstreams in one repository
From: Greg KH @ 2009-01-20 3:34 UTC (permalink / raw)
To: Ciprian Dorin, Craciun; +Cc: david, Bryan Donlan, git
In-Reply-To: <8e04b5820901182352n29b3885cj850e6ddae6ca237f@mail.gmail.com>
On Mon, Jan 19, 2009 at 09:52:16AM +0200, Ciprian Dorin, Craciun wrote:
> I use something even simpler, please see the attached .git/config
> file that I use. It also uses remote branches, and rewrites the refs
> to something like: stable/v2.6.25/master or torvalds/v2.6/master. Also
> in order to fetch them I use git fetch stable/v2.6.25
You all do know that all of the -stable trees are automatically kept in
one repo on kernel.org, so you don't have to jump through all of these
hoops, right?
confused,
greg k-h
^ permalink raw reply
* Re: [PATCH,v4] git-checkout(1): mention fate of extraneous files
From: Junio C Hamano @ 2009-01-20 3:35 UTC (permalink / raw)
To: Boyd Stephen Smith Jr.; +Cc: git, Johannes Schindelin, jidanni
In-Reply-To: <200901191854.58029.bss@iguanasuicide.net>
"Boyd Stephen Smith Jr." <bss@iguanasuicide.net> writes:
> I think some users will expect to get a clean checkout when simply
> doing "git checkout <branch>". It would be nice for the documentation
> mention that is not the case, and reference the tool that helps get the
> tree into that state. Just my opinion, though.
>
> It seems natural to me for this to be mentioned in the 'git checkout'
> documentation. Perhaps there's a better place?
I think this is probably on the other side of the borderline.
First I was about to agree with you, but the more I think about it, I do
not think it is natural at all to expect "checkout" to behave as if you
did "rm -fr" everything and then "tar xf" over the void. What other SCM
implements branch switching that way, and what workflow would such a
behaviour help?
We need to draw a line somewhere to avoid cluttering the documentation too
much, and I do not think this is something even a person with CVS
braindamage would get confused about, which is where I think is a
reasonable place to draw that line.
Having said all that, I share a desire to help people who do not have any
prior experience nor knowledge to form a reasonable expectation out of the
system.
For example, a person who does not have a clue on what version control
system is about may think that the contents recorded in a branch is like a
tarball, a version control system is not about helping you make changes
but its _sole_ purpose is to extract one such tarball after another on
demand to let you travel in time. Running an equivalent of "git clean"
automatically upon checkout may feel as if it is a valid a convenience
feature to deal with files that was in one tarball but not in the new one.
It is not completely implausible that such a person may be confused upon
learning that "checkout" leaves untracked paths intact. If you start from
a flawed understanding of what problem the system helps you to solve, you
end up with flawed expectations.
It would not help him if you only taught that checkout leaves untracked
paths intact. You have instead to teach him why you may have, and you
would want to keep, untracked paths in the work tree (i.e. they are build
products and notes you take while developing, iow, files that are
essential during your work, but does not belong to the end product, and
you would want to keep them around even while switching branches because
you may be growing both branches at the same time), and that it is one of
the prime design concern to _any_ command in git not to lose them without
being told. When a user lacks such a basic understanding of the system,
what it was designed for and what it was designed to do, the user's
expectation will never match what many parts of the system do. The user
will stay confused.
I've always thought such basic concepts should be covered by the tutorial
and the users are expected to read them before reading about individual
commands, but that approach may not work in practice. Perhaps a separate
section "Basic Understanding" at the end of each manual page of the
command to cover minimum necessary basics to understand the command might
help. The section may quote from the tutorial, or written afresh.
But I think that such a basic description should be in a separate section
so that it does not clutter the main text for people who understand the
basics. Also I fear there will be quite a lot of repetition (e.g. you may
have to repeat that untracked files are unintersting and that is why the
command does not say anything about them in manual pages for "diff",
"grep", "checkout", etc.). Once it is understood, the user does not need
it to be repeated, but if we want to let the user freely start reading
from anywhere, the repetition cannot be avoided.
^ permalink raw reply
* Re: [PATCH] contrib/difftool: remove distracting 'echo' in the SIGINT handler
From: David Aguilar @ 2009-01-20 3:04 UTC (permalink / raw)
To: markus.heidelberg; +Cc: Junio C Hamano, git
In-Reply-To: <200901200041.18793.markus.heidelberg@web.de>
On Mon, Jan 19, 2009 at 3:41 PM, Markus Heidelberg
<markus.heidelberg@web.de> wrote:
> When interrupting git-difftool with Ctrl-C, the output of this echo
> command led to having the cursor at the beginning of the line below the
> shell prompt.
>
> Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
> ---
>
> David, have you intentionally added this 'echo', did it fix
> anything for you?
I think you're right in that we're better off without the echo. There
wasn't really any reason for it to be there.
>
> contrib/difftool/git-difftool-helper | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/difftool/git-difftool-helper b/contrib/difftool/git-difftool-helper
> index a2eb59b..0c48506 100755
> --- a/contrib/difftool/git-difftool-helper
> +++ b/contrib/difftool/git-difftool-helper
> @@ -32,7 +32,6 @@ cleanup_temp_files () {
>
> # This is called when users Ctrl-C out of git-difftool-helper
> sigint_handler () {
> - echo
> cleanup_temp_files
> exit 1
> }
> --
> 1.6.1.216.g3acd
>
>
--
David
^ permalink raw reply
* Re: [PATCH] Provide pessimistic defaults for cross compilation tests.
From: Junio C Hamano @ 2009-01-20 2:49 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Ralf Wildenhues, Julius Naperkowski, git
In-Reply-To: <alpine.DEB.1.00.0901200037510.3586@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> How do you deal with the hardcoded limitation that uname_S is defined to
> be the output of "uname -s" on the _build_ system, and that quite a large
> part of the Makefile sets variables dependent on this?
>
> IOW are you certain that configure (with your patch) will override _all_
> uname_S dependent settings?
It may be a valid question but it is not limited to cross compilation, is
it? The matter is if values the Makefile wants to default to can be
overriden by whatever is placed in config.mak, and as long as that is Ok
we won't have a problem with or without use of configure (which is a
second class citizen).
^ permalink raw reply
* [PATCH] Add tests for diff.color-words configuration option.
From: Boyd Stephen Smith Jr. @ 2009-01-20 2:17 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Santi Béjar, Thomas Rast, git, Junio C Hamano, Teemu Likonen
In-Reply-To: <alpine.DEB.1.00.0901200031350.3586@pacific.mpi-cbg.de>
Signed-Off-By: Boyd Stephen Smith Jr. <bss@iguanasuicide.net>
---
On Monday 19 January 2009, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote about 'Re: [PATCH v4
0/7] customizable --color-words':
>On Mon, 19 Jan 2009, Santi Béjar wrote:
>> Also, having a config (diff.color-words?) to set the default regexp
>> would be great. Thanks.
>
>From "git log --author==Santi --stat" it seems that you are quite capable
>of providing that patch.
>
>A few pointers:
>
>- Add a test to t4034 that tests that the config sets a default, and that
> the command line can override it.
Here's a couple tests to get someone started, adds one "known breakage" to
the results of the test suite. This is to be applied on top of
the existing patches.
Yes, I also think I'll work on the actual implementation, but I'd be glad
to have someone beat me to it. I'm not sure why the diff is crazy long.
t/t4034-diff-words.sh | 50 +++++++++++++++++++++++++++++++++++-------------
1 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 744221b..6ebce9d 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -63,7 +63,7 @@ test_expect_success 'word diff with runs of whitespace' '
'
-cat > expect <<\EOF
+cat > expect.letter-runs-are-words <<\EOF
<WHITE>diff --git a/pre b/post<RESET>
<WHITE>index 330b04f..5ed8eff 100644<RESET>
<WHITE>--- a/pre<RESET>
@@ -77,6 +77,7 @@ a = b + c<RESET>
<GREEN>aeff = aeff * ( aaa<RESET> )
EOF
+cp expect.letter-runs-are-words expect
test_expect_success 'word diff with a regular expression' '
@@ -84,21 +85,11 @@ test_expect_success 'word diff with a regular expression' '
'
-test_expect_success 'set a diff driver' '
- git config diff.testdriver.wordregex "[^[:space:]]" &&
- cat <<EOF > .gitattributes
-pre diff=testdriver
-post diff=testdriver
-EOF
-'
-
-test_expect_success 'option overrides default' '
-
- word_diff --color-words="[a-z]+"
-
+test_expect_success 'add configuration for default regex' '
+ git config diff.color-words "[^[:space:]]"
'
-cat > expect <<\EOF
+cat > expect.non-whitespace-is-word <<\EOF
<WHITE>diff --git a/pre b/post<RESET>
<WHITE>index 330b04f..5ed8eff 100644<RESET>
<WHITE>--- a/pre<RESET>
@@ -112,6 +103,37 @@ a = b + c<RESET>
<GREEN>aeff = aeff * ( aaa )<RESET>
EOF
+cp expect.non-whitespace-is-word expect
+
+test_expect_failure 'use default supplied by config' '
+
+ word_diff --color-words
+
+'
+
+cp expect.letter-runs-are-words expect
+
+test_expect_success 'option overrides config-default' '
+
+ word_diff --color-words="[a-z]+"
+
+'
+
+test_expect_success 'set a diff driver' '
+ git config diff.testdriver.wordregex "[^[:space:]]" &&
+ cat <<EOF > .gitattributes
+pre diff=testdriver
+post diff=testdriver
+EOF
+'
+
+test_expect_success 'option overrides default' '
+
+ word_diff --color-words="[a-z]+"
+
+'
+
+cp expect.non-whitespace-is-word expect
test_expect_success 'use default supplied by driver' '
--
1.5.6.5
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
^ permalink raw reply related
* [PATCH] Makefile: use shell for-loop rather than Make's foreach loop during install
From: Brandon Casey @ 2009-01-20 1:41 UTC (permalink / raw)
To: gitster; +Cc: git, Brandon Casey
The install target uses a foreach loop to generate a single long shell
command line to handle installation of the built-in git commands. The
maximum length of the argument list varies by platform, and this use of
foreach quickly grows the length of the argument list. Current git can
exceed the default maximum argument list length on IRIX 6.5 of 20480
depending on the installation path.
Rather than using make's foreach loop to pre-generate the shell command
line, use a shell for-loop and allow the shell to iterate through each of
the built-in commands.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
Makefile | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index 2b873fa..fa6c51c 100644
--- a/Makefile
+++ b/Makefile
@@ -1441,10 +1441,12 @@ endif
{ $(RM) "$$execdir/git-add$X" && \
ln git-add$X "$$execdir/git-add$X" 2>/dev/null || \
cp git-add$X "$$execdir/git-add$X"; } && \
- { $(foreach p,$(filter-out git-add$X,$(BUILT_INS)), $(RM) "$$execdir/$p" && \
- ln "$$execdir/git-add$X" "$$execdir/$p" 2>/dev/null || \
- ln -s "git-add$X" "$$execdir/$p" 2>/dev/null || \
- cp "$$execdir/git-add$X" "$$execdir/$p" || exit;) } && \
+ { for p in $(filter-out git-add$X,$(BUILT_INS)); do \
+ $(RM) "$$execdir/$$p" && \
+ ln "$$execdir/git-add$X" "$$execdir/$$p" 2>/dev/null || \
+ ln -s "git-add$X" "$$execdir/$$p" 2>/dev/null || \
+ cp "$$execdir/git-add$X" "$$execdir/$$p" || exit; \
+ done } && \
./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
install-doc:
--
1.6.1.239.gcf3bf
^ permalink raw reply related
* [PATCH] Makefile: use shell for-loop rather than Make's foreach loop during install
From: Brandon Casey @ 2009-01-20 1:44 UTC (permalink / raw)
To: gitster; +Cc: git
The install target uses a foreach loop to generate a single long shell
command line to handle installation of the built-in git commands. The
maximum length of the argument list varies by platform, and this use of
foreach quickly grows the length of the argument list. Current git can
exceed the default maximum argument list length on IRIX 6.5 of 20480
depending on the installation path.
Rather than using make's foreach loop to pre-generate the shell command
line, use a shell for-loop and allow the shell to iterate through each of
the built-in commands.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
Makefile | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index 2b873fa..fa6c51c 100644
--- a/Makefile
+++ b/Makefile
@@ -1441,10 +1441,12 @@ endif
{ $(RM) "$$execdir/git-add$X" && \
ln git-add$X "$$execdir/git-add$X" 2>/dev/null || \
cp git-add$X "$$execdir/git-add$X"; } && \
- { $(foreach p,$(filter-out git-add$X,$(BUILT_INS)), $(RM) "$$execdir/$p" && \
- ln "$$execdir/git-add$X" "$$execdir/$p" 2>/dev/null || \
- ln -s "git-add$X" "$$execdir/$p" 2>/dev/null || \
- cp "$$execdir/git-add$X" "$$execdir/$p" || exit;) } && \
+ { for p in $(filter-out git-add$X,$(BUILT_INS)); do \
+ $(RM) "$$execdir/$$p" && \
+ ln "$$execdir/git-add$X" "$$execdir/$$p" 2>/dev/null || \
+ ln -s "git-add$X" "$$execdir/$$p" 2>/dev/null || \
+ cp "$$execdir/git-add$X" "$$execdir/$$p" || exit; \
+ done } && \
./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-add$X"
install-doc:
--
1.6.1.239.gcf3bf
^ permalink raw reply related
* Re: [PATCH] git-svn: fix SVN 1.1.x compatibility
From: Eric Wong @ 2009-01-20 1:23 UTC (permalink / raw)
To: Tom G. Christensen; +Cc: git@vger.kernel.org
In-Reply-To: <4974450D.4090608@statsbiblioteket.dk>
"Tom G. Christensen" <tgc@statsbiblioteket.dk> wrote:
> Eric Wong wrote:
>> The get_log() function in the Perl SVN API introduced the limit
>> parameter in 1.2.0. However, this got discarded in our SVN::Ra
>> compatibility layer when used with SVN 1.1.x. We now emulate
>> the limit functionality in older SVN versions by preventing the
>> original callback from being called if the given limit has been
>> reached. This emulation is less bandwidth efficient, but SVN
>> 1.1.x is becoming rarer now.
>>
>> Additionally, the --limit parameter in svn(1) uses the
>> aforementioned get_log() functionality change in SVN 1.2.x.
>> t9129 no longer depends on --limit to work and instead uses
>> Perl to parse out the commit message.
>>
>> Thanks to Tom G. Christensen for the bug report.
>>
>> Signed-off-by: Eric Wong <normalperson@yhbt.net>
>
> I applied this to 1.6.1 and the testsuite now passes using SVN 1.1.4 on
> RHEL 4/i386 (t9106 still fails on RHEL 4/x86_64).
Any chance I could have access to that RHEL4/x86_64 environment? I'll
try to work around bugs in older SVN as best I can, but can't guarantee
anything.
--
Eric Wong
^ permalink raw reply
* Re: [PATCH,v4] git-checkout(1): mention fate of extraneous files
From: Boyd Stephen Smith Jr. @ 2009-01-20 0:54 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, jidanni
In-Reply-To: <alpine.DEB.1.00.0901200110410.3586@pacific.mpi-cbg.de>
[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]
On Monday 19 January 2009, Johannes Schindelin <Johannes.Schindelin@gmx.de>
wrote about 'Re: [PATCH,v4] git-checkout(1): mention fate of extraneous
files':
>On Tue, 20 Jan 2009, jidanni@jidanni.org wrote:
>> Signed-off-by: jidanni <jidanni@jidanni.org>
>> ---
>> OK thanks Johannes.
>> I'm still worried that there is no exact statement on the fate of the
>> various different classes of files, but OK, moving this to only a SEE
>> ALSO.
>
>You completely misread me. So I will say it out directly: I think no
>patch is needed.
I think some users will expect to get a clean checkout when simply
doing "git checkout <branch>". It would be nice for the documentation
mention that is not the case, and reference the tool that helps get the
tree into that state. Just my opinion, though.
It seems natural to me for this to be mentioned in the 'git checkout'
documentation. Perhaps there's a better place?
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ 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