* Re: git format-patch should honor notes
From: Michael J Gruber @ 2010-12-08 8:20 UTC (permalink / raw)
To: Jeff King; +Cc: Eric Blake, Thomas Rast, Git Mailing List
In-Reply-To: <20101207221151.GC1036@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 07.12.2010 23:11:
> On Tue, Dec 07, 2010 at 02:53:09PM -0700, Eric Blake wrote:
>
>> My workflow is that I post patch series for upstream review via 'git
>> send-email'. Often, that results in feedback that requires me to
>> amend/rebase my series, and post a v2 or v3 of the series. By adding
>> 'git config notes.rewriteRef refs/notes/commits', I can add notes that
>> will carry across my rebase, and remind me what I changed in v2 (for
>> example, git notes add -m 'v2: fix foo, per mail xyz@example.com').
>> This is handy for me, and I think it is also handy for reviewers -
>> someone who took the time to read through v1 should know what I changed
>> in response to their comments, and only have to focus in on commits with
>> changes, rather than on the entire resent series.
>
> Yeah, that is a workflow that some others have mentioned using here,
> too. And I think there is general agreement that notes should go after
> the "---" in format-patch. We just need a working patch.
>
> Thomas posted one in February:
>
> http://article.gmane.org/gmane.comp.version-control.git/140819
>
> But there were some issues and it never got polished. Michael suggested
> that he does something similar here:
>
> http://article.gmane.org/gmane.comp.version-control.git/140819
>
> but there was no indication on whether it happens manually or if he has
> a patch. I don't know if anything else has happened in that area. I'm
> sure if you feel like working on a patch it would be well received.
>
> -Peff
I do it with ":r!git notes show" in vim (after "/---"), which has the
advantage over "format-patch --show-notes" that the notes are not
indented nor preceded by a "Notes:" header. (I wouldn't mind the
latter.) This is comfortable enough to have kept me from writing a patch.
Also, in order to be really useful, I would need a place to store the
cover letter also. I was experimenting a while back with a design for
annotating branchnames which "basically" worked but haven't had time to
really implement it. If I remember correctly, I had to set up some
"bogus" refs to keep my notes from being garbage collected and was still
figuring out the best place to put them. I'll dig it up when I have time to.
Michael
^ permalink raw reply
* Re: diff for deleted file only, when changed to directory
From: Jonathan Nieder @ 2010-12-08 8:21 UTC (permalink / raw)
To: Bert Wesarg; +Cc: Git Mailing List, Nguyễn Thái Ngọc Duy
In-Reply-To: <AANLkTimXOKC_9TJU1foNkTqPHouFPouzZw3XU_9Za=na@mail.gmail.com>
Bert Wesarg wrote:
> git
> diff --cached -- foo shows me the diff for both the file foo and file
> foo/bar.
[...]
> But for the case
> file->directory, I would like to see only the deleted diff, not
> recursing into the directory.
If I understand correctly, this is a documentation bug and feature
request.
The documentation bug: the "git diff" documentation says something
like
git diff [--options] [<tree>[..<tree>]] [--] [<path>...]
but the <path>s after "--" are actually patterns (path specifiers).
See [1] ([RFD] git glossary: define pathspec, 2010-11-29).
The feature request: there is no way to specify an "exact match"
or "negative match" when specifying paths. At least "negative
match" has been suggested before.
[1] http://thread.gmane.org/gmane.comp.version-control.git/162379
^ permalink raw reply
* diff for deleted file only, when changed to directory
From: Bert Wesarg @ 2010-12-08 8:12 UTC (permalink / raw)
To: Git Mailing List
Hi,
when I stage a file deletion (named foo) and than add a new file
(named foo/bar) in a directory with the name of deleted file. git
diff --cached -- foo shows me the diff for both the file foo and file
foo/bar.
Here is a recipe:
echo foo >f
git add f
git commit -mfoo
git rm f
mkdir f
echo bar >f/0
git add f/0
git diff -- f
git diff --cached -- f
gives me this:
diff --git c/f i/f
deleted file mode 100644
index 257cc56..0000000
--- c/f
+++ /dev/null
@@ -1 +0,0 @@
-foo
diff --git c/f/0 i/f/0
new file mode 100644
index 0000000..5716ca5
--- /dev/null
+++ i/f/0
@@ -0,0 +1 @@
+bar
For the other case, when I stage a symlink under the name foo, I can
understand that git diff shows me two diffs. But for the case
file->directory, I would like to see only the deleted diff, not
recursing into the directory.
Bert
^ permalink raw reply related
* Re: [PATCH] git-rm.txt: Fix quoting
From: Michael J Gruber @ 2010-12-08 8:08 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20101207164315.GA21749@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 07.12.2010 17:43:
> On Tue, Dec 07, 2010 at 10:07:11AM +0100, Michael J Gruber wrote:
>
>> Literal " produces typographically incorrect quotations, but "works" in
>> most circumstances. In the subheadings of git-rm.txt, it "works" for the
>> html backend but not for the docbook conversion to nroff: double "" and
>> spurious double spaces appear in the output.
>>
>> Replace "incorrect" quotations by ``correct'' ones, and fix other
>> "quotations" which are really `code fragments`.
>>
>> This should make git-rm.txt "-clean.
>
> Thanks, this is a much better solution than what I posted earlier.
>
> Acked-by: Jeff King <peff@peff.net>
>
>> We still have a lingering inconsistency for denoting code fragments.
>> Single backticks merely are a literal monospaced environment; html outputcolors
>> this, nroff does not indicate it at all. I'm staying consistent with the
>> surrounding text here.
>
> Try setting MAN_BOLD_LITERAL in your config.mak.
Always learning something new about our doc tooclchain... and what a
great name for an option! Although it should be "Man, bold, literally!".
Michael
^ permalink raw reply
* Re: [PATCHv3 5/6] web--browse: use *www-browser if available
From: Giuseppe Bilotta @ 2010-12-08 7:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Christian Couder, Jonathan Nieder
In-Reply-To: <7vaakhnok7.fsf@alter.siamese.dyndns.org>
On Wed, Dec 8, 2010 at 12:36 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
>
>> + # if the linked executable doesn't match a browser name we know about,
>> + # look at the version string
>> +
>> + # even though most browsers (and applications, in fact) will show their
>> + # name and version on the first line of the --version output, this is
>> + # not true in particular for the KDE apps (e.g. konqueror and kfmclient),
>> + # which display their name and version on the LAST line. So we cannot
>> + # clip the version string at the first line when retrieving it. Rather,
>> + # we keep it whole and then limit it when we know what we're dealing with.
>
> I'd be more worried about the ones that do not understand --version and
> spawn a new window.
That's indeed a potential issue (although I _do_ wonder if there are
browsers that act like that; the only one I couldn't try, on Linux,
was dillo).
>> + verstring="$("$testexe" --version 2> /dev/null)"
>> + browser="$(echo "$verstring" | head -n 1 | cut -f1 -d' ' | tr A-Z a-z)"
>> + case "$browser" in
>> + mozilla)
>
> What was the first patch in this series about again ;-)?
Oh my! LOL. Ok, I can fix that 8-D
>> + verstring="$(echo "$verstring" | head -n 1)"
>> + browser="$(echo "$verstring" | cut -f2 -d' ' | tr A-Z a-z)"
>> + ;;
>> + google)
>> + verstring="$(echo "$verstring" | head -n 1)"
>> + browser="google-chrome"
>> + ;;
>> + qt:)
>> + # konqueror, kfmclient or other KDE app
>> + verstring="$(echo "$verstring" | tail -n 1)"
>> + browser="$(echo "$verstring" | cut -f1 -d:)"
>> + ;;
>> + *)
>> + verstring="$(echo "$verstring" | head -n 1)"
>> + ;;
>> +
>> + esac
>> + if valid_tool "$browser" ; then
>> + browser_path="$i"
>> + return 0
>> + fi
>> +
>> + echo >&2 "$testexe (detected as $browser) is not a supported browser, skipping"
>> + browser=""
>> + return 1
>> +}
>
> Sorry, but I simply do not think it is worth this ugliness to get slight
> customization between -new-tab, newTab, and nothingness.
I personally don't have a particular feeling for forcing the opening
in a new tab (and I might have over-engineered it, although it's not
really much more complex than the previous versions of this patchset,
it's just refactored.
If this requirement is relaxed, the patch becomes much simpler, and we
can include direct support for xdg-open, sensible-browser, htlmview &
so forth.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [PATCHv3 4/6] web--browse: better support for chromium
From: Giuseppe Bilotta @ 2010-12-08 7:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Christian Couder, Jonathan Nieder
In-Reply-To: <7vy681m9w5.fsf@alter.siamese.dyndns.org>
On Wed, Dec 8, 2010 at 12:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Up to this point of the series, I think v3 is a regression compared to
> what is queued on 'pu'.
Ouch. Including or excluding this one? (Unless I did a mistake, the
only difference here was the merging of the conditionals when
path-checking chromium)
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [PATCH] imap-send: link against libcrypto for HMAC and others
From: Anders Kaseorg @ 2010-12-08 4:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Diego Elio Pettenò, git
In-Reply-To: <7v39qbtoq9.fsf@alter.siamese.dyndns.org>
On Sun, 5 Dec 2010, Junio C Hamano wrote:
> Anders, wouldn't this be a better fix for NO_OPENSSL build, than reverting
> a fix for an incorrect ld invocation?
It works for me. I agree it isn’t beautiful but I don’t think I have a
better idea at this time.
(It’s worth pointing out that $(OPENSSL_LINK) is duplicated in the
git-imap-send build command, once directly and once indirectly via
$(LIB_4_CRYPTO), but I assume this is intentional for clarity.)
Anders
^ permalink raw reply
* [PATCH v2 4/4] describe: Delay looking up commits until searching for an inexact match
From: Anders Kaseorg @ 2010-12-08 4:46 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, SZEDER Gábor, Kirill Smelkov,
Thomas Rast
In-Reply-To: <alpine.DEB.2.02.1012072341570.23348@dr-wily.mit.edu>
Now that struct commit.util is not used until after we’ve checked that
the argument doesn’t exactly match a tag, we can wait until then to
look up the commits for each tag.
This avoids a lot of I/O on --exact-match queries in repositories with
many tags. For example, ‘git describe --exact-match HEAD’ becomes
about 12 times faster on a cold cache (3.2s instead of 39s) in a
linux-2.6 repository with 2000 packed tags. That’s a huge win for the
interactivity of the __git_ps1 shell prompt helper when on a detached
HEAD.
Signed-off-by: Anders Kaseorg <andersk@ksplice.com>
---
(The benchmark numbers are different because my linux-2.6 repository has
changed; I don’t think this version is actually slower.)
builtin/describe.c | 35 ++++++++++++++++++++---------------
1 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index afc50c0..2633371 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -24,6 +24,7 @@ static int longformat;
static int abbrev = DEFAULT_ABBREV;
static int max_candidates = 10;
struct hash_table names;
+static int have_util;
static const char *pattern;
static int always;
static const char *dirty;
@@ -60,6 +61,15 @@ static inline struct commit_name *find_commit_name(const unsigned char *peeled)
return n;
}
+static int set_util(void *vn)
+{
+ struct commit_name *n = vn;
+ struct commit *c = lookup_commit_reference_gently(n->peeled, 1);
+ if (c)
+ c->util = n;
+ return 0;
+}
+
static int replace_name(struct commit_name *e,
int prio,
const unsigned char *sha1,
@@ -94,18 +104,16 @@ static int replace_name(struct commit_name *e,
}
static void add_to_known_names(const char *path,
- struct commit *commit,
+ const unsigned char *peeled,
int prio,
const unsigned char *sha1)
{
- const unsigned char *peeled = commit->object.sha1;
struct commit_name *e = find_commit_name(peeled);
struct tag *tag = NULL;
if (replace_name(e, prio, sha1, &tag)) {
if (!e) {
void **pos;
e = xmalloc(sizeof(struct commit_name));
- commit->util = e;
hashcpy(e->peeled, peeled);
pos = insert_hash(hash_sha1(peeled), e, &names);
if (pos) {
@@ -126,8 +134,6 @@ static void add_to_known_names(const char *path,
static int get_name(const char *path, const unsigned char *sha1, int flag, void *cb_data)
{
int might_be_tag = !prefixcmp(path, "refs/tags/");
- struct commit *commit;
- struct object *object;
unsigned char peeled[20];
int is_tag, prio;
@@ -135,16 +141,10 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
return 0;
if (!peel_ref(path, peeled) && !is_null_sha1(peeled)) {
- commit = lookup_commit_reference_gently(peeled, 1);
- if (!commit)
- return 0;
- is_tag = !!hashcmp(sha1, commit->object.sha1);
+ is_tag = !!hashcmp(sha1, peeled);
} else {
- commit = lookup_commit_reference_gently(sha1, 1);
- object = parse_object(sha1);
- if (!commit || !object)
- return 0;
- is_tag = object->type == OBJ_TAG;
+ hashcpy(peeled, sha1);
+ is_tag = 0;
}
/* If --all, then any refs are used.
@@ -167,7 +167,7 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
if (!prio)
return 0;
}
- add_to_known_names(all ? path + 5 : path + 10, commit, prio, sha1);
+ add_to_known_names(all ? path + 5 : path + 10, peeled, prio, sha1);
return 0;
}
@@ -284,6 +284,11 @@ static void describe(const char *arg, int last_one)
if (debug)
fprintf(stderr, "searching to describe %s\n", arg);
+ if (!have_util) {
+ for_each_hash(&names, set_util);
+ have_util = 1;
+ }
+
list = NULL;
cmit->object.flags = SEEN;
commit_list_insert(cmit, &list);
--
1.7.3.3
^ permalink raw reply related
* [PATCH v2 3/4] describe: Store commit_names in a hash table by commit SHA1
From: Anders Kaseorg @ 2010-12-08 4:43 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, SZEDER Gábor, Kirill Smelkov,
Thomas Rast
In-Reply-To: <alpine.DEB.2.02.1012072341570.23348@dr-wily.mit.edu>
describe is currently forced to look up the commit at each tag in
order to store the struct commit_name pointers in struct commit.util.
For --exact-match queries, those lookups are wasteful. In preparation
for removing them, put the commit_names into a hash table, indexed by
commit SHA1, that can be used to quickly check for exact matches.
Signed-off-by: Anders Kaseorg <andersk@ksplice.com>
---
builtin/describe.c | 36 +++++++++++++++++++++++++++++++-----
1 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index 5b8461d..afc50c0 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -6,6 +6,7 @@
#include "exec_cmd.h"
#include "parse-options.h"
#include "diff.h"
+#include "hash.h"
#define SEEN (1u<<0)
#define MAX_TAGS (FLAG_BITS - 1)
@@ -22,7 +23,7 @@ static int tags; /* Allow lightweight tags */
static int longformat;
static int abbrev = DEFAULT_ABBREV;
static int max_candidates = 10;
-static int found_names;
+struct hash_table names;
static const char *pattern;
static int always;
static const char *dirty;
@@ -34,6 +35,8 @@ static const char *diff_index_args[] = {
struct commit_name {
+ struct commit_name *next;
+ unsigned char peeled[20];
struct tag *tag;
unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
unsigned name_checked:1;
@@ -44,6 +47,19 @@ static const char *prio_names[] = {
"head", "lightweight", "annotated",
};
+static inline unsigned int hash_sha1(const unsigned char *sha1)
+{
+ return *(unsigned int *)sha1;
+}
+
+static inline struct commit_name *find_commit_name(const unsigned char *peeled)
+{
+ struct commit_name *n = lookup_hash(hash_sha1(peeled), &names);
+ while (n && !!hashcmp(peeled, n->peeled))
+ n = n->next;
+ return n;
+}
+
static int replace_name(struct commit_name *e,
int prio,
const unsigned char *sha1,
@@ -82,12 +98,22 @@ static void add_to_known_names(const char *path,
int prio,
const unsigned char *sha1)
{
- struct commit_name *e = commit->util;
+ const unsigned char *peeled = commit->object.sha1;
+ struct commit_name *e = find_commit_name(peeled);
struct tag *tag = NULL;
if (replace_name(e, prio, sha1, &tag)) {
if (!e) {
+ void **pos;
e = xmalloc(sizeof(struct commit_name));
commit->util = e;
+ hashcpy(e->peeled, peeled);
+ pos = insert_hash(hash_sha1(peeled), e, &names);
+ if (pos) {
+ e->next = *pos;
+ *pos = e;
+ } else {
+ e->next = NULL;
+ }
}
e->tag = tag;
e->prio = prio;
@@ -95,7 +121,6 @@ static void add_to_known_names(const char *path,
hashcpy(e->sha1, sha1);
e->path = path;
}
- found_names = 1;
}
static int get_name(const char *path, const unsigned char *sha1, int flag, void *cb_data)
@@ -240,7 +265,7 @@ static void describe(const char *arg, int last_one)
if (!cmit)
die("%s is not a valid '%s' object", arg, commit_type);
- n = cmit->util;
+ n = find_commit_name(cmit->object.sha1);
if (n && (tags || all || n->prio == 2)) {
/*
* Exact match to an existing ref.
@@ -418,8 +443,9 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
return cmd_name_rev(i + argc, args, prefix);
}
+ init_hash(&names);
for_each_rawref(get_name, NULL);
- if (!found_names && !always)
+ if (!names.nr && !always)
die("No names found, cannot describe anything.");
if (argc == 0) {
--
1.7.3.3
^ permalink raw reply related
* [PATCH v2 2/4] describe: Don’t use a flex array in struct commit_name
From: Anders Kaseorg @ 2010-12-08 4:43 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, SZEDER Gábor, Kirill Smelkov,
Thomas Rast
In-Reply-To: <alpine.DEB.2.02.1012072341570.23348@dr-wily.mit.edu>
Now add_to_known_names overwrites commit_names in place when multiple
tags point to the same commit. This will make it easier to store
commit_names in a hash table.
Signed-off-by: Anders Kaseorg <andersk@ksplice.com>
---
builtin/describe.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index 700f740..5b8461d 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -38,7 +38,7 @@ struct commit_name {
unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
unsigned name_checked:1;
unsigned char sha1[20];
- char path[FLEX_ARRAY]; /* more */
+ const char *path;
};
static const char *prio_names[] = {
"head", "lightweight", "annotated",
@@ -85,15 +85,15 @@ static void add_to_known_names(const char *path,
struct commit_name *e = commit->util;
struct tag *tag = NULL;
if (replace_name(e, prio, sha1, &tag)) {
- size_t len = strlen(path)+1;
- free(e);
- e = xmalloc(sizeof(struct commit_name) + len);
+ if (!e) {
+ e = xmalloc(sizeof(struct commit_name));
+ commit->util = e;
+ }
e->tag = tag;
e->prio = prio;
e->name_checked = 0;
hashcpy(e->sha1, sha1);
- memcpy(e->path, path, len);
- commit->util = e;
+ e->path = path;
}
found_names = 1;
}
--
1.7.3.3
^ permalink raw reply related
* [PATCH v2 1/4] describe: Use for_each_rawref
From: Anders Kaseorg @ 2010-12-08 4:42 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, SZEDER Gábor, Kirill Smelkov,
Thomas Rast
In-Reply-To: <alpine.DEB.2.02.1012072204371.23348@dr-wily.mit.edu>
Don’t waste time checking for dangling refs; they wouldn’t affect the
output of ‘git describe’ anyway. Although this doesn’t gain much
performance by itself, it does in conjunction with the next commits.
Signed-off-by: Anders Kaseorg <andersk@ksplice.com>
---
builtin/describe.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index 43caff2..700f740 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -418,7 +418,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
return cmd_name_rev(i + argc, args, prefix);
}
- for_each_ref(get_name, NULL);
+ for_each_rawref(get_name, NULL);
if (!found_names && !always)
die("No names found, cannot describe anything.");
--
1.7.3.3
^ permalink raw reply related
* Re: [PATCH] describe: Don’t look up commits with --exact-match
From: Anders Kaseorg @ 2010-12-08 4:41 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, SZEDER Gábor, Kirill Smelkov,
Thomas Rast
In-Reply-To: <7vfwu9qvew.fsf@alter.siamese.dyndns.org>
On Tue, 7 Dec 2010, Junio C Hamano wrote:
> I however think this may hurt when more than one objects are asked to be
> described and there is no exact match.
Yeah, that turns out to be true. So I rewrote the patch to put the
commit_names into a hash table instead of a linked list, run
replace_name() only once per tag, and run the lookup code only once per
describe(). I also tried to split things up a bit more and explain more
in the commit messages.
> * Please run tests before submitting patches. It appears to break t7407.
Oops, I was lazy and only ran the tests that (directly) use describe; I’ll
be more careful in the future. The new series doesn’t have this problem
because it does the replace_name()s in the same order as the old code, and
it passes all tests.
fixed 1
success 6426
failed 0
broken 39
total 6496
Anders
^ permalink raw reply
* Re: how to make a commit only contain existing files
From: Jeff King @ 2010-12-08 3:39 UTC (permalink / raw)
To: david; +Cc: git
In-Reply-To: <alpine.DEB.2.00.1012071652160.662@asgard.lang.hm>
On Tue, Dec 07, 2010 at 04:52:48PM -0800, david@lang.hm wrote:
> >Won't "git add -A; git commit" do what you want?
>
> it very well may, I'll have to upgrade git on that box to a version
> that supports it.
It was introduced in 1.6.0. If you have something older, "git add -u &&
git add ." will do the same thing.
-Peff
^ permalink raw reply
* [PATCH 2/3] diffstat: Use new diffstat config values
From: mmr15 @ 2010-12-08 2:44 UTC (permalink / raw)
To: git; +Cc: Matthew Ruffalo
In-Reply-To: <1291776263-16320-1-git-send-email-matthew.ruffalo@case.edu>
From: Matthew Ruffalo <matthew.ruffalo@case.edu>
Previously, the diffstat width could only be specified with the
command-line options '--width' and '--name-width'. This patch adds
support for config file options 'diffstat.width' and
'diffstat.namewidth'.
The diffstat width values are obtained in this order (of increasing
precedence):
1. Compile-time defaults (80 width, 50 namewidth)
2. Standard git config mechanism
3. Command-line options
This required removing the diffstat options from 'struct diff_options'
and adding these values as static ints in diff.c. This preserves the
style of "config options are static ints, command-line options are
diff_options members". stat_opt now directly sets the global options.
Signed-off-by: Matthew Ruffalo <matthew.ruffalo@case.edu>
---
diff.c | 24 ++++++++++++++++++------
diff.h | 2 --
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/diff.c b/diff.c
index 75938e4..d88c7f4 100644
--- a/diff.c
+++ b/diff.c
@@ -31,6 +31,8 @@ static const char *external_diff_cmd_cfg;
int diff_auto_refresh_index = 1;
static int diff_mnemonic_prefix;
static int diff_no_prefix;
+static int diff_stat_width;
+static int diff_stat_name_width;
static struct diff_options default_diff_options;
static char diff_colors[][COLOR_MAXLEN] = {
@@ -148,6 +150,16 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
if (!prefixcmp(var, "submodule."))
return parse_submodule_config_option(var, value);
+ if (!strcmp(var, "diffstat.width")) {
+ diff_stat_width = git_config_int(var, value);
+ return 0;
+ }
+
+ if (!strcmp(var, "diffstat.namewidth")) {
+ diff_stat_name_width = git_config_int(var, value);
+ return 0;
+ }
+
return git_color_default_config(var, value, cb);
}
@@ -1247,8 +1259,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
line_prefix = msg->buf;
}
- width = options->stat_width ? options->stat_width : DIFF_STAT_DEFAULT_WIDTH;
- name_width = options->stat_name_width ? options->stat_name_width : DIFF_STAT_DEFAULT_NAME_WIDTH;
+ width = diff_stat_width ? diff_stat_width : DIFF_STAT_DEFAULT_WIDTH;
+ name_width = diff_stat_name_width ? diff_stat_name_width : DIFF_STAT_DEFAULT_NAME_WIDTH;
/* Sanity: give at least 5 columns to the graph,
* but leave at least 10 columns for the name.
@@ -3057,8 +3069,8 @@ static int stat_opt(struct diff_options *options, const char **av)
{
const char *arg = av[0];
char *end;
- int width = options->stat_width;
- int name_width = options->stat_name_width;
+ int width = diff_stat_width;
+ int name_width = diff_stat_name_width;
int argcount = 1;
arg += strlen("--stat");
@@ -3098,8 +3110,8 @@ static int stat_opt(struct diff_options *options, const char **av)
if (*end)
return 0;
options->output_format |= DIFF_FORMAT_DIFFSTAT;
- options->stat_name_width = name_width;
- options->stat_width = width;
+ diff_stat_name_width = name_width;
+ diff_stat_width = width;
return argcount;
}
diff --git a/diff.h b/diff.h
index 7b509c5..011f2ac 100644
--- a/diff.h
+++ b/diff.h
@@ -122,8 +122,6 @@ struct diff_options {
const char *stat_sep;
long xdl_opts;
- int stat_width;
- int stat_name_width;
const char *word_regex;
enum diff_words_type word_diff;
--
1.7.1
^ permalink raw reply related
* [PATCH 3/3] Add documentation for new diffstat config options
From: mmr15 @ 2010-12-08 2:44 UTC (permalink / raw)
To: git; +Cc: Matthew Ruffalo
In-Reply-To: <1291776263-16320-1-git-send-email-matthew.ruffalo@case.edu>
From: Matthew Ruffalo <matthew.ruffalo@case.edu>
Signed-off-by: Matthew Ruffalo <matthew.ruffalo@case.edu>
---
Documentation/config.txt | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index ad5eb5f..5cae0f4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -862,6 +862,15 @@ diff.ignoreSubmodules::
commands such as 'git diff-files'. 'git checkout' also honors
this setting when reporting uncommitted changes.
+diffstat.width::
+ Controls the default width of 'git diff --stat' output. Can be
+ overridden with the command line option '--stat-width'.
+
+diffstat.namewidth::
+ Controls the default width of the filenames in 'git diff --stat'
+ output. Can be overridden with the command line option
+ '--stat-namewidth'.
+
diff.suppressBlankEmpty::
A boolean to inhibit the standard behavior of printing a space
before each empty output line. Defaults to false.
--
1.7.1
^ permalink raw reply related
* [PATCH 1/3] diffstat width: #define defaults in diff.h
From: mmr15 @ 2010-12-08 2:44 UTC (permalink / raw)
To: git; +Cc: Matthew Ruffalo
In-Reply-To: <7vzksr280s.fsf@alter.siamese.dyndns.org>
From: Matthew Ruffalo <matthew.ruffalo@case.edu>
Signed-off-by: Matthew Ruffalo <matthew.ruffalo@case.edu>
---
diff.c | 4 ++--
diff.h | 3 +++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index db5e844..75938e4 100644
--- a/diff.c
+++ b/diff.c
@@ -1247,8 +1247,8 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
line_prefix = msg->buf;
}
- width = options->stat_width ? options->stat_width : 80;
- name_width = options->stat_name_width ? options->stat_name_width : 50;
+ width = options->stat_width ? options->stat_width : DIFF_STAT_DEFAULT_WIDTH;
+ name_width = options->stat_name_width ? options->stat_name_width : DIFF_STAT_DEFAULT_NAME_WIDTH;
/* Sanity: give at least 5 columns to the graph,
* but leave at least 10 columns for the name.
diff --git a/diff.h b/diff.h
index 0083d92..7b509c5 100644
--- a/diff.h
+++ b/diff.h
@@ -86,6 +86,9 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
#define DIFF_XDL_SET(opts, flag) ((opts)->xdl_opts |= XDF_##flag)
#define DIFF_XDL_CLR(opts, flag) ((opts)->xdl_opts &= ~XDF_##flag)
+#define DIFF_STAT_DEFAULT_WIDTH 80
+#define DIFF_STAT_DEFAULT_NAME_WIDTH 50
+
enum diff_words_type {
DIFF_WORDS_NONE = 0,
DIFF_WORDS_PORCELAIN,
--
1.7.1
^ permalink raw reply related
* Tonight's pushout
From: Junio C Hamano @ 2010-12-08 2:24 UTC (permalink / raw)
To: git
All four main integration branches in tonight's pushout will pass tests if
you drop three-tip commits from 'pu'. That might be just showing that
there are some caps in the tests, but I am somewhat happier than usual.
Tomorrow, I hopefully will start moving stuff out of 'next' to 'master',
and 'pu' to 'next', in preparation for 1.7.4 prerelease freeze.
^ permalink raw reply
* Re: [PATCH] imap-send: link against libcrypto for HMAC and others
From: Junio C Hamano @ 2010-12-08 2:12 UTC (permalink / raw)
To: Jared Hance; +Cc: Diego Elio Pettenò, Anders Kaseorg, git
In-Reply-To: <20101208015144.GA4868@localhost.localdomain>
Jared Hance <jaredhance@gmail.com> writes:
>> You are both correct; the point of NO_OPENSSL is not to link with anything
>> from openssl suite, so we need a separate mechanism to address this.
>>
>> Anders, wouldn't this be a better fix for NO_OPENSSL build, than reverting
>> a fix for an incorrect ld invocation?
>
> Could we get this fixup patch into master? Leaving the original patch
> in without it doesn't seem like a good idea when it breaks the build.
That depends on what Diego and Anders would say/report, and that is why
their address were on To/Cc in the message you are replying to. Why did
you cull Cc from your message?
One thing I do not like about it is that the "fixup" is ugly. It makes
people expect to add $(LIB_4_CRYPTO) on their linker command line to get a
correct linking with -lcrypto library, when all it does is _not_ to link
with -lcrypto at all, so whatever program that uses it needs to know about
NO_OPENSSL and refrain from using the symbols from that library.
^ permalink raw reply
* Re: [PATCH] Improved error messages when temporary file creation fails
From: Junio C Hamano @ 2010-12-08 2:01 UTC (permalink / raw)
To: Arnout Engelen; +Cc: git
In-Reply-To: <20101207212041.GG25767@bzzt.net>
Arnout Engelen <arnouten@bzzt.net> writes:
> Thanks to you and Jonathan again for the feedback.
>
> On Tue, Dec 07, 2010 at 12:56:17PM -0800, Junio C Hamano wrote:
>> > + char origtemplate[255];
>> > + strlcpy(origtemplate, template, 255);
>>
>> Why "255"?
>
> Random - 'i had to choose something'.
>
>> It may happen to be sufficiently large for the current callers, but what
>> provisions if any are made to help the compiler or the runtime protect us
>> from new and broken callers? Use of strlcpy() there hides the issue from
>> the runtime by avoiding segfault, but it actively harms us by making the
>> code silently behave incorrectly without segfaulting, no?
>
> Only in a small way: when a bigger template is encountered and the mkstemp
> call succeeds, there is no problem. Only when xmkstemp fails *and* clears the
> template, the diagnostic error message shows a truncated version of the
> original.
Ah, ok, it seems that I misread the patch. This copy you are making is
not used to actually construct the filename used for creating the
temporary file, so there is no risk the function misbehaving; we would
just give a truncated error report, which is no worse than what we have
been giving the users anyway.
^ permalink raw reply
* Re: [PATCH] imap-send: link against libcrypto for HMAC and others
From: Jared Hance @ 2010-12-08 1:51 UTC (permalink / raw)
To: git
In-Reply-To: <7v39qbtoq9.fsf@alter.siamese.dyndns.org>
> You are both correct; the point of NO_OPENSSL is not to link with anything
> from openssl suite, so we need a separate mechanism to address this.
>
> Anders, wouldn't this be a better fix for NO_OPENSSL build, than reverting
> a fix for an incorrect ld invocation?
Could we get this fixup patch into master? Leaving the original patch
in without it doesn't seem like a good idea when it breaks the build.
^ permalink raw reply
* Re: how to make a commit only contain existing files
From: david @ 2010-12-08 0:52 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20101207211126.GA29453@sigill.intra.peff.net>
On Tue, 7 Dec 2010, Jeff King wrote:
> On Tue, Dec 07, 2010 at 01:00:18PM -0800, david@lang.hm wrote:
>
>> I know that I can do a git rm to explictly remove files, but is there
>> an easy way to just say that this commit should contain all the files
>> that exist at this point in time, without carrying over any files
>> that were in a prior commit but that don't exist now?
>
> Won't "git add -A; git commit" do what you want?
it very well may, I'll have to upgrade git on that box to a version that
supports it.
Thanks.
David Lang
^ permalink raw reply
* Re: [PATCH] completion: Add PS1 configuration for submodules
From: Ævar Arnfjörð Bjarmason @ 2010-12-08 0:27 UTC (permalink / raw)
To: Kevin Ballard; +Cc: Scott Kyle, git
In-Reply-To: <4F33DA75-D095-415D-99F8-5236D6F93EE1@sb.org>
On Tue, Dec 7, 2010 at 21:41, Kevin Ballard <kevin@sb.org> wrote:
> On Dec 7, 2010, at 12:37 PM, Scott Kyle wrote:
>
>> On Tue, Dec 7, 2010 at 4:15 AM, Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
>>>
>>> On Tue, Dec 7, 2010 at 00:22, Scott Kyle <scott@appden.com> wrote:
>>>> For those who often work on repositories with submodules, the dirty
>>>> indicator for unstaged changes will almost always show because development
>>>> is simultaneously happening on those submodules. The config option
>>>> diff.ignoreSubmodules is not appropriate for this use because it has larger
>>>> implications.
>>>
>>> Wouldn't it be a lot better to instead add support for showing
>>> submodule dirtyness as distinct from the main tree's dirtyness? Then
>>> you could easily spot if you had either your tree / submodule tree
>>> changes, without just ignoring them.
>>
>> I considered that, but thought it to be a rather disruptive change,
>> and one that conceptually didn't work. The way I see it, either
>> somebody thinks of their repo as dirty when the submodules are dirty,
>> or not. And I think since this behavior has perpetuated for so long,
>> most users are content with how it currently works. I, however, was
>> not, and so that is why I added an option for people like me.
>
> The big win for such a change, from my perspective, is it tells me if I need
> to do a `git submodule update --recursive`, or if I actually have dirty changes.
> Because of that, if nobody else picks this up, I'll probably write a patch
> to introduce such a config at some point in the future. But as I said before,
> that's something that can be done later and doesn't need to affect this patch.
Yeah. I didn't mean to imply that the current patch wasn't useful. It
also is for people like Scott that just want to ignore submodules, but
most of us care about them being dirty.
So having support for both (ignoring and tracking) in __git_ps1 would
be great. It would be very useful if you or someone else could pick
this up.
^ permalink raw reply
* Re: [PATCH 01/14] msvc: Fix compilation errors in compat/win32/sys/poll.c
From: Ramsay Jones @ 2010-12-08 0:18 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, kusmabite, GIT Mailing-list
In-Reply-To: <201012042222.23384.j6t@kdbg.org>
Johannes Sixt wrote:
> On Samstag, 4. Dezember 2010, Ramsay Jones wrote:
>> diff --git a/compat/win32/sys/poll.c b/compat/win32/sys/poll.c
>> index 7e74ebe..708a6c9 100644
>> --- a/compat/win32/sys/poll.c
>> +++ b/compat/win32/sys/poll.c
>> @@ -34,6 +34,9 @@
>>
>> #if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
>> # define WIN32_NATIVE
>> +# if defined (_MSC_VER)
>> +# define _WIN32_WINNT 0x0502
>> +# endif
>> # include <winsock2.h>
>> # include <windows.h>
>> # include <io.h>
>
> Don't you have to do the same in git-compat-util.h?
No. compat/win32/sys/poll.c doesn't include git-compat-util.h (and I
don't think it should), so adding it there would not solve the immediate
problem. Also, I don't see any reason to restrict the API used by both
MinGW and msvc in other parts of git.
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 02/14] msvc: git-daemon.exe: Fix linker "unresolved externals" error
From: Ramsay Jones @ 2010-12-08 0:11 UTC (permalink / raw)
To: kusmabite; +Cc: Junio C Hamano, Johannes Sixt, GIT Mailing-list
In-Reply-To: <AANLkTi=cVTcaQTQqNeQoNHhUrEBZvE5B=aFDy=r+k0uQ@mail.gmail.com>
Erik Faye-Lund wrote:
> On Sat, Dec 4, 2010 at 7:55 PM, Ramsay Jones <ramsay@ramsay1.demon.co.uk> wrote:
>> The msvc linker complains about external symbols referenced by
>> the new poll() emulation code. In particular, it complains about
>> the DispatchMessage(), PeekMessage(), TranslateMessage() and
>> MsgWaitForMultipleObjects() functions.
>>
>> In order to satisfy the external references, we link against the
>> user32.lib library.
>>
>> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
>> ---
>> Makefile | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 1d42413..bda9fa6 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1087,7 +1087,7 @@ ifeq ($(uname_S),Windows)
>> COMPAT_OBJS = compat/msvc.o compat/fnmatch/fnmatch.o compat/winansi.o compat/win32/pthread.o compat/win32/syslog.o compat/win32/sys/poll.o
>> COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/fnmatch -Icompat/regex -Icompat/fnmatch -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
>> BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib
>> - EXTLIBS = advapi32.lib shell32.lib wininet.lib ws2_32.lib
>> + EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib
>> PTHREAD_LIBS =
>> lib =
>> ifndef DEBUG
>
> Looks sane. But it's a bit odd - it currently builds fine without this
> patch from the IDE, but not from the makefile. Is the linker-libs
> different between these?
Er... dunno! ;-) I don't use the IDE (except to read the msdn docs), I'm
*much* more productive in vim (yeah, I'm a luddite!).
So, how did you create your project file? What does it say?
ie. you tell me! :-P
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH 04/14] msvc: Fix macro redefinition warnings
From: Ramsay Jones @ 2010-12-08 0:05 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Junio C Hamano, GIT Mailing-list, sschuberth
In-Reply-To: <201012042204.52002.j6t@kdbg.org>
Johannes Sixt wrote:
> On Samstag, 4. Dezember 2010, Ramsay Jones wrote:
>> --- a/compat/mingw.h
>> +++ b/compat/mingw.h
>> @@ -14,12 +14,6 @@ typedef int socklen_t;
>> #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)
>> #define S_ISSOCK(x) 0
>>
>> -#ifndef _STAT_H_
>
> Instead of removing the macros, wouldn't we be much safer with just
>
> #ifndef S_IWUSR
>
> ? ...
Er... no.
Commit 4091bfc (which added these macros) does not provide any motivation
for the change, and I'm having a hard time trying to imagine a useful
purpose for this part of the commit. (I'm not saying there isn't one - just
that I can't see it :-P )
On MinGW, the <sys/stat.h> header is always included prior to this header,
so the _STAT_H_ include guard is always defined, so these macros will
never be defined (which is a *good* thing; have you looked at the definitions).
Trying to use compat/mingw.h without having first included <sys/stat.h> is
not going to work!
Also, note that the include guard for the msvc <sys/stat.h> file is _INC_STAT.
So, on msvc, including <sys/stat.h> does not suppress these macro definitions
(Not that it actually matters here, because it doesn't define these symbols
anyway!). Which is why msvc issues these macro redefinition warnings (they
conflict with the definitions in compat/vcbuild/include/unistd.h). We most
definitely don't want to use the macros in compat/mingw.h on msvc. (They are
positively *wrong*)
[Hmmm, I've just noticed that the msvc compat header is missing a definition of
the _S_IRWXU macro!]
So, once again, I see no reason to keep them ... Unless you know otherwise.
ATB,
Ramsay Jones
^ 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