git diff bug?

git diff bug?

[David Abrahams]

Please see
http://github.com/techarcana/elisp/commit/63d672c296316c85690085930b05c642b88a9978#diff-2

Note how the @@ ... @@ clauses are followed by text from the previous
line's comment.  Not sure, but this strikes me as a line-ending issue.
custom.el was originally built on a linux machine; now I'm using a Mac.

Thanks!

-- 
Dave Abrahams
BoostPro Computing
http://www.boostpro.com

Re: git diff bug?

[Jeff King]

On Fri, Apr 03, 2009 at 09:10:42PM -0400, David Abrahams wrote:

> Please see
> http://github.com/techarcana/elisp/commit/63d672c296316c85690085930b05c642b88a9978#diff-2
> 
> Note how the @@ ... @@ clauses are followed by text from the previous
> line's comment.  Not sure, but this strikes me as a line-ending issue.
> custom.el was originally built on a linux machine; now I'm using a Mac.

This is as designed. The original file ("git show e7dd7db") contains (my
numbering seems different than what git produces; it is produced by "nl"
which is maybe treating some line endings differently earlier in the
file):

   102   '(mm-attachment-override-types (quote ("text/x-vcard" "application/pkcs7-mime" "application/x-pkcs7-mime" "application/pkcs7-signature" "application/x-pkcs7-signature" "image/*")) nil nil "
   103  Added image/* to display attached images inline")
   104   '(mm-discouraged-alternatives (quote ("text/html" "text/richtext" "image/.*")) nil nil "
   105  The documentation for this variable says it all")
   106   '(mm-inline-text-html-with-images t)
   107   '(muse-project-alist (quote (("WikiPlanner" ("~/plans" :default "index" :major-mode planner-mode :visit-link planner-visit-link)))))
   108   '(org-agenda-files (quote ("~/organizer.org")))

The changed text in your diff starts on 108. So we show 105-107 as
context lines. The text after the @@ clause is the "function header";
this is equivalent to "-p" in GNU diff. It's basically a guess about the
most interesting context to show, and looks alphabetic characters that
are left-aligned. In the case of lisp, it really isn't all that
interesting (and what looks so weird is that your file contains
a lot of

  "\nSome text"

so the text strings are all left-aligned. You can customize the regex
used to guess at the function header. See "defining a custom
hunk-header" in "git help attributes".

-Peff

Re: git diff bug?

[Jeff King]

On Fri, Apr 03, 2009 at 09:45:27PM -0400, Jeff King wrote:

> This is as designed. The original file ("git show e7dd7db") contains (my
> numbering seems different than what git produces; it is produced by "nl"
> which is maybe treating some line endings differently earlier in the
> file):

Sorry, this was just me failing to use "nl" correctly. I needed to use
"-ba" to number empty lines. So the line numbers do match git, and the
rest of my explanation holds.

-Peff

Re: git diff bug?

[David Abrahams]

On Apr 3, 2009, at 9:45 PM, Jeff King wrote:

> On Fri, Apr 03, 2009 at 09:10:42PM -0400, David Abrahams wrote:
>
>> Please see
>> http://github.com/techarcana/elisp/commit/63d672c296316c85690085930b05c642b88a9978#diff-2
>>
>> Note how the @@ ... @@ clauses are followed by text from the previous
>> line's comment.  Not sure, but this strikes me as a line-ending  
>> issue.
>> custom.el was originally built on a linux machine; now I'm using a  
>> Mac.
>
> This is as designed. The original file ("git show e7dd7db") contains  
> (my
> numbering seems different than what git produces; it is produced by  
> "nl"
> which is maybe treating some line endings differently earlier in the
> file):
>
>   102   '(mm-attachment-override-types (quote ("text/x-vcard"  
> "application/pkcs7-mime" "application/x-pkcs7-mime" "application/ 
> pkcs7-signature" "application/x-pkcs7-signature" "image/*")) nil nil "
>   103  Added image/* to display attached images inline")
>   104   '(mm-discouraged-alternatives (quote ("text/html" "text/ 
> richtext" "image/.*")) nil nil "
>   105  The documentation for this variable says it all")
>   106   '(mm-inline-text-html-with-images t)
>   107   '(muse-project-alist (quote (("WikiPlanner" ("~/ 
> plans" :default "index" :major-mode planner-mode :visit-link planner- 
> visit-link)))))
>   108   '(org-agenda-files (quote ("~/organizer.org")))
>
> The changed text in your diff starts on 108. So we show 105-107 as
> context lines. The text after the @@ clause is the "function header";
> this is equivalent to "-p" in GNU diff. It's basically a guess about  
> the
> most interesting context to show, and looks alphabetic characters that
> are left-aligned. In the case of lisp, it really isn't all that
> interesting (and what looks so weird is that your file contains
> a lot of
>
>  "\nSome text"
>
> so the text strings are all left-aligned. You can customize the regex
> used to guess at the function header. See "defining a custom
> hunk-header" in "git help attributes".

Hmm, so I tried sticking this .gitattributes in my repo

   *.el   diff=el
   [diff "el"]
         xfuncname = "^(\\(def[a-z]+ .+)$"

and git diff barfed with

   "el"] is not a valid attribute name: .gitattributes:2
   "^(\\(def[a-z]+ is not a valid attribute name: .gitattributes:3

What am I missing?  I tried googling, but from what turns up for me,  
it doesn't look like anyone else has ever tried to use this feature!

TIA,

--
David Abrahams
BoostPro Computing
http://boostpro.com

Re: git diff bug?

[Michael J Gruber]

David Abrahams venit, vidit, dixit 06.04.2009 11:09:
> 
> On Apr 3, 2009, at 9:45 PM, Jeff King wrote:
> 
>> On Fri, Apr 03, 2009 at 09:10:42PM -0400, David Abrahams wrote:
>>
>>> Please see
>>> http://github.com/techarcana/elisp/commit/63d672c296316c85690085930b05c642b88a9978#diff-2
>>>
>>> Note how the @@ ... @@ clauses are followed by text from the previous
>>> line's comment.  Not sure, but this strikes me as a line-ending  
>>> issue.
>>> custom.el was originally built on a linux machine; now I'm using a  
>>> Mac.
>>
>> This is as designed. The original file ("git show e7dd7db") contains  
>> (my
>> numbering seems different than what git produces; it is produced by  
>> "nl"
>> which is maybe treating some line endings differently earlier in the
>> file):
>>
>>   102   '(mm-attachment-override-types (quote ("text/x-vcard"  
>> "application/pkcs7-mime" "application/x-pkcs7-mime" "application/ 
>> pkcs7-signature" "application/x-pkcs7-signature" "image/*")) nil nil "
>>   103  Added image/* to display attached images inline")
>>   104   '(mm-discouraged-alternatives (quote ("text/html" "text/ 
>> richtext" "image/.*")) nil nil "
>>   105  The documentation for this variable says it all")
>>   106   '(mm-inline-text-html-with-images t)
>>   107   '(muse-project-alist (quote (("WikiPlanner" ("~/ 
>> plans" :default "index" :major-mode planner-mode :visit-link planner- 
>> visit-link)))))
>>   108   '(org-agenda-files (quote ("~/organizer.org")))
>>
>> The changed text in your diff starts on 108. So we show 105-107 as
>> context lines. The text after the @@ clause is the "function header";
>> this is equivalent to "-p" in GNU diff. It's basically a guess about  
>> the
>> most interesting context to show, and looks alphabetic characters that
>> are left-aligned. In the case of lisp, it really isn't all that
>> interesting (and what looks so weird is that your file contains
>> a lot of
>>
>>  "\nSome text"
>>
>> so the text strings are all left-aligned. You can customize the regex
>> used to guess at the function header. See "defining a custom
>> hunk-header" in "git help attributes".
> 
> Hmm, so I tried sticking this .gitattributes in my repo
> 
>    *.el   diff=el
>    [diff "el"]
>          xfuncname = "^(\\(def[a-z]+ .+)$"
> 
> and git diff barfed with
> 
>    "el"] is not a valid attribute name: .gitattributes:2
>    "^(\\(def[a-z]+ is not a valid attribute name: .gitattributes:3
> 
> What am I missing?  I tried googling, but from what turns up for me,  
> it doesn't look like anyone else has ever tried to use this feature!

Well, I don't think anayone else has tried putting config lines into
.gitattributes ;)

The "*.el" line goes into .gitattributes (or .git/info/a...), the other
lines are config lines and thus goe into .git/config or .gitconfig.

Michael

Re: git diff bug?

[Jeff King]

On Mon, Apr 06, 2009 at 05:09:32AM -0400, David Abrahams wrote:

> Hmm, so I tried sticking this .gitattributes in my repo
>
>   *.el   diff=el
>   [diff "el"]
>         xfuncname = "^(\\(def[a-z]+ .+)$"
>
> and git diff barfed with

As Michael explained, the first line goes into .gitattributes, and the
second two go into your .git/config.

The reason is that the first line is describing a property of files
in the repo: "*.el is of type 'el' for diffing". And thus it makes sense
for it to be committed along with the content in the repo.

But the latter two lines are about preference: "here is how _I_ would
like to generate diffs for el files". So they go in your personal config
which is not shipped with the repo. And most likely you would want to
put them not in per-repo config, but in your user config:
$HOME/.gitconfig.

All of that being said, it is obviously a documentation failure if it
didn't explain in which files the various parts go. If you don't mind,
can you skim over the documentation you read one more time and point out
where it led you wrong?  Maybe we can make it a little clearer.

-Peff

Re: git diff bug?

[Matthieu Moy]

Jeff King <peff@peff.net> writes:

> But the latter two lines are about preference: "here is how _I_ would
> like to generate diffs for el files".

... and they are commands to be executed. If they were in the
repository, and propagated with clone, then doing

git clone git://some.git/repo
cd repo
git diff

would execute arbitrary commands, which wouldn't be acceptable for
security reasons.

-- 
Matthieu

Re: git diff bug?

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> Jeff King <peff@peff.net> writes:
>
>> But the latter two lines are about preference: "here is how _I_ would
>> like to generate diffs for el files".
>
> ... and they are commands to be executed. If they were in the
> repository, and propagated with clone, then doing
>
> git clone git://some.git/repo
> cd repo
> git diff
>
> would execute arbitrary commands, which wouldn't be acceptable for
> security reasons.

Other configuration variables have such security implications, but diff
hunk header bit doesn't.  You are a bit overly cautious in this particular
case.

Re: git diff bug?

[David Abrahams]

On Apr 6, 2009, at 10:44 AM, Michael J Gruber wrote:

>
> Well, I don't think anayone else has tried putting config lines into
> .gitattributes ;)
>
> The "*.el" line goes into .gitattributes (or .git/info/a...), the  
> other
> lines are config lines and thus goe into .git/config or .gitconfig.


Aha, thanks.  The hunk header section in the gitattributes manpage  
turns out to be the only one that shows config stuff without  
mentioning a config file, thus my confusion.  I suggest a tiny patch  
('cept I don't know how to write manpage format)

--
David Abrahams
BoostPro Computing
http://boostpro.com

Re: git diff bug?

[David Abrahams]

On Apr 6, 2009, at 11:53 AM, Jeff King wrote:

> All of that being said, it is obviously a documentation failure if it
> didn't explain in which files the various parts go. If you don't mind,
> can you skim over the documentation you read one more time and point  
> out
> where it led you wrong?  Maybe we can make it a little clearer.

Wow, that is a *really* admirable response.  Git itself may be a tad  
arcane, but the community has been fantastic. Thanks billionses for  
that -- it can only mean more a more learnable Git down the road.  I  
posted an answer to your question in a different reply.

--
David Abrahams
BoostPro Computing
http://boostpro.com

Re: git diff bug?

[Jeff King]

On Mon, Apr 06, 2009 at 01:58:08PM -0400, David Abrahams wrote:

>> The "*.el" line goes into .gitattributes (or .git/info/a...), the other
>> lines are config lines and thus goe into .git/config or .gitconfig.
>
> Aha, thanks.  The hunk header section in the gitattributes manpage turns 
> out to be the only one that shows config stuff without mentioning a config 
> file, thus my confusion.  I suggest a tiny patch ('cept I don't know how 
> to write manpage format)

Neither do I, but fortunately the git docs are all written in asciidoc. :)

Junio, does the patch below make sense to you?

-- >8 --
Subject: [PATCH] doc/gitattributes: clarify location of config text

The gitattributes documentation has a section on the "diff"
attribute, with subsections for each of the things you might
want to configure in your diff config section (external
diff, hunk headers, etc). The first such subsection
specifically notes that the definition of the diff driver
should go into $GIT_DIR/config, but subsequent sections do
not.

This location is implied if you are reading the
documentation sequentially, but it is not uncommon for a new
user to jump to (or be referred to) a specific section. For
a new user who does not know git well enough to recognize
the config syntax, it is not clear that those directives
don't also go into the gitattributes file.

This patch just mentions the config file in each subsection,
similar to the way it is mentioned in the first.

Signed-off-by: Jeff King <peff@peff.net>
---
Mentioning $GIT_DIR/config _and_ $HOME/.gitconfig in each subsection may
seem like overkill, but I really think we shouldn't make any assumption
that a reader has seen the previous sections. They otherwise stand alone
very well.

I arrived at these three sections by grepping for '^[' to find example
config file syntax.

 Documentation/gitattributes.txt |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 55668e3..b6260b1 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -297,7 +297,8 @@ for paths.
 
 Then, you would define a "diff.tex.xfuncname" configuration to
 specify a regular expression that matches a line that you would
-want to appear as the hunk header "TEXT", like this:
+want to appear as the hunk header "TEXT". Add a section to your
+`$GIT_DIR/config` file (or `$HOME/.gitconfig` file) like this:
 
 ------------------------
 [diff "tex"]
@@ -345,7 +346,8 @@ split words in a line, by specifying an appropriate regular expression
 in the "diff.*.wordRegex" configuration variable.  For example, in TeX
 a backslash followed by a sequence of letters forms a command, but
 several such commands can be run together without intervening
-whitespace.  To separate them, use a regular expression such as
+whitespace.  To separate them, use a regular expression in your
+`$GIT_DIR/config` file (or `$HOME/.gitconfig` file) like this:
 
 ------------------------
 [diff "tex"]
@@ -373,7 +375,8 @@ resulting text on stdout.
 
 For example, to show the diff of the exif information of a
 file instead of the binary information (assuming you have the
-exif tool installed):
+exif tool installed), add the following section to your
+`$GIT_DIR/config` file (or `$HOME/.gitconfig` file):
 
 ------------------------
 [diff "jpg"]
-- 
1.6.2.2.585.g1e067

Re: git diff bug?

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> Jeff King <peff@peff.net> writes:
>>
>>> But the latter two lines are about preference: "here is how _I_ would
>>> like to generate diffs for el files".
>>
>> ... and they are commands to be executed. If they were in the
>> repository, and propagated with clone, then doing
>>
>> git clone git://some.git/repo
>> cd repo
>> git diff
>>
>> would execute arbitrary commands, which wouldn't be acceptable for
>> security reasons.
>
> Other configuration variables have such security implications, but diff
> hunk header bit doesn't.  You are a bit overly cautious in this particular
> case.

Oops, right, I was thinking of other parameters in the same section of
the config file. Thanks for correcting.

-- 
Matthieu

Re: git diff bug?

[Antriksh Pany]

I believe you need to have only the line

*.el diff=el

in the .gitattributes file.

The part

[diff = "el"]
   xfuncname = ...

has to be in the git config file. The .git/config file would do for this.


On Mon, Apr 6, 2009 at 2:39 PM, David Abrahams <dave@boostpro.com> wrote:
>
> On Apr 3, 2009, at 9:45 PM, Jeff King wrote:
>
>> On Fri, Apr 03, 2009 at 09:10:42PM -0400, David Abrahams wrote:
>>
>>> Please see
>>> http://github.com/techarcana/elisp/commit/63d672c296316c85690085930b05c642b88a9978#diff-2
>>>
>>> Note how the @@ ... @@ clauses are followed by text from the previous
>>> line's comment.  Not sure, but this strikes me as a line-ending issue.
>>> custom.el was originally built on a linux machine; now I'm using a Mac.
>>
>> This is as designed. The original file ("git show e7dd7db") contains (my
>> numbering seems different than what git produces; it is produced by "nl"
>> which is maybe treating some line endings differently earlier in the
>> file):
>>
>>  102   '(mm-attachment-override-types (quote ("text/x-vcard" "application/pkcs7-mime" "application/x-pkcs7-mime" "application/pkcs7-signature" "application/x-pkcs7-signature" "image/*")) nil nil "
>>  103  Added image/* to display attached images inline")
>>  104   '(mm-discouraged-alternatives (quote ("text/html" "text/richtext" "image/.*")) nil nil "
>>  105  The documentation for this variable says it all")
>>  106   '(mm-inline-text-html-with-images t)
>>  107   '(muse-project-alist (quote (("WikiPlanner" ("~/plans" :default "index" :major-mode planner-mode :visit-link planner-visit-link)))))
>>  108   '(org-agenda-files (quote ("~/organizer.org")))
>>
>> The changed text in your diff starts on 108. So we show 105-107 as
>> context lines. The text after the @@ clause is the "function header";
>> this is equivalent to "-p" in GNU diff. It's basically a guess about the
>> most interesting context to show, and looks alphabetic characters that
>> are left-aligned. In the case of lisp, it really isn't all that
>> interesting (and what looks so weird is that your file contains
>> a lot of
>>
>>  "\nSome text"
>>
>> so the text strings are all left-aligned. You can customize the regex
>> used to guess at the function header. See "defining a custom
>> hunk-header" in "git help attributes".
>
> Hmm, so I tried sticking this .gitattributes in my repo
>
>  *.el   diff=el
>  [diff "el"]
>        xfuncname = "^(\\(def[a-z]+ .+)$"
>
> and git diff barfed with
>
>  "el"] is not a valid attribute name: .gitattributes:2
>  "^(\\(def[a-z]+ is not a valid attribute name: .gitattributes:3
>
> What am I missing?  I tried googling, but from what turns up for me, it doesn't look like anyone else has ever tried to use this feature!
>
> TIA,
>
> --
> David Abrahams
> BoostPro Computing
> http://boostpro.com
>
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

git diff bug?

[Sarma Tangirala]

Hello All,

If I did 'git diff HEAD^..HEAD -- file' should git not report some
kind of warning if it could not match the file? For example, if 'file'
were infact 'dir/file' and 'file' were unique, would it not be a good
idea to report that in the present working directory 'file' were not
found but 'dir/file' were a match?

Apologies if I missed this in the man page.

Thanks

--
010
001
111

Re: git diff bug?

[Célestin Matte]

Le 06/06/2013 23:26, Sarma Tangirala a écrit :
> Hello All,
> 
> If I did 'git diff HEAD^..HEAD -- file' should git not report some
> kind of warning if it could not match the file? For example, if 'file'
> were infact 'dir/file' and 'file' were unique, would it not be a good
> idea to report that in the present working directory 'file' were not
> found but 'dir/file' were a match?

I don't know any program doing such a thing, and I don't think it is the
role of the program to predict which file the user actually wanted to
provide in the command line.
That would imply looking for files with the same name or a close name in
the current directory and its subdirectories - and maybe even in the
superdirectory? It is hard to decide when you have to stop looking for
the file.

Re: git diff bug?

[Junio C Hamano]

Célestin Matte <celestin.matte@ensimag.fr> writes:

> Le 06/06/2013 23:26, Sarma Tangirala a écrit :
>> Hello All,
>> 
>> If I did 'git diff HEAD^..HEAD -- file' should git not report some
>> kind of warning if it could not match the file? For example, if 'file'
>> were infact 'dir/file' and 'file' were unique, would it not be a good
>> idea to report that in the present working directory 'file' were not
>> found but 'dir/file' were a match?
>
> I don't know any program doing such a thing, and I don't think it is the
> role of the program to predict which file the user actually wanted to
> provide in the command line.
> That would imply looking for files with the same name or a close name in
> the current directory and its subdirectories - and maybe even in the
> superdirectory? It is hard to decide when you have to stop looking for
> the file.

The parameters after "--" are pathspecs, which is a set of patterns
the paths discovered by the operation (in this case "diff" that
finds paths in HEAD^ and HEAD) are matched against.  They are used
to filter out uninteresting paths.

If HEAD^ and HEAD does not have anything that match the given
pattern (in this case, literal four-letter string "file"), the set
of interesting paths may become empty and that is perfectly normal.

So this is working as designed.

Having said that, we do detect typo by noticing when a pathspec did
not find _any_ path that matched it in some front-end Porcelain
commands, e.g.

	$ git add 'foo*'
        fatal: pathspec 'foo*' did not match any files

It is unreasonable to do the same in "git log old..new -- path" and
error out when the pathspec does not match, because it is normal for
some revisions to have path while some other revisions to lack it.

But for a two-endpoint diff Porcelain (not the plumbing diff-files,
diff-index and diff-tree), I do not think it is particularly a bad
idea to add such a "typo-detection" feature.

Re: git diff bug?

[Sarma Tangirala]

On Thu, Jun 6, 2013 at 6:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Célestin Matte <celestin.matte@ensimag.fr> writes:
>

> But for a two-endpoint diff Porcelain (not the plumbing diff-files,
> diff-index and diff-tree), I do not think it is particularly a bad
> idea to add such a "typo-detection" feature.

I was wondering if this feature is going to be added and if I could
try implementing it.

--
010
001
111

Re: git diff bug?

[Célestin Matte]

Le 07/06/2013 18:01, Sarma Tangirala a écrit :
> On Thu, Jun 6, 2013 at 6:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Célestin Matte <celestin.matte@ensimag.fr> writes:
>>
> 
>> But for a two-endpoint diff Porcelain (not the plumbing diff-files,
>> diff-index and diff-tree), I do not think it is particularly a bad
>> idea to add such a "typo-detection" feature.
> 
> I was wondering if this feature is going to be added and if I could
> try implementing it.

Since nobody answered you (publicly at least), I will try doing it myself:
I think the best thing to do if you want a feature to be added is to
come with a patch and request for comments on it. Then, people will
discuss it and decide whether it's worth adding it to git. So yes, you
can try implementing it - all work is welcome :)

-- 
Célestin Matte

Re: git diff bug?

[Sarma Tangirala]

On Mon, Jun 10, 2013 at 8:44 AM, Célestin Matte
<celestin.matte@ensimag.fr> wrote:

> Since nobody answered you (publicly at least), I will try doing it myself:
> I think the best thing to do if you want a feature to be added is to
> come with a patch and request for comments on it. Then, people will
> discuss it and decide whether it's worth adding it to git. So yes, you
> can try implementing it - all work is welcome :)

That sounds great. I will try implementing and send out a patch soon!


--
010
001
111