[PATCH/RFC] Introduce a built-in attribute "text"

[PATCH/RFC] Introduce a built-in attribute "text"

[Michael J Gruber]

"text is the opposite of "binary": It sets the attributes "crlf" and
"diff". It is needed because attribute macros can't be negated,
and some users may want to force git into treating certain files as
text which are not recognized by the internal logic.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
This gives the user the chance to mess up completely (given an
"appropriate" setting of autocrlf), but I still think it's a good idea
to have that "-binary" built-in. attributes aren't used by noobs anyways.
In many cases, "diff" might be preferred over "text". Should I add a warning
to the doc?

Michael

 Documentation/gitattributes.txt |    4 +++-
 attr.c                          |    1 +
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 2694559..2a00f8c 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -510,7 +510,9 @@ the same time.  The system knows a built-in attribute macro, `binary`:
 which is equivalent to the above.  Note that the attribute macros can only
 be "Set" (see the above example that sets "binary" macro as if it were an
 ordinary attribute --- setting it in turn unsets "crlf" and "diff").
-
+Therefore, there is also a built-in attribute macro `text` which allows
+you to mark certain files as text which git does not recognize automatically.
+It is equivalent to setting `crlf diff`.
 
 DEFINING ATTRIBUTE MACROS
 -------------------------
diff --git a/attr.c b/attr.c
index 17f6a4d..63e2837 100644
--- a/attr.c
+++ b/attr.c
@@ -283,6 +283,7 @@ static void free_attr_elem(struct attr_stack *e)
 
 static const char *builtin_attr[] = {
 	"[attr]binary -diff -crlf",
+	"[attr]text diff crlf",
 	NULL,
 };
 
-- 
1.6.0.3.514.g2f91b

Re* [PATCH/RFC] Introduce a built-in attribute "text"

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> "text is the opposite of "binary": It sets the attributes "crlf" and
> "diff". It is needed because attribute macros can't be negated,

If the reason for this patch is really (and only) because macros cannot be
negated, perhaps we should be allowing to negate them?  That is, given:

	[attr] binary = -diff -crlf
        *.bar !binary

you would set diff and crlf to TRUE for b.bar, because the normal action
is to set them to FALSE.

If the original attribute macro expands to anything but setting other
attributes to TRUE or FALSE (i.e. to a string value, or reseting to
UNSPECIFIED), or if a related attribute is not the exact opposite, you
cannot simply negate, but need to introduce a new attribute like your
patch does.  For example, it is conceivable you might instead want to
define:

	[attr] text = diff

i.e. without "crlf" bit.

Since what you want to do here is merely to negate what is already there,
it might make more sense to simply bite the bullet and give the attribute
mechanism the ability to negate an attribute macro.

This is of course untested (you can tell that by noticing that it does not
come with patch to the test scripts) nor undocumented, but it looks
obvious enough ;-).  Originally we silently ignored attempts to negate or
to reset an attribute macro.  We now allow you to negate an attribute
macro, and the definition of "negating" is:

 - negation of setting to TRUE is setting to FALSE.
 - negation of setting to FALSE is setting to TRUE.
 - negation of anything else is itself.

The third one is arbitrary and people may want to come up with a different
behaviour (with solid argument explaining why that behaviour is more
sensible).

 attr.c |   34 +++++++++++++++++++++++++++++-----
 1 files changed, 29 insertions(+), 5 deletions(-)

diff --git c/attr.c w/attr.c
index 17f6a4d..0021cec 100644
--- c/attr.c
+++ w/attr.c
@@ -558,7 +558,24 @@ static int path_matches(const char *pathname, int pathlen,
 	return fnmatch(pattern, pathname + baselen, FNM_PATHNAME) == 0;
 }
 
-static int fill_one(const char *what, struct match_attr *a, int rem)
+static const char *attr_negate(const char *value)
+{
+	if (value == ATTR__TRUE)
+		return ATTR__FALSE;
+	else if (value == ATTR__FALSE)
+		return ATTR__TRUE;
+	else
+		/*
+		 * NEEDSWORK: here we define negation of setting
+		 * to anything but TRUE and FALSE is itself.  We
+		 * may want to instead say it is not touching the
+		 * attribute, in which case you could simply return
+		 * NULL from here.
+		 */
+		return value;
+}
+
+static int fill_one(const char *what, struct match_attr *a, int rem, int negated)
 {
 	struct git_attr_check *check = check_all_attr;
 	int i;
@@ -569,6 +586,11 @@ static int fill_one(const char *what, struct match_attr *a, int rem)
 		const char *v = a->state[i].setto;
 
 		if (*n == ATTR__UNKNOWN) {
+			if (negated) {
+				v = attr_negate(v);
+				if (!v)
+					continue;
+			}
 			debug_set(what, a->u.pattern, attr, v);
 			*n = v;
 			rem--;
@@ -588,7 +610,7 @@ static int fill(const char *path, int pathlen, struct attr_stack *stk, int rem)
 			continue;
 		if (path_matches(path, pathlen,
 				 a->u.pattern, base, strlen(base)))
-			rem = fill_one("fill", a, rem);
+			rem = fill_one("fill", a, rem, 0);
 	}
 	return rem;
 }
@@ -602,9 +624,11 @@ static int macroexpand(struct attr_stack *stk, int rem)
 		struct match_attr *a = stk->attrs[i];
 		if (!a->is_macro)
 			continue;
-		if (check[a->u.attr->attr_nr].value != ATTR__TRUE)
-			continue;
-		rem = fill_one("expand", a, rem);
+		if (check[a->u.attr->attr_nr].value == ATTR__TRUE)
+			rem = fill_one("expand", a, rem, 0);
+		else if (check[a->u.attr->attr_nr].value == ATTR__FALSE)
+			rem = fill_one("expand", a, rem, 1);
+			
 	}
 	return rem;
 }