Skip to contentSkip to conversation

git diff bug?

DISCUSSION David Abrahams 6 participants 13 messages Jun 15, 2016 Documentation

lore
Share & tools
lore
#1 lore raw
Quoted 6 lines · 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.

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

author
#2 lore raw
Quoted 33 lines · Jeff King
Quoted 6 lines · 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.

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

Quoted 5 lines

"\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

#3 lore raw
Quoted 7 lines · David Abrahams

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

author
#4 lore raw
Quoted 4 lines · Jeff King

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

#5 lore raw
In reply to Jeff King
Quoted 2 lines · Jeff King

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.

Signature · 1 line
Matthieu
#6 lore raw
Quoted 8 lines · Matthieu Moy
Quoted 2 lines · Jeff King

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

Quoted 6 lines

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.

#7 lore raw
Quoted 14 lines · Junio C Hamano
Quoted 12 lines · Matthieu Moy
Quoted 2 lines · Jeff King

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

Quoted 6 lines
Quoted 2 lines

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.

Signature · 1 line
Matthieu
#8 lore raw
In reply to David Abrahams

David Abrahams venit, vidit, dixit 06.04.2009 11:09:

Quoted 55 lines · Jeff King
Quoted 41 lines · David Abrahams
Quoted 9 lines

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

Quoted 5 lines

"\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]+ .+)$"
Quoted 7 lines

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

author
#9 lore raw
Quoted 6 lines · Michael J Gruber

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

#10 lore raw
Quoted 7 lines · David Abrahams
Quoted 2 lines

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 <[email protected]>

---
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(-)
Documentation/gitattributes.txt +6 -3
index 55668e3..b6260b1 100644
@@ -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"]
Signature · 1 line
1.6.2.2.585.g1e067
#11 lore raw
In reply to David Abrahams

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.

Quoted 66 lines · David Abrahams
Quoted 30 lines · Jeff King
Quoted 6 lines · 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.

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

Quoted 5 lines

 "\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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

#12 lore raw
In reply to Jeff King
Quoted 4 lines · Jeff King

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