threads / patch / 43348

patchRe: [PATCH] xdiff: Do not consider lines starting by # hunkworthy

Subject: Re: [PATCH] xdiff: Do not consider lines starting by # hunkworthy

## tl;dr

4 messages between Oct 25, 2006 and Oct 25, 2006. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Petr Baudis· Oct 25, 2006, 00:07 UTC · lore

[PATCH] xdiff: Do not consider lines starting by # hunkworthy

This will be probably controversial but in my personal experience, the amount of time this is the right thing to do because of #defines is negligible compared to amount of time it is wrong, especially because of #ifs and #endifs in the middle of functions and also because of comments at the line start when it concerns non-C files.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 xdiff/xemit.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

Show changes to xdiff/xemit.c +1 −2

diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 714c563..4f20075 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -86,8 +86,7 @@ static void xdl_find_func(xdfile_t *xf, 
 		if (len > 0 &&
 		    (isalpha((unsigned char)*rec) || /* identifier? */
 		     *rec == '_' ||	/* also identifier? */
-		     *rec == '(' ||	/* lisp defun? */
-		     *rec == '#')) {	/* #define? */
+		     *rec == '(')) {	/* #define? */
 			if (len > sz)
 				len = sz;

Junio C Hamano· Oct 25, 2006, 00:16 UTC · re: Petr Baudis · lore

Petr Baudis <pasky@suse.cz> writes:

Show 26 quoted lines

> This will be probably controversial but in my personal experience, the
> amount of time this is the right thing to do because of #defines is negligible
> compared to amount of time it is wrong, especially because of #ifs and #endifs
> in the middle of functions and also because of comments at the line start when
> it concerns non-C files.
>
> Signed-off-by: Petr Baudis <pasky@suse.cz>
> ---
>
>  xdiff/xemit.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/xdiff/xemit.c b/xdiff/xemit.c
> index 714c563..4f20075 100644
> --- a/xdiff/xemit.c
> +++ b/xdiff/xemit.c
> @@ -86,8 +86,7 @@ static void xdl_find_func(xdfile_t *xf, 
>  		if (len > 0 &&
>  		    (isalpha((unsigned char)*rec) || /* identifier? */
>  		     *rec == '_' ||	/* also identifier? */
> -		     *rec == '(' ||	/* lisp defun? */
> -		     *rec == '#')) {	/* #define? */
> +		     *rec == '(')) {	/* #define? */
>  			if (len > sz)
>  				len = sz;
>  			if (len && rec[len - 1] == '\n')

I'd either omit the opening parenthesis or fix the comment ;-).

More seriously, I'd rather just match default GNU diff behaviour to use isalpha, underscore or '$'. I do not particularly like to have '$' but I feel it is the easiest to match a prior art in cases like this because I do not have to defend my position when somebody says "Why do you include '#'??? It makes no sense!". Since I do not care too much about it, being able to just say "Well we match what GNU diff does by default." is a good thing.

Petr Baudis· Oct 25, 2006, 00:28 UTC · re: Junio C Hamano · lore

[PATCH] xdiff: Match GNU diff behaviour when deciding hunk comment worthiness of lines

This removes the '#' and '(' tests and adds a '$' test instead although I have no idea what it is actually good for - but hey, if that's what GNU diff does...

Pasky only went and did as Junio sayeth.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 xdiff/xemit.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

Show changes to xdiff/xemit.c +1 −2

diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 714c563..4139d55 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -86,8 +86,7 @@ static void xdl_find_func(xdfile_t *xf, 
 		if (len > 0 &&
 		    (isalpha((unsigned char)*rec) || /* identifier? */
 		     *rec == '_' ||	/* also identifier? */
-		     *rec == '(' ||	/* lisp defun? */
-		     *rec == '#')) {	/* #define? */
+		     *rec == '$')) {	/* mysterious GNU diff's invention */
 			if (len > sz)
 				len = sz;

Jakub Narebski· Oct 25, 2006, 00:17 UTC · re: Petr Baudis · lore

Petr Baudis wrote:

> -                    *rec == '(' ||     /* lisp defun? */
> -                    *rec == '#')) {    /* #define? */
> +                    *rec == '(')) {    /* #define? */
>                         if (len > sz)

Shouldn't it be:

+                    *rec == '(')) {    /* lisp defun? */

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git