[PATCH 1/3] Introduce diff_filespec_is_binary()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 6, 2007, 08:37 UTC
- Message-ID
- <7vejjmexm0.fsf_-_@assigned-by-dhcp.cox.net>
- In-Reply-To
- <Pine.LNX.4.64.0707051223520.9789@racer.site>
This replaces an explicit initialization of filespec->is_binary field used for rename/break followed by direct access to that field with a wrapper function that lazily iniaitlizes and accesses the field. We would add more attribute accesses for the use of diff routines, and it would be better to make this abstraction earlier.
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> It is quite cute to hide this in the funcname patch...
So this is the proper refactoring _before_ any of your topics.
diff.c | 71 +++++++++++++++++++++++++++-------------------------- diffcore-delta.c | 2 +- diffcore.h | 2 + 3 files changed, 39 insertions(+), 36 deletions(-)
diff --git a/diff.c b/diff.c index 1958970..16ea710 100644 --- a/diff.c +++ b/diff.c @@ -1102,30 +1102,45 @@ static void setup_diff_attr_check(struct git_attr_check *check) { static struct git_attr *attr_diff; - if (!attr_diff) + if (!attr_diff) { attr_diff = git_attr("diff", 4); - check->attr = attr_diff; + } + check[0].attr = attr_diff; } -static int file_is_binary(struct diff_filespec *one) +static void diff_filespec_check_attr(struct diff_filespec *one) { - struct git_attr_check attr_diff_check; + struct git_attr_check attr_diff_check[1]; - setup_diff_attr_check(&attr_diff_check); - if (!git_checkattr(one->path, 1, &attr_diff_check)) { - const char *value = attr_diff_check.value; + if (one->checked_attr) + return; + + setup_diff_attr_check(attr_diff_check); + one->is_binary = 0; + + if (!git_checkattr(one->path, ARRAY_SIZE(attr_diff_check), attr_diff_check)) { + const char *value; + + /* binaryness */ + value = attr_diff_check[0].value; if (ATTR_TRUE(value)) - return 0; + ; else if (ATTR_FALSE(value)) - return 1; + one->is_binary = 1; } - if (!one->data) { - if (!DIFF_FILE_VALID(one)) - return 0; + if (!one->data && DIFF_FILE_VALID(one)) diff_populate_filespec(one, 0); - } - return buffer_is_binary(one->data, one->size); + + if (one->data) + one->is_binary = buffer_is_binary(one->data, one->size); + +} + +int diff_filespec_is_binary(struct diff_filespec *one) +{ + diff_filespec_check_attr(one); + return one->is_binary; } static void builtin_diff(const char *name_a, @@ -1182,7 +1197,8 @@ static void builtin_diff(const char *name_a, if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0) die("unable to read files to diff"); - if (!o->text && (file_is_binary(one) || file_is_binary(two))) { + if (!o->text && + (diff_filespec_is_binary(one) || diff_filespec_is_binary(two))) { /* Quite common confusing case */ if (mf1.size == mf2.size && !memcmp(mf1.ptr, mf2.ptr, mf1.size)) @@ -1260,7 +1276,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b, if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0) die("unable to read files to diff"); - if (file_is_binary(one) || file_is_binary(two)) { + if (diff_filespec_is_binary(one) || diff_filespec_is_binary(two)) { data->is_binary = 1; data->added = mf2.size; data->deleted = mf1.size; @@ -1302,7 +1318,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b, if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0) die("unable to read files to diff"); - if (file_is_binary(two)) + if (diff_filespec_is_binary(two)) goto free_and_return; else { /* Crazy xdl interfaces.. */ @@ -1880,8 +1896,8 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o) if (o->binary) { mmfile_t mf; - if ((!fill_mmfile(&mf, one) && file_is_binary(one)) || - (!fill_mmfile(&mf, two) && file_is_binary(two))) + if ((!fill_mmfile(&mf, one) && diff_filespec_is_binary(one)) || + (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two))) abbrev = 40; } len += snprintf(msg + len, sizeof(msg) - len, @@ -2783,7 +2799,7 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1) return error("unable to read files to diff"); /* Maybe hash p->two? into the patch id? */ - if (file_is_binary(p->two)) + if (diff_filespec_is_binary(p->two)) continue; len1 = remove_space(p->one->path, strlen(p->one->path)); @@ -3011,21 +3027,6 @@ void diffcore_std(struct diff_options *options) if (options->quiet) return; - /* - * break/rename count similarity differently depending on - * the binary-ness. - */ - if ((options->break_opt != -1) || (options->detect_rename)) { - struct diff_queue_struct *q = &diff_queued_diff; - int i; - - for (i = 0; i < q->nr; i++) { - struct diff_filepair *p = q->queue[i]; - p->one->is_binary = file_is_binary(p->one); - p->two->is_binary = file_is_binary(p->two); - } - } - if (options->break_opt != -1) diffcore_break(options->break_opt); if (options->detect_rename) diff --git a/diffcore-delta.c b/diffcore-delta.c index a038b16..d9729e5 100644 --- a/diffcore-delta.c +++ b/diffcore-delta.c @@ -129,7 +129,7 @@ static struct spanhash_top *hash_chars(struct diff_filespec *one) struct spanhash_top *hash; unsigned char *buf = one->data; unsigned int sz = one->size; - int is_text = !one->is_binary; + int is_text = !diff_filespec_is_binary(one); i = INITIAL_HASH_SIZE; hash = xmalloc(sizeof(*hash) + sizeof(struct spanhash) * (1<<i)); diff --git a/diffcore.h b/diffcore.h index 0c8abb5..dcab7e2 100644 --- a/diffcore.h +++ b/diffcore.h @@ -37,6 +37,7 @@ struct diff_filespec { #define DIFF_FILE_VALID(spec) (((spec)->mode) != 0) unsigned should_free : 1; /* data should be free()'ed */ unsigned should_munmap : 1; /* data should be munmap()'ed */ + unsigned checked_attr : 1; unsigned is_binary : 1; /* data should be considered "binary" */ }; @@ -46,6 +47,7 @@ extern void fill_filespec(struct diff_filespec *, const unsigned char *, extern int diff_populate_filespec(struct diff_filespec *, int); extern void diff_free_filespec_data(struct diff_filespec *); +extern int diff_filespec_is_binary(struct diff_filespec *); struct diff_filepair { struct diff_filespec *one;
-- 1.5.3.rc0.822.g38609