* Re: General support for ! in git-config values
From: Junio C Hamano @ 2012-02-01 22:21 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Jeff King, Git Mailing List
In-Reply-To: <CACBZZX5mX55Rh8b2GYv7wKbCCypCkrn5AiM9BpXydgcYxHWdQA@mail.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>> I.e., everything pertaining to "!" happens after we get the config
>> string. So what is it that you want "git config --with-exec" to do?
>
> I agree that that's how it should work, I just suggested --with-exec
> in case anyone complained about the backwards compatibility issue of
> changing the meaning of "!" for existing configs.
Now you made me utterly confused.
What "backwards compatibility" issue do you have in mind? If I name
myself '!me' with "user.name = !me", do I suddenly break backwards
compatibility of Git with previous versions somehow? If so how?
The --with-exec option you talk about seems to me the option about the
backward compatibility of the _calling script_ of "git config". The old
version of git-blorl script may have used foo.bar as a mere string, but a
new version of it may (optionally) interpret foo.bar's value in a special
way when it begins with a "!", introducing a backward compatibility issues
to git-blorl script, and users who want to take advantage of that feature
may want to run it as "git-blorl --with-exec", and the relevant part of
the "git-blorl" script might look like this:
foo_bar=$(git config foo.bar)
case "$with_exec,$foo_bar" in
yes,\!*)
foo_bar=$(eval ${foo_bar#\!}) ;;
esac
... then do something interesting using $foo_bar ...
^ permalink raw reply
* Request for git-svn man page
From: Dr. Lars Hanke @ 2012-02-01 22:32 UTC (permalink / raw)
To: git
Please add instructions to the git-svn man page concerning cloning a SVN
repositiory through a proxy. In particular make clear that also the SVN
software has to be configured to use the proxy. See e.g.
http://stackoverflow.com/questions/9057111/git-svn-ignores-http-proxy-debian.
I'd prepare a patch, but I have no clue about SVN, so I'm probably not
the best choice for writing a SVN related manual. ;)
Kind regards,
- lars.
^ permalink raw reply
* Re: [PATCH v3 1/2] Factor find_pack_entry()'s core out
From: Junio C Hamano @ 2012-02-01 22:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Pitre, Nguyễn Thái Ngọc Duy, git
In-Reply-To: <7vaa522oum.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Nicolas Pitre <nico@fluxnic.net> writes:
>
>>> +static int find_pack_entry_1(const unsigned char *sha1,
>>> + struct packed_git *p, struct pack_entry *e)
>>
>> This looks all goot but the name. Pretty please, try to find something
>> that is more descriptive than "1". Suggestions:
>> "find_pack_entry_lookup", "find_pack_entry_inner", etc.
>
> Perhaps "find_pack_entry_in_pack(sha1, e, p)"?
> That would go well with the caller "find_pack_entry(sha1, e)".
I amended 1/2 (and adjusted 2/2 to match) to call it fill_pack_entry.
Will push out the result tonight.
^ permalink raw reply
* Re: [RFC/PATCH] add update to branch support for "floating submodules"
From: Jens Lehmann @ 2012-02-01 22:37 UTC (permalink / raw)
To: Phil Hord; +Cc: Junio C Hamano, Leif Gruenwoldt, git
In-Reply-To: <CABURp0pSGGT8eyzNad-dNNx49oioAxOPOf3dmqu7M3fnV+PzdA@mail.gmail.com>
Am 31.01.2012 23:50, schrieb Phil Hord:
> What I mean is that a developer may be completely focused on one
> particular submodule (his domain). He does his work in this module,
> and when it's ready he commits and pushes to the server. 'git status'
> shows him that his directory is clean. But this is only because he
> doesn't really know where the submodules top-directories are, so he
> doesn't realize that he has changes in another submodule that he has
> not committed. He has to know to run 'git status' from somewhere in
> the superproject (ostensibly in the root directory of that
> superproject). But he may forget since 'git status' already assured
> him he was done.
<snip>
> I guess what would help here is something like the opposite of 'git
> status' showing the status of descendant submodules; it would help if
> it showed the status of sibling submodules and the superproject as
> well.
Hmm, I really think the fact that submodules are unaware that they
are part of a superproject is a feature. I'd prefer seeing that kind
of problem being tackled by the CI server and/or user education. Or
maybe a pre-commit hook which issues a warning in that case?
^ permalink raw reply
* Re: Bug report: stash in upstream caused remote fetch to fail
From: Andrew Walrond @ 2012-02-01 22:37 UTC (permalink / raw)
To: Thomas Rast; +Cc: git
In-Reply-To: <874nvap9hj.fsf@thomas.inf.ethz.ch>
On Wed, Feb 01, 2012 at 09:46:48PM +0100, Thomas Rast wrote:
>
> Do you, by any chance, still have a copy of the upstream repo before you
> trashed the stash? It would be interesting to know whether there was
> actually some repository corruption going on (that went unnoticed by
> fsck, no less) or if there was a bug in the transmission.
I tried to reproduce the problem but without success :(
Disk corruption is unlikely - raided drives etc etc. More likely stash
related somehow as I was applying patches manually on older versions
checked out in a temp branch, stashing the results then applying the
stash onto master/HEAD.
But why was a remote clone involved with the upstream stash at all I
wonder??
Andrew Walrond
^ permalink raw reply
* [RFC PATCH] gitweb: use CGI with -utf8
From: Michał Kiedrowicz @ 2012-02-01 22:50 UTC (permalink / raw)
To: git; +Cc: Michał Kiedrowicz
I noticed that gitweb tries a lot to properly process UTF-8 data, for
example it prints my name correctly in log and commit information, but
it echos junk in the search field. It looks like:
Michał Kiedrowicz
I don't know CGI well and I never touched gitewb code, but I found this
on http://www.lemoda.net/cgi/perl-unicode/index.html:
use CGI '-utf8';
my $value = params ('input');
I tried it and that fixed my problem. I'm not sure about the
consequences, maybe someone more experienced in CGI might help?
---
gitweb/gitweb.perl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index abb5a79..74d45b1 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -10,7 +10,7 @@
use 5.008;
use strict;
use warnings;
-use CGI qw(:standard :escapeHTML -nosticky);
+use CGI qw(:standard :escapeHTML -nosticky -utf8);
use CGI::Util qw(unescape);
use CGI::Carp qw(fatalsToBrowser set_message);
use Encode;
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH] Don't search files with an unset "grep" attribute
From: Jeff King @ 2012-02-01 23:20 UTC (permalink / raw)
To: Conrad Irwin; +Cc: Junio C Hamano, git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <20120201221437.GA19044@sigill.intra.peff.net>
On Wed, Feb 01, 2012 at 05:14:37PM -0500, Jeff King wrote:
> > The first time I introduced this behaviour[1], I made it conditional
> > on a preference — those who wanted "good" grep could set the
> > preference, while those who wanted "fast" grep could not. I think
> > that's not a good idea, though if the performance issues are
> > show-stoppers, I'd suggest the opposite preference (so speed-freaks
> > can disable the checks).
>
> I've been able to get somewhat better performance by hoisting the
> attribute lookup into the parent thread. That means it happens in order
> (which lets the attr code's stack optimizations work), and there's no
> lock contention.
>
> I'll post finished patches with numbers in a few minutes.
OK, here they are. After playing with some options, I'm satisfied this
is a sane way to do it. I don't think it's worth having a config option.
There is a measurable slowdown, but it's simply not that big.
[1/2]: grep: let grep_buffer callers specify a binary flag
[2/2]: grep: respect diff attributes for binary-ness
There are a few optimizations I didn't do that you could put on top:
1. When "-a" is given, we can avoid the attribute lookup altogether.
2. When "-I" is given, we can actually check attributes _before_
loading the file or blob into memory. This can help with very large
binaries.
3. When "-I" is given but we have no attribute, we can stream the
beginning of the file or blob to check for binary-ness, and then
avoid loading the whole thing if it turns out to be binary.
I think (1) and (2) should be easy. Doing (3) is a little messier,
because binary detection happens inside grep_buffer, but we can hoist it
out. However, for large files, it might be nice to have a streaming grep
interface anyway, and (3) could be part of that.
-Peff
^ permalink raw reply
* [PATCH 1/2] grep: let grep_buffer callers specify a binary flag
From: Jeff King @ 2012-02-01 23:21 UTC (permalink / raw)
To: Conrad Irwin; +Cc: Junio C Hamano, git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <20120201221437.GA19044@sigill.intra.peff.net>
The caller of grep_buffer may have extra information about
whether a buffer is binary or not (e.g., from configuration).
Let's give them a chance to pass along that information and
override our binary auto-detection.
Callers can still pass "-1" to get the regular
auto-detection (and all callers are converted to do this,
meaning there should be no behavior change yet).
We could maintain source compatibility for callers by adding
a new "grep_buffer_with_flags" and leaving "grep_buffer" as
a wrapper that always passes "-1". But there are only 5
callers of grep_buffer, and only 1 of those (grepping commit
buffers) will not be converted to pass something useful in
the next patch. So it's simpler to just add a "-1" there.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/grep.c | 8 ++++----
grep.c | 23 ++++++++++++++++-------
grep.h | 2 +-
revision.c | 1 +
4 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 9ce064a..e328316 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -221,14 +221,14 @@ static void *run(void *arg)
void* data = load_sha1(w->identifier, &sz, w->name);
if (data) {
- hit |= grep_buffer(opt, w->name, data, sz);
+ hit |= grep_buffer(opt, w->name, -1, data, sz);
free(data);
}
} else if (w->type == WORK_FILE) {
size_t sz;
void* data = load_file(w->identifier, &sz);
if (data) {
- hit |= grep_buffer(opt, w->name, data, sz);
+ hit |= grep_buffer(opt, w->name, -1, data, sz);
free(data);
}
} else {
@@ -421,7 +421,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
if (!data)
hit = 0;
else
- hit = grep_buffer(opt, name, data, sz);
+ hit = grep_buffer(opt, name, -1, data, sz);
free(data);
free(name);
@@ -483,7 +483,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)
if (!data)
hit = 0;
else
- hit = grep_buffer(opt, name, data, sz);
+ hit = grep_buffer(opt, name, -1, data, sz);
free(data);
free(name);
diff --git a/grep.c b/grep.c
index 486230b..e547db2 100644
--- a/grep.c
+++ b/grep.c
@@ -983,8 +983,16 @@ static void std_output(struct grep_opt *opt, const void *buf, size_t size)
fwrite(buf, size, 1, stdout);
}
+static int grep_buffer_is_binary(char *buf, unsigned long size, int flag)
+{
+ if (flag == -1)
+ flag = buffer_is_binary(buf, size);
+ return flag;
+}
+
static int grep_buffer_1(struct grep_opt *opt, const char *name,
- char *buf, unsigned long size, int collect_hits)
+ int is_binary, char *buf, unsigned long size,
+ int collect_hits)
{
char *bol = buf;
unsigned long left = size;
@@ -1017,11 +1025,11 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
switch (opt->binary) {
case GREP_BINARY_DEFAULT:
- if (buffer_is_binary(buf, size))
+ if (grep_buffer_is_binary(buf, size, is_binary))
binary_match_only = 1;
break;
case GREP_BINARY_NOMATCH:
- if (buffer_is_binary(buf, size))
+ if (grep_buffer_is_binary(buf, size, is_binary))
return 0; /* Assume unmatch */
break;
case GREP_BINARY_TEXT:
@@ -1182,23 +1190,24 @@ static int chk_hit_marker(struct grep_expr *x)
}
}
-int grep_buffer(struct grep_opt *opt, const char *name, char *buf, unsigned long size)
+int grep_buffer(struct grep_opt *opt, const char *name, int is_binary,
+ char *buf, unsigned long size)
{
/*
* we do not have to do the two-pass grep when we do not check
* buffer-wide "all-match".
*/
if (!opt->all_match)
- return grep_buffer_1(opt, name, buf, size, 0);
+ return grep_buffer_1(opt, name, is_binary, buf, size, 0);
/* Otherwise the toplevel "or" terms hit a bit differently.
* We first clear hit markers from them.
*/
clr_hit_marker(opt->pattern_expression);
- grep_buffer_1(opt, name, buf, size, 1);
+ grep_buffer_1(opt, name, is_binary, buf, size, 1);
if (!chk_hit_marker(opt->pattern_expression))
return 0;
- return grep_buffer_1(opt, name, buf, size, 0);
+ return grep_buffer_1(opt, name, is_binary, buf, size, 0);
}
diff --git a/grep.h b/grep.h
index fb205f3..8447e4c 100644
--- a/grep.h
+++ b/grep.h
@@ -128,7 +128,7 @@ extern void append_grep_pattern(struct grep_opt *opt, const char *pat, const cha
extern void append_header_grep_pattern(struct grep_opt *, enum grep_header_field, const char *);
extern void compile_grep_patterns(struct grep_opt *opt);
extern void free_grep_patterns(struct grep_opt *opt);
-extern int grep_buffer(struct grep_opt *opt, const char *name, char *buf, unsigned long size);
+extern int grep_buffer(struct grep_opt *opt, const char *name, int is_binary, char *buf, unsigned long size);
extern struct grep_opt *grep_opt_dup(const struct grep_opt *opt);
extern int grep_threads_ok(const struct grep_opt *opt);
diff --git a/revision.c b/revision.c
index c97d834..3dcd968 100644
--- a/revision.c
+++ b/revision.c
@@ -2150,6 +2150,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
return 1;
return grep_buffer(&opt->grep_filter,
NULL, /* we say nothing, not even filename */
+ -1,
commit->buffer, strlen(commit->buffer));
}
--
1.7.9.3.gc3fce1.dirty
^ permalink raw reply related
* [PATCH 2/2] grep: respect diff attributes for binary-ness
From: Jeff King @ 2012-02-01 23:21 UTC (permalink / raw)
To: Conrad Irwin; +Cc: Junio C Hamano, git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <20120201221437.GA19044@sigill.intra.peff.net>
There is currently no way for users to tell git-grep that a
particular path is or is not a binary file; instead, grep
always relies on its auto-detection (or the user specifying
"-a" to treat all binary-looking files like text).
This patch teaches git-grep to use the same attribute lookup
that is used by git-diff. We could add a new "grep" flag,
but that is unnecessarily complex and unlikely to be useful.
Despite the name, the "-diff" attribute (or "diff=foo" and
the associated diff.foo.binary config option) are really
about describing the contents of the path. It's simply
historical that diff was the only thing that cared about
these attributes in the past.
And if this simple approach turns out to be insufficient, we
still have a backwards-compatible path forward: we can add a
separate "grep" attribute, and fall back to respecting
"diff" if it is unset.
There are a few things worth nothing about the
implementation.
One is that the attribute lookup happens outside of the
grep.[ch] interface (i.e., outside of grep_buffer). We could
do it at a lower level, which would be slightly more
convenient for callers. However, this interacts badly with
threading. The attribute-lookup code performs best when
lookup order matches the filesystem order (so looking up
"a/b/c" is cheaper if we just looked up "a/b/d" than if we
just did "x/y/z"). Because we issue many simultaneous
requests to grep_buffer, performing the attribute lookup at
that level will cause requests from unrelated paths to
interleave, and we lose the locality that makes the lookup
optimization work.
Instead, in the threaded case we check the attributes as
they are added to the work queue, meaning they are looked up
in the optimal order (in the single threaded case, this is a
non-issue, as we process the files serially in the optimal
order).
Here are a few numbers showing the difference. The first is
a best-of-five time for "git grep foo" on the linux-2.6 repo
before this patch (on a 4-core HT processor, using 8
threads):
real 0m0.306s
user 0m1.512s
sys 0m0.412s
Now here's the time for the same operation with a trial
implementation looking up attributes in grep_buffer,
showing a 31% slowdown:
real 0m0.401s
user 0m1.760s
sys 0m0.636s
And here's the same operation with this patch, with only an
11% slowdown:
real 0m0.339s
user 0m1.444s
sys 0m0.584s
Note that while the percentages are big, the absolute
numbers are pretty small. In particular, this is a very
inexpensive grep to do. A more complex regex should have the
same absolute slowdown from the attribute lookup, but it
would be a much smaller percentage of the total processing
time. So doing it this way is not a huge win, but it does
help on small greps.
The second issue worth noting is that while we do a full
attribute lookup, we pass along only the binary flag to
grep_buffer. When the "-p" flag is given to grep, we will
actually look up the same attributes to find funcname
patterns of matches. We could pass along the pointer to the
userdiff driver for reuse.
However, it's not worth doing this. It clutters the code, as
the driver has to be passed through a large number of helper
functions (and pollutes the grep_buffer interface with
userdiff code). And in my tests, it didn't actually improve
performance. Because we only have to look up the attribute
for a grep hit, in most cases we will only do the funcname
lookup for a small subset of files. The cost of the extra
lookups turns out to be negligible.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/grep.c | 26 ++++++++++++++++++++++----
t/t7008-grep-binary.sh | 24 ++++++++++++++++++++++++
2 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index e328316..bb38804 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -42,6 +42,7 @@ enum work_type {WORK_SHA1, WORK_FILE};
struct work_item {
enum work_type type;
char *name;
+ int is_binary;
/* if type == WORK_SHA1, then 'identifier' is a SHA1,
* otherwise type == WORK_FILE, and 'identifier' is a NUL
@@ -113,6 +114,14 @@ static pthread_cond_t cond_result;
static int skip_first_line;
+static int path_is_binary(const char *path)
+{
+ struct userdiff_driver *drv = userdiff_find_by_path(path);
+ if (drv)
+ return drv->binary;
+ return -1;
+}
+
static void add_work(enum work_type type, char *name, void *id)
{
grep_lock();
@@ -123,6 +132,11 @@ static void add_work(enum work_type type, char *name, void *id)
todo[todo_end].type = type;
todo[todo_end].name = name;
+
+ pthread_mutex_lock(&grep_attr_mutex);
+ todo[todo_end].is_binary = path_is_binary(name);
+ pthread_mutex_unlock(&grep_attr_mutex);
+
todo[todo_end].identifier = id;
todo[todo_end].done = 0;
strbuf_reset(&todo[todo_end].out);
@@ -221,14 +235,16 @@ static void *run(void *arg)
void* data = load_sha1(w->identifier, &sz, w->name);
if (data) {
- hit |= grep_buffer(opt, w->name, -1, data, sz);
+ hit |= grep_buffer(opt, w->name, w->is_binary,
+ data, sz);
free(data);
}
} else if (w->type == WORK_FILE) {
size_t sz;
void* data = load_file(w->identifier, &sz);
if (data) {
- hit |= grep_buffer(opt, w->name, -1, data, sz);
+ hit |= grep_buffer(opt, w->name, w->is_binary,
+ data, sz);
free(data);
}
} else {
@@ -421,7 +437,8 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
if (!data)
hit = 0;
else
- hit = grep_buffer(opt, name, -1, data, sz);
+ hit = grep_buffer(opt, name, path_is_binary(name),
+ data, sz);
free(data);
free(name);
@@ -483,7 +500,8 @@ static int grep_file(struct grep_opt *opt, const char *filename)
if (!data)
hit = 0;
else
- hit = grep_buffer(opt, name, -1, data, sz);
+ hit = grep_buffer(opt, name, path_is_binary(name),
+ data, sz);
free(data);
free(name);
diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh
index 917a264..fd6410f 100755
--- a/t/t7008-grep-binary.sh
+++ b/t/t7008-grep-binary.sh
@@ -99,4 +99,28 @@ test_expect_success 'git grep y<NUL>x a' "
test_must_fail git grep -f f a
"
+test_expect_success 'grep respects binary diff attribute' '
+ echo text >t &&
+ git add t &&
+ echo t:text >expect &&
+ git grep text t >actual &&
+ test_cmp expect actual &&
+ echo "t -diff" >.gitattributes &&
+ echo "Binary file t matches" >expect &&
+ git grep text t >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'grep respects not-binary diff attribute' '
+ echo binQary | q_to_nul >b &&
+ git add b &&
+ echo "Binary file b matches" >expect &&
+ git grep bin b >actual &&
+ test_cmp expect actual &&
+ echo "b diff" >.gitattributes &&
+ echo "b:binQary" >expect &&
+ git grep bin b | nul_to_q >actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.7.9.3.gc3fce1.dirty
^ permalink raw reply related
* Re: [PATCH v2] Use correct grammar in diffstat summary line
From: Jonathan Nieder @ 2012-02-01 23:35 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Junio C Hamano, Ævar Arnfjörð,
Frederik Schwarzer, Brandon Casey
In-Reply-To: <1328100907-20397-1-git-send-email-pclouds@gmail.com>
(dropping Thomas from cc)
Nguyễn Thái Ngọc Duy wrote:
> And this patch's diffstat looks just scary due to test suite's updates.
Not about this patch, but: let's see if we can chisel away at that.
Hidden below are also two minor bug reports about the patch. See (*)
and (**) below.
> --- a/t/t0023-crlf-am.sh
> +++ b/t/t0023-crlf-am.sh
> @@ -12,7 +12,7 @@ Subject: test1
>
> ---
> foo | 1 +
> - 1 files changed, 1 insertions(+), 0 deletions(-)
> + 1 file changed, 1 insertion(+)
> create mode 100644 foo
This patchfile is sample input to "git am", and this patch hunk
changes it to match the modified format-patch output format. But
don't we want "git am" to work with old patches, too? So this hunk is
unnecessary.
[...]
> --- a/t/t1200-tutorial.sh
> +++ b/t/t1200-tutorial.sh
> @@ -156,7 +156,7 @@ Updating VARIABLE..VARIABLE
> FASTFORWARD (no commit created; -m option ignored)
> example | 1 +
> hello | 1 +
> - 2 files changed, 2 insertions(+), 0 deletions(-)
> + 2 files changed, 2 insertions(+)
> EOF
Yes, this one's necessary.
(*) It's reminding us to update the gitcore-tutorial. :)
[...]
> --- a/t/t3300-funny-names.sh
> +++ b/t/t3300-funny-names.sh
> @@ -167,7 +167,7 @@ test_expect_success TABS_IN_FILENAMES 'git diff-tree delete with-funny' \
> test_expect_success TABS_IN_FILENAMES 'setup expect' '
> cat >expected <<\EOF
> "tabs\t,\" (dq) and spaces"
> - 1 files changed, 0 insertions(+), 0 deletions(-)
> + 1 file changed, 0 insertions(+), 0 deletions(-)
> EOF
> '
I'm not sure why this and other tests in the same file use "git apply
--stat" to check their work. The descriptions refer to diff-tree but
the tests themselves use diff-index | apply --stat. Puzzling.
[...]
> --- a/t/t3508-cherry-pick-many-commits.sh
> +++ b/t/t3508-cherry-pick-many-commits.sh
> @@ -38,13 +38,13 @@ test_expect_success 'cherry-pick first..fourth works' '
> cat <<-\EOF >expected &&
> [master OBJID] second
> Author: A U Thor <author@example.com>
> - 1 files changed, 1 insertions(+), 0 deletions(-)
> + 1 file changed, 1 insertion(+)
> [master OBJID] third
> Author: A U Thor <author@example.com>
> - 1 files changed, 1 insertions(+), 0 deletions(-)
> + 1 file changed, 1 insertion(+)
> [master OBJID] fourth
> Author: A U Thor <author@example.com>
> - 1 files changed, 1 insertions(+), 0 deletions(-)
> + 1 file changed, 1 insertion(+)
> EOF
Probably should be split out as a separate test so cherry-pick's
behavior and progress reporting can be tested separately. Aside from
that detail, makes sense.
[...]
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -444,7 +444,7 @@ test_expect_success 'stash show - stashes on stack, stash-like argument' '
> git reset --hard &&
> cat >expected <<-EOF &&
> file | 1 +
> - 1 files changed, 1 insertions(+), 0 deletions(-)
> + 1 file changed, 1 insertion(+)
Hm. "git stash show" defaults to --stat, but these tests are not
really about that. Maybe there should be one test that checks that
the default uses --stat and others could use --numstat or similar?
This will be a problem with GETTEXT_POISON (except your patch bypasses
poison so it got missed --- will send a relevant fix as a reply).
[...]
> +++ b/t/t4013/diff.diff-tree_--cc_--patch-with-stat_--summary_master
> +++ b/t/t4013/diff.diff-tree_--cc_--patch-with-stat_--summary_side
> +++ b/t/t4013/diff.diff-tree_--cc_--patch-with-stat_master
> +++ b/t/t4013/diff.diff-tree_--cc_--stat_--summary_master
[...]
> +++ b/t/t4013/diff.format-patch_--attach_--stdout_initial..side
> +++ b/t/t4013/diff.format-patch_--inline_--stdout_--numbered-files_initial..master
> +++ b/t/t4013/diff.format-patch_--inline_--stdout_--subject-prefix=TESTCASE_initial..master
[...]
Testing the --stat option. Can't really be avoided. (Though maybe
some more modular tests for these combinations can be made some day.)
[...]
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -520,7 +520,7 @@ test_expect_success 'shortlog of cover-letter wraps overly-long onelines' '
> cat > expect << EOF
> ---
> file | 16 ++++++++++++++++
> - 1 files changed, 16 insertions(+), 0 deletions(-)
> + 1 file changed, 16 insertions(+)
Not relevant to the detail being tested, should probably be fuzzed
out.
[...]
> --- a/t/t4030-diff-textconv.sh
> +++ b/t/t4030-diff-textconv.sh
> @@ -86,7 +86,7 @@ test_expect_success 'status -v produces text' '
>
> cat >expect.stat <<'EOF'
> file | Bin 2 -> 4 bytes
> - 1 files changed, 0 insertions(+), 0 deletions(-)
> + 1 file changed, 0 insertions(+), 0 deletions(-)
> EOF
> test_expect_success 'diffstat does not run textconv' '
> echo file diff=fail >.gitattributes &&
Testing diffstat. Would be nice to have an accompanying test of
numstat.
[...]
> --- a/t/t4045-diff-relative.sh
> +++ b/t/t4045-diff-relative.sh
> @@ -33,7 +33,7 @@ check_stat() {
> expect=$1; shift
> cat >expected <<EOF
> $expect | 1 +
> - 1 files changed, 1 insertions(+), 0 deletions(-)
> + 1 file changed, 1 insertion(+)
> EOF
Not the detail being tested, so technically it would be nicer to
fuzz the last line out.
[...]
> --- a/t/t4049-diff-stat-count.sh
> +++ b/t/t4049-diff-stat-count.sh
> @@ -16,7 +16,7 @@ test_expect_success setup '
> cat >expect <<-\EOF
> a | 1 +
> b | 1 +
> - 2 files changed, 2 insertions(+), 0 deletions(-)
> + 2 files changed, 2 insertions(+)
> EOF
> git diff --stat --stat-count=2 >actual &&
> test_cmp expect actual
Testing diffstat. Looks sane.
[...]
> +++ b/t/t4100/t-apply-8.expect
> @@ -1,2 +1,2 @@
> t/t4100-apply-stat.sh | 2 +-
> - 1 files changed, 1 insertions(+), 1 deletions(-)
> + 1 file changed, 1 insertion(+), 1 deletion(-)
[...]
> +++ b/t/t4100/t-apply-9.expect
> @@ -1,2 +1,2 @@
> t/t4100-apply-stat.sh | 2 +-
> - 1 files changed, 1 insertions(+), 1 deletions(-)
> + 1 file changed, 1 insertion(+), 1 deletion(-)
Testing apply --stat. Tests of apply --numstat would be more
interesting, but then we'd still need some tests like these to make
sure --stat is consistent with it, so it still seems sane.
> --- a/t/t5150-request-pull.sh
> +++ b/t/t5150-request-pull.sh
> @@ -95,7 +95,7 @@ test_expect_success 'setup: two scripts for reading pull requests' '
> b
> : diffstat
> n
> - / [0-9]* files changed/ {
> + / [0-9]* files\? changed/ {
Mimicking a human.
(**) Should probably use "*" instead of \? --- \? is a GNU extension,
not a BRE.
[...]
> +++ b/t/t7602-merge-octopus-many.sh
> @@ -57,7 +57,7 @@ Merge made by the 'octopus' strategy.
> c2.c | 1 +
> c3.c | 1 +
> c4.c | 1 +
> - 3 files changed, 3 insertions(+), 0 deletions(-)
> + 3 files changed, 3 insertions(+)
> create mode 100644 c2.c
> create mode 100644 c3.c
> create mode 100644 c4.c
Test is about the "Trying simple merge with" lines, not the final
diffstat summary. Should probably be fuzzed out.
Thanks, that was interesting.
Jonathan
^ permalink raw reply
* Re: [PATCH v3 1/2] Factor find_pack_entry()'s core out
From: Nicolas Pitre @ 2012-02-01 23:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <7v1uqe2ngh.fsf@alter.siamese.dyndns.org>
On Wed, 1 Feb 2012, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Nicolas Pitre <nico@fluxnic.net> writes:
> >
> >>> +static int find_pack_entry_1(const unsigned char *sha1,
> >>> + struct packed_git *p, struct pack_entry *e)
> >>
> >> This looks all goot but the name. Pretty please, try to find something
> >> that is more descriptive than "1". Suggestions:
> >> "find_pack_entry_lookup", "find_pack_entry_inner", etc.
> >
> > Perhaps "find_pack_entry_in_pack(sha1, e, p)"?
> > That would go well with the caller "find_pack_entry(sha1, e)".
>
> I amended 1/2 (and adjusted 2/2 to match) to call it fill_pack_entry.
> Will push out the result tonight.
Looks fine to me.
Nicolas
^ permalink raw reply
* Re: [PATCH] i18n: po for zh_cn
From: Junio C Hamano @ 2012-02-01 23:45 UTC (permalink / raw)
To: Jiang Xin; +Cc: Git List
In-Reply-To: <1328111436-58925-1-git-send-email-worldhello.net@gmail.com>
Jiang Xin <worldhello.net@gmail.com> writes:
> Git can speak Chinese now.
>
> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
> ---
> po/zh_cn.po | 3568 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 个文件被修改, 3568 处添加(+), 0 处删除(-)
> create mode 100644 po/zh_cn.po
I do not mind Chinese in the patch text (i.e. below), but I would have
preferred the above not to be in Chinese, which I do not read---I can
guess what 文件, 添加 and 删除 are, and I can also guess that 个 and 处
are units of counting, but nevertheless...
I haven't decided what to do with this, though. If we are going to get
tons of l10n patches[*1*], it may make sense to have a separate clone of
git.git, with possibly a maintainer other than myself, forked at v1.7.9
and updates _only_ files in po/ hierarchy.
Let me think about how to manage the l10n patches a bit.
Thanks for a patch.
[Footnote]
*1* Ah, another thing, shouldn't the title be labelled as l10n instead of
i18n?
^ permalink raw reply
* Re: [PATCH] i18n: git-commit whence_s "merge/cherry-pick" message
From: Junio C Hamano @ 2012-02-01 23:46 UTC (permalink / raw)
To: Jiang Xin; +Cc: Git List, avarab
In-Reply-To: <1328116830-61290-1-git-send-email-worldhello.net@gmail.com>
Jiang Xin <worldhello.net@gmail.com> writes:
> Mark the "merge/cherry-pick" messages in whence_s for translation.
> These messages returned from whence_s function are used as argument
> to build other messages.
>
> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
> ---
> builtin/commit.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index eba137..663e1 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -202,10 +202,10 @@ static const char *whence_s(void)
> case FROM_COMMIT:
> break;
> case FROM_MERGE:
> - s = "merge";
> + s = _("merge");
> break;
> case FROM_CHERRY_PICK:
> - s = "cherry-pick";
> + s = _("cherry-pick");
> break;
> }
builtin/commit.c: In function 'whence_s':
builtin/commit.c:206: error: assignment discards qualifiers from pointer target type
builtin/commit.c:209: error: assignment discards qualifiers from pointer target type
I'll squash the following in. Thanks.
builtin/commit.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 663e1fd..470b4a4 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -196,7 +196,7 @@ static void determine_whence(struct wt_status *s)
static const char *whence_s(void)
{
- char *s = "";
+ const char *s = "";
switch (whence) {
case FROM_COMMIT:
^ permalink raw reply related
* Re: General support for ! in git-config values
From: demerphq @ 2012-02-02 0:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ævar Arnfjörð, Jeff King, Git Mailing List
In-Reply-To: <7v62fq2o03.fsf@alter.siamese.dyndns.org>
On 1 February 2012 23:21, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> I.e., everything pertaining to "!" happens after we get the config
>>> string. So what is it that you want "git config --with-exec" to do?
>>
>> I agree that that's how it should work, I just suggested --with-exec
>> in case anyone complained about the backwards compatibility issue of
>> changing the meaning of "!" for existing configs.
>
> Now you made me utterly confused.
>
> What "backwards compatibility" issue do you have in mind? If I name
> myself '!me' with "user.name = !me", do I suddenly break backwards
> compatibility of Git with previous versions somehow? If so how?
(I am piping up because Ævar and I discussed this idea earlier today
while we discussed git-deploy)
If Ævar's proposal was implemented without a --with-exec switch then
user.name = !me
would set your name to the result of executing the "me" command, which
likely would have the same result as not having set your name at all,
but presumably with a warning about a bad command.
> The --with-exec option you talk about seems to me the option about the
> backward compatibility of the _calling script_ of "git config".
By backward compatibility he meant that anything not providing the
--with-exec option to git-config would continue to behave as before,
and anything that did would get the new more powerful behavior, where
! at the front of a value signified that it meant a command to be
executed whose output would be used as the value.
I think it would be convenient to be able to enable this in the config
file itself. Consider a bunch of devs share a box. Their full name and
email address can be located via a LDAP lookup. So if we could enable
Ævar's proposal in the config then we can automagically have their
commits have the correct username and email if we were to put in the
/etc/gitconfig something like this:
config.enable_executable_options=true
user.name=!ldap_full_name
user.email=!ldap_email
So now, user.name and user.email don't have to be configured by the
users. They just get an account on the box and all their commits have
the right details, no fuss.
I am aware that there are other ways of solving /this/ particular
problem, so it specifically is not a big deal, but still I think the
idea makes a lot of sense, I can imagine it being used in a lot of
interesting ways that are not obvious right away.
cheers,
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"
^ permalink raw reply
* [RFC/PATCH] i18n: do not define gettext/ngettext in NO_GETTEXT case
From: Jonathan Nieder @ 2012-02-02 0:20 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Junio C Hamano, Ævar Arnfjörð,
Frederik Schwarzer, Brandon Casey
In-Reply-To: <20120201233515.GC29599@burratino>
gettext.h is responsible for defining the _ and Q_ helpers which are
either simple stubs (in the NO_GETTEXT case) or synonyms for the
libintl functions used to translated git output. As an implementation
artifact, ever since the !NO_GETTEXT case was implemented
(v1.7.9-rc0~42^2, 2011-11-18), they have also defined gettext and
ngettext macros in the NO_GETTEXT case:
#ifdef gettext
# undef gettext
#endif
#define gettext(x) (x)
New readers looking at this header might be tempted to use gettext and
ngettext directly, since they are defined in all cases, instead of
using the _ and Q_ wrappers which look like shortcuts.
However gettext and ngettext bypass the GETTEXT_POISON feature. So
any code using them directly will produce perfectly sensible English
output when run in the test suite with GETTEXT_POISON set, instead of
the poison markers that normally would make it easy to catch output
intended for machines that has been marked for translation by mistake.
Avoid the temptation by _not_ providing fallback definitions for
gettext and ngettext ourselves. This way, the header is less
misleading and code that uses gettext et al directly will fail to
compile when NO_GETTEXT is set so we can catch it early.
We also take the opportunity to avoid a little ifdef-ery by splitting
the NO_GETTEXT and !NO_GETTEXT implementations into different headers.
Unfortunately this involves some duplication of code.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jonathan Nieder wrote:
> This will be a problem with GETTEXT_POISON (except your patch bypasses
> poison so it got missed --- will send a relevant fix as a reply).
Here's a rough patch to make that kind of thing easier to catch. It
might make more sense to do
#ifdef NO_GETTEXT
#define git_gettext_(x) (x)
#define git_ngettext_(msgid, plu, n) ...
#else
#define git_gettext_(x) gettext(x)
#define git_ngettext_(msgid, plu, n) ngettext(msgid, plu, n)
#endif
static inline const char *_(const char *msgid)
{
return poisoning() ? "poison!" : git_gettext_(msgid);
}
...
but I didn't think of it until I had already written this patch.
Anyway, this way you get both approaches for easy comparison. ;-)
Makefile | 3 +++
gettext-libintl.h | 26 ++++++++++++++++++++++++++
gettext-noop.h | 30 ++++++++++++++++++++++++++++++
gettext.h | 53 +++--------------------------------------------------
git-compat-util.h | 6 ++++++
5 files changed, 68 insertions(+), 50 deletions(-)
create mode 100644 gettext-libintl.h
create mode 100644 gettext-noop.h
diff --git a/Makefile b/Makefile
index c457c34f..83e8c0f1 100644
--- a/Makefile
+++ b/Makefile
@@ -1525,6 +1525,9 @@ endif
ifdef NO_GETTEXT
BASIC_CFLAGS += -DNO_GETTEXT
USE_GETTEXT_SCHEME ?= fallthrough
+ LIB_H += gettext-noop.h
+else
+ LIB_H += gettext-libintl.h
endif
ifdef NO_STRCASESTR
COMPAT_CFLAGS += -DNO_STRCASESTR
diff --git a/gettext-libintl.h b/gettext-libintl.h
new file mode 100644
index 00000000..c9199703
--- /dev/null
+++ b/gettext-libintl.h
@@ -0,0 +1,26 @@
+#ifndef GETTEXT_LIBINTL_H
+#define GETTEXT_LIBINTL_H
+
+#include <libintl.h>
+
+#define FORMAT_PRESERVING(n) __attribute__((format_arg(n)))
+
+extern void git_setup_gettext(void);
+
+static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
+{
+ return use_gettext_poison() ? "# GETTEXT POISON #" : gettext(msgid);
+}
+
+static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)
+const char *Q_(const char *msgid, const char *plu, unsigned long n)
+{
+ if (use_gettext_poison())
+ return "# GETTEXT POISON #";
+ return ngettext(msgid, plu, n);
+}
+
+/* Mark msgid for translation but do not translate it. */
+#define N_(msgid) msgid
+
+#endif
diff --git a/gettext-noop.h b/gettext-noop.h
new file mode 100644
index 00000000..28843a61
--- /dev/null
+++ b/gettext-noop.h
@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2010-2011 Ævar Arnfjörð Bjarmason
+ *
+ * This is a skeleton no-op implementation of gettext for Git.
+ */
+
+#ifndef GETTEXT_NOOP_H
+#define GETTEXT_NOOP_H
+
+#define FORMAT_PRESERVING(n) __attribute__((format_arg(n)))
+
+static inline void git_setup_gettext(void) { }
+
+static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
+{
+ return use_gettext_poison() ? "# GETTEXT POISON #" : msgid;
+}
+
+static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)
+const char *Q_(const char *msgid, const char *plu, unsigned long n)
+{
+ if (use_gettext_poison())
+ return "# GETTEXT POISON #";
+ return n == 1 ? msgid : plu;
+}
+
+/* Mark msgid for translation but do not translate it. */
+#define N_(msgid) msgid
+
+#endif
diff --git a/gettext.h b/gettext.h
index 57ba8bb0..9bc67243 100644
--- a/gettext.h
+++ b/gettext.h
@@ -1,11 +1,3 @@
-/*
- * Copyright (c) 2010-2011 Ævar Arnfjörð Bjarmason
- *
- * This is a skeleton no-op implementation of gettext for Git.
- * You can replace it with something that uses libintl.h and wraps
- * gettext() to try out the translations.
- */
-
#ifndef GETTEXT_H
#define GETTEXT_H
@@ -13,49 +5,10 @@
#error "namespace conflict: '_' or 'Q_' is pre-defined?"
#endif
-#ifndef NO_GETTEXT
-# include <libintl.h>
+#ifdef NO_GETTEXT
+#include "gettext-noop.h"
#else
-# ifdef gettext
-# undef gettext
-# endif
-# define gettext(s) (s)
-# ifdef ngettext
-# undef ngettext
-# endif
-# define ngettext(s, p, n) ((n == 1) ? (s) : (p))
+#include "gettext-libintl.h"
#endif
-#define FORMAT_PRESERVING(n) __attribute__((format_arg(n)))
-
-#ifndef NO_GETTEXT
-extern void git_setup_gettext(void);
-#else
-static inline void git_setup_gettext(void)
-{
-}
-#endif
-
-#ifdef GETTEXT_POISON
-extern int use_gettext_poison(void);
-#else
-#define use_gettext_poison() 0
-#endif
-
-static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
-{
- return use_gettext_poison() ? "# GETTEXT POISON #" : gettext(msgid);
-}
-
-static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)
-const char *Q_(const char *msgid, const char *plu, unsigned long n)
-{
- if (use_gettext_poison())
- return "# GETTEXT POISON #";
- return ngettext(msgid, plu, n);
-}
-
-/* Mark msgid for translation but do not translate it. */
-#define N_(msgid) msgid
-
#endif
diff --git a/git-compat-util.h b/git-compat-util.h
index 8f3972cd..ca4a4f19 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -236,6 +236,12 @@ extern char *gitbasename(char *);
#endif
#endif
+#ifdef GETTEXT_POISON
+extern int use_gettext_poison(void);
+#else
+#define use_gettext_poison() 0
+#endif
+
#include "compat/bswap.h"
/* General helper functions */
--
1.7.9
^ permalink raw reply related
* Re: General support for ! in git-config values
From: Junio C Hamano @ 2012-02-02 0:27 UTC (permalink / raw)
To: demerphq; +Cc: Ævar Arnfjörð, Jeff King, Git Mailing List
In-Reply-To: <CANgJU+X+UZmycwE6xkJ-zHfT7ai6nV9zbeR4WYnAXczL5JtqjA@mail.gmail.com>
demerphq <demerphq@gmail.com> writes:
> If Ævar's proposal was implemented without a --with-exec switch then
>
> user.name = !me
>
> would set your name to the result of executing the "me" command, which
> likely would have the same result as not having set your name at all,
> but presumably with a warning about a bad command.
>
> user.name=!ldap_full_name
> user.email=!ldap_email
I didn't get the impression that that was what he was proposing, but
regardless of who's suggesting it, the above does not make any sense,
from the syntax point of view.
When you say "alias.co = !git checkout --foo-bar", you are not saying
"instead of usual aliases like 'alias.co = checkout' that defines what
string replaces the 'co' alias, please run !whatever command, capture
its output, and that is the _value_ of alias.co variable, i.e. the command
line that is going to be run".
^ permalink raw reply
* Re: [PATCH v2] Use correct grammar in diffstat summary line
From: Jonathan Nieder @ 2012-02-02 0:27 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Junio C Hamano, Ævar Arnfjörð,
Frederik Schwarzer, Brandon Casey
In-Reply-To: <20120201233515.GC29599@burratino>
Jonathan Nieder wrote:
> [...]
>> --- a/t/t1200-tutorial.sh
>> +++ b/t/t1200-tutorial.sh
>> @@ -156,7 +156,7 @@ Updating VARIABLE..VARIABLE
>> FASTFORWARD (no commit created; -m option ignored)
>> example | 1 +
>> hello | 1 +
>> - 2 files changed, 2 insertions(+), 0 deletions(-)
>> + 2 files changed, 2 insertions(+)
>> EOF
>
> Yes, this one's necessary.
>
> (*) It's reminding us to update the gitcore-tutorial. :)
Patch for squashing (untested) below.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Documentation/gitcore-tutorial.txt | 2 +-
Documentation/gittutorial-2.txt | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/gitcore-tutorial.txt b/Documentation/gitcore-tutorial.txt
index c27d086f..fb0d5692 100644
--- a/Documentation/gitcore-tutorial.txt
+++ b/Documentation/gitcore-tutorial.txt
@@ -1004,7 +1004,7 @@ Updating from ae3a2da... to a80b4aa....
Fast-forward (no commit created; -m option ignored)
example | 1 +
hello | 1 +
- 2 files changed, 2 insertions(+), 0 deletions(-)
+ 2 files changed, 2 insertions(+)
----------------
Because your branch did not contain anything more than what had
diff --git a/Documentation/gittutorial-2.txt b/Documentation/gittutorial-2.txt
index f1e4422a..e00a4d21 100644
--- a/Documentation/gittutorial-2.txt
+++ b/Documentation/gittutorial-2.txt
@@ -34,12 +34,12 @@ $ echo 'hello world' > file.txt
$ git add .
$ git commit -a -m "initial commit"
[master (root-commit) 54196cc] initial commit
- 1 files changed, 1 insertions(+), 0 deletions(-)
+ 1 file changed, 1 insertion(+)
create mode 100644 file.txt
$ echo 'hello world!' >file.txt
$ git commit -a -m "add emphasis"
[master c4d59f3] add emphasis
- 1 files changed, 1 insertions(+), 1 deletions(-)
+ 1 file changed, 1 insertion(+), 1 deletion(-)
------------------------------------------------
What are the 7 digits of hex that git responded to the commit with?
--
1.7.9
^ permalink raw reply related
* Re: [PATCH v2] Use correct grammar in diffstat summary line
From: Jonathan Nieder @ 2012-02-02 0:31 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Junio C Hamano, Ævar Arnfjörð,
Frederik Schwarzer, Brandon Casey
In-Reply-To: <20120201233515.GC29599@burratino>
Jonathan Nieder wrote:
> Nguyễn Thái Ngọc Duy wrote:
>> --- a/t/t5150-request-pull.sh
>> +++ b/t/t5150-request-pull.sh
>> @@ -95,7 +95,7 @@ test_expect_success 'setup: two scripts for reading pull requests' '
>> b
>> : diffstat
>> n
>> - / [0-9]* files changed/ {
>> + / [0-9]* files\? changed/ {
>
> Mimicking a human.
>
> (**) Should probably use "*" instead of \? --- \? is a GNU extension,
> not a BRE.
... and here's a patch making that change for convenience (also untested).
---
t/t5150-request-pull.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 34c482f1..ec5f7368 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -95,7 +95,7 @@ test_expect_success 'setup: two scripts for reading pull requests' '
b
: diffstat
n
- / [0-9]* files\? changed/ {
+ / [0-9]* files* changed/ {
a\\
DIFFSTAT
b
--
1.7.9
^ permalink raw reply related
* Re: [PATCH 1/2] grep: let grep_buffer callers specify a binary flag
From: Junio C Hamano @ 2012-02-02 0:47 UTC (permalink / raw)
To: Jeff King; +Cc: Conrad Irwin, git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <20120201232109.GA2652@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> The caller of grep_buffer may have extra information about
> whether a buffer is binary or not (e.g., from configuration).
> Let's give them a chance to pass along that information and
> override our binary auto-detection.
Hrm, I would have expected a patch that turns "const char *name" into a
structure that has name and drv as its members, so that later we can tell
the function more about the nature of the contents. Or a separate pointer
to drv in place of your "binary" flag word.
I am not saying that your patch is wrong. It was just somewhat unexpected
that "binary" is the only additional thing we want to tell the function.
^ permalink raw reply
* Re: [PATCH 1/2] grep: let grep_buffer callers specify a binary flag
From: Jeff King @ 2012-02-02 0:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Conrad Irwin, git, Nguyen Thai Ngoc Duy, Dov Grobgeld
In-Reply-To: <7vhaza12ol.fsf@alter.siamese.dyndns.org>
On Wed, Feb 01, 2012 at 04:47:38PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > The caller of grep_buffer may have extra information about
> > whether a buffer is binary or not (e.g., from configuration).
> > Let's give them a chance to pass along that information and
> > override our binary auto-detection.
>
> Hrm, I would have expected a patch that turns "const char *name" into a
> structure that has name and drv as its members, so that later we can tell
> the function more about the nature of the contents. Or a separate pointer
> to drv in place of your "binary" flag word.
Hmm. Yeah, I would be OK with that, as it's really encapsulating the
idea of "stuff we want to tell grep about this buffer".
What I really didn't want to do was pass the userdiff driver directly,
as that feels way too much like an implementation detail (and while it
can be used to avoid further lookups, it doesn't seem to make a
difference in practice -- see the following patch). And passing it
around became a big messy chore.
But if it were a "struct grep_context" that encapsulated those things, I
think it would be much nicer (it could even carry a pointer to "struct
grep_opt" in it, making the code _more_ pleasant to read, not less).
I'll take a look at re-working it that way.
-Peff
^ permalink raw reply
* Re: General support for ! in git-config values
From: demerphq @ 2012-02-02 1:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Ævar Arnfjörð, Jeff King, Git Mailing List
In-Reply-To: <7vliom13lm.fsf@alter.siamese.dyndns.org>
On 2 February 2012 01:27, Junio C Hamano <gitster@pobox.com> wrote:
> demerphq <demerphq@gmail.com> writes:
>
>> If Ævar's proposal was implemented without a --with-exec switch then
>>
>> user.name = !me
>>
>> would set your name to the result of executing the "me" command, which
>> likely would have the same result as not having set your name at all,
>> but presumably with a warning about a bad command.
>>
>> user.name=!ldap_full_name
>> user.email=!ldap_email
>
> I didn't get the impression that that was what he was proposing, but
> regardless of who's suggesting it, the above does not make any sense,
> from the syntax point of view.
Does that mean that from a utility point of view it does?
As for the suggestion and who is making it, this came up because Ævar
and I were discussing git-deploy and a scenario much like I described,
where we use tags to mark out rollouts and we have multiple users
using a common staging server and git repo to do the rollouts from, we
want the tags to have proper details, so we have to go through some
hoops to ensure that users have properly configured things before the
do their first rollout. Since Ævar was aware of the "!" notation for
aliases and thought it made some kind of sense to reuse the syntax
elsewhere he decided to see what people thought. I was not aware of
the notation until he pointed it out. I don't use git-alias.
> When you say "alias.co = !git checkout --foo-bar", you are not saying
> "instead of usual aliases like 'alias.co = checkout' that defines what
> string replaces the 'co' alias, please run !whatever command, capture
> its output, and that is the _value_ of alias.co variable, i.e. the command
> line that is going to be run".
I think I see your point. Using the same "!" notation for "execute
this command and exit" and "execute this command and use its result as
the config value" might be confusing.
Do you have a suggestion for a better syntax? Backtick quoting perhaps?
cheers,
yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"
^ permalink raw reply
* [PATCH v3 0/4] completion: couple of cleanups
From: Felipe Contreras @ 2012-02-02 1:15 UTC (permalink / raw)
To: git; +Cc: SZEDER Gábor, Jonathan Nieder, Felipe Contreras
And an improvement for zsh.
Junio: I see you already picked most of them for 'pu', but I've made further changes based on the feedback:
* completion: be nicer with zsh
Improved the code-style
* completion: simplify __gitcomp*
Fix
Improved commit message
Cheers.
Felipe Contreras (4):
completion: be nicer with zsh
completion: simplify __git_remotes
completion: remove unused code
completion: simplify __gitcomp*
contrib/completion/git-completion.bash | 66 +++++---------------------------
1 files changed, 10 insertions(+), 56 deletions(-)
--
1.7.9
^ permalink raw reply
* [PATCH v2] i18n: format_tracking_info "Your branch is behind" message
From: Jiang Xin @ 2012-02-02 1:37 UTC (permalink / raw)
To: Git List, avarab; +Cc: Jiang Xin
Function format_tracking_info in remote.c is called by
wt_status_print_tracking in wt-status.c, which will print
branch tracking message in git-status. git-checkout also
show these messages through it's report_tracking function.
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
remote.c | 48 +++++++++++++++++++++++++++++++++++++-----------
1 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/remote.c b/remote.c
index 73a38..45ac1 100644
--- a/remote.c
+++ b/remote.c
@@ -1572,19 +1572,45 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
base = branch->merge[0]->dst;
base = shorten_unambiguous_ref(base, 0);
if (!num_theirs)
- strbuf_addf(sb, "Your branch is ahead of '%s' "
- "by %d commit%s.\n",
- base, num_ours, (num_ours == 1) ? "" : "s");
+ strbuf_addf(sb,
+ Q_(
+ /* The singular version */
+ "Your branch is ahead of '%s' by %d commit.\n",
+ /* The plural version */
+ "Your branch is ahead of '%s' by %d commits.\n",
+ /* Give ngettext() the count */
+ num_ours),
+ base,
+ num_ours);
else if (!num_ours)
- strbuf_addf(sb, "Your branch is behind '%s' "
- "by %d commit%s, "
- "and can be fast-forwarded.\n",
- base, num_theirs, (num_theirs == 1) ? "" : "s");
+ strbuf_addf(sb,
+ Q_(
+ /* The singular version */
+ "Your branch is behind '%s' by %d commit, "
+ "and can be fast-forwarded.\n",
+ /* The plural version */
+ "Your branch is behind '%s' by %d commits, "
+ "and can be fast-forwarded.\n",
+ /* Give ngettext() the count */
+ num_theirs),
+ base,
+ num_theirs);
else
- strbuf_addf(sb, "Your branch and '%s' have diverged,\n"
- "and have %d and %d different commit(s) each, "
- "respectively.\n",
- base, num_ours, num_theirs);
+ strbuf_addf(sb,
+ Q_(
+ /* The singular version */
+ "Your branch and '%s' have diverged,\n"
+ "and have %d and %d different commit each, "
+ "respectively.\n",
+ /* The plural version */
+ "Your branch and '%s' have diverged,\n"
+ "and have %d and %d different commits each, "
+ "respectively.\n",
+ /* Give ngettext() the count */
+ num_theirs),
+ base,
+ num_ours,
+ num_theirs);
return 1;
}
--
1.7.9.109.gd927b
^ permalink raw reply
* Re: General support for ! in git-config values
From: Junio C Hamano @ 2012-02-02 1:39 UTC (permalink / raw)
To: demerphq; +Cc: Ævar Arnfjörð, Jeff King, Git Mailing List
In-Reply-To: <CANgJU+XQWdFmfmBJ4KX4GBz1a=TrVbp9BphGmhEb5Gphmzogjw@mail.gmail.com>
demerphq <demerphq@gmail.com> writes:
> On 2 February 2012 01:27, Junio C Hamano <gitster@pobox.com> wrote:
>> demerphq <demerphq@gmail.com> writes:
>>
>>> user.name=!ldap_full_name
>>> user.email=!ldap_email
>>
>> I didn't get the impression that that was what he was proposing, but
>> regardless of who's suggesting it, the above does not make any sense,
>> from the syntax point of view.
>
> Does that mean that from a utility point of view it does?
Not really.
I do not think whatever "utility" value outweighs the hassle of having to
think through the ramifications (including but not limited to security) of
running arbitrary user command every time a value is looked up.
^ permalink raw reply
* Re: General support for ! in git-config values
From: Junio C Hamano @ 2012-02-02 1:41 UTC (permalink / raw)
To: demerphq; +Cc: Ævar Arnfjörð, Jeff King, Git Mailing List
In-Reply-To: <CANgJU+XQWdFmfmBJ4KX4GBz1a=TrVbp9BphGmhEb5Gphmzogjw@mail.gmail.com>
demerphq <demerphq@gmail.com> writes:
>> When you say "alias.co = !git checkout --foo-bar", you are not saying
>> "instead of usual aliases like 'alias.co = checkout' that defines what
>> string replaces the 'co' alias, please run !whatever command, capture
>> its output, and that is the _value_ of alias.co variable, i.e. the command
>> line that is going to be run".
>
> I think I see your point. Using the same "!" notation for "execute
> this command and exit" and "execute this command and use its result as
> the config value" might be confusing.
I am afraid that you are not seeing it. In "alias.co = !whatever", bang
does *NOT* mean "execute this command and exit".
^ 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