Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
On Thu, Jul 23, 2015 at 10:02:27PM +0200, Sebastian Schuberth wrote: > On Thu, Jul 23, 2015 at 8:08 PM, Jeff King wrote: > > > mode. Actually asking for a two-endpoint tree diff: > > > > git diff-tree --quiet --ignore-space-change $commit^ $commit > > > > will do what you want. > > Yes, I know, thanks. But I deliberately wanted to specify only a > single commit as an optimization, hoping that it would be slightly > faster than computing a commit range. Ah, I see. It should not be any faster, as git has to internally find the first-parent of $commit either way. The big thing you lose with the above syntax is that you are specifying two endpoints, so you cannot do anything clever with merge commits (e.g., if you gave "--cc"). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
Sebastian Schuberth writes: > On Thu, Jul 23, 2015 at 9:39 PM, Junio C Hamano wrote: > >> I haven't dug into why that happens, but possible ways to fix that >> are to make "--quiet" output all (making it consistent with "-s") or >> no (making the command totally silent) output at all ;-). > > Exactly, and I chose the latter to add some value to --quiet instead > of making it an alias for -s. Heh. You didn't even know when "diff-tree --stdin --quiet" would be useful, let alone that it had a bug that made it useless for that exact use case. So it cannot be "I chose the latter". I just gave you a hint so that you can write a plausible-sounding justification, and we both know that it is very different from your original motivation. Be honest. Perhaps the log message would say something like this: $ git rev-list ... | git diff-tree --stdin --quiet [$pathspec] is a way to list the commits that modifies the named paths, but this bug <<>> makes it not to emit all such commits. It couldn't have been used by existing scripts with this longstanding bug. We could fix it so that it does not randomly skip commits that ought to be shown, but that feature is already available by the "-s" option instead of "--quiet". So let's change the meaning of "--quiet" to make it really quiet, without giving any output. Strictly speaking, this may break backward compatibility but the existing behaviour to randomly omit commits couldn't have been useful, so there is no harm done. And as an added bonus, $ git diff-tree --quiet $commit [$pathspec] would stop showing the commit object name. The analysis of the bug is really crucial for the above description to work as justification for this change, substanciating the words "longstanding" and "randomly omit" that are used to convince us that this option couldn't have been used by real scripts. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
On Thu, Jul 23, 2015 at 9:39 PM, Junio C Hamano wrote: > I haven't dug into why that happens, but possible ways to fix that > are to make "--quiet" output all (making it consistent with "-s") or > no (making the command totally silent) output at all ;-). Exactly, and I chose the latter to add some value to --quiet instead of making it an alias for -s. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
On Thu, Jul 23, 2015 at 7:06 PM, Junio C Hamano wrote: > Existing scripts by definition would not be using a new option you > will invent that used not to be a valid one. So that would be one > way that you can shorten your script without breaking other people. True. If it was only for shortening my script, I still could do "> /dev/null 2>&1" which is just as short (or long) as a newly introduced "--really-quiet" option. But I'm also concerned about consistency and making options do what they sound they would do. > In "git rev-list ... | git diff-tree --stdin" output, the commit > object name is absolutely necessary, with or without --quiet, as it Why is printing the object name also necessary with "--quiet"? I'd argue that any script that uses diff-tree that way uses --stdin without --quiet, just like you do in your example, so suppressing the object name if "--quiet" is given probably would not break as many scripts as you think. > But we do not live in an ideal world. True, but we should never stop striving after making it one :-) -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
On Thu, Jul 23, 2015 at 8:08 PM, Jeff King wrote: > mode. Actually asking for a two-endpoint tree diff: > > git diff-tree --quiet --ignore-space-change $commit^ $commit > > will do what you want. Yes, I know, thanks. But I deliberately wanted to specify only a single commit as an optimization, hoping that it would be slightly faster than computing a commit range. -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
Jeff King writes: > I have not been following the thread closely, but I do not recall seeing > anyone mention that the reason for the sha1-output is handing > only a single commit-ish to diff-tree is what puts it into its log-like > mode. Actually asking for a two-endpoint tree diff: > > git diff-tree --quiet --ignore-space-change $commit^ $commit > > will do what you want. Yeah, if we were living in an ideal world equipped with a time machine, I would redesign "git diff-tree $commit" so that it does not show the commit object name in its output at all, with or without "--quiet". In "git rev-list ... | git diff-tree --stdin" output, the commit object name is absolutely necessary, with or without --quiet, as it serves as the sign that the output switched to talk about a different commit. But the case that feeds a single commit to the command, used as a poor-man's "git show $commit", does not need one---the caller knows exactly which commit the output is about. It is an unfortunate historical accident that a single commit usage is defined to be a degenerate case of feeding a sequence of commits to the command and the length of the sequence happens to be one. "diff-tree $commit" could instead have been defined as a short-hand for "diff-tree $commit^ $commit", but (1) we do not live in an ideal world, and (2) it ignores $commit^2 and later parents. This is a tangent, but I suspect that the current implementation of "diff-tree --stdin --quiet" may be buggy and does not consistently show the commits that touch the given path. $ git rev-list master..jc/rerere | git diff-tree --stdin -s rerere.h gives what is expected (shows the commit object names, but being silent on the differences), while s/-s/--quiet/ seems to omit every other commit from the output, or something silly like that. I haven't dug into why that happens, but possible ways to fix that are to make "--quiet" output all (making it consistent with "-s") or no (making the command totally silent) output at all ;-). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
On Thu, Jul 23, 2015 at 09:06:01AM +0200, Sebastian Schuberth wrote: > My use-case (also see [1]) is that I wanted to checked whether some > given commits change nothing but whitespace. So I did > > if git diff-tree --quiet --ignore-space-change $commit; then > echo "$commit only changes whitespace." > fi > > just to see those SHA1s being printed to the console. > > I probably could instead do > > if git diff-tree --exit-code --ignore-space-change $commit > /dev/null > 2>&1; then > echo "$commit only changes whitespace." > fi > > but that defeats the purpose of having "--quiet" in the first place. I have not been following the thread closely, but I do not recall seeing anyone mention that the reason for the sha1-output is handing only a single commit-ish to diff-tree is what puts it into its log-like mode. Actually asking for a two-endpoint tree diff: git diff-tree --quiet --ignore-space-change $commit^ $commit will do what you want. I know that does not necessarily help the greater issue of "what diff-tree is doing is confusing", but perhaps that sheds some light at least on why it is doing what it is doing. :) -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
Junio C Hamano writes: > Sebastian Schuberth writes: > >> Well, from a user's perspective it does not matter which part of the >> internal implementation of diff-tree is responsible for printing that >> single line,... > > That is not "internal implementation", but "logically separate > parts". View it more like "'git show -s' does squelch the diff part > but does not squelch the log output". After all, a single commit form > of 'diff-tree' is a degenerate use case of feeding a single commit > to 'diff-tree --stdin' from its standard input, which is a rough > plumbing-level equivalent of 'show'. > > Documenting the behaviour correctly is the best thing you could do > at this point, as this is one of the oldest part of the system that > existing scripts would rely on. Having said that. Existing scripts by definition would not be using a new option you will invent that used not to be a valid one. So that would be one way that you can shorten your script without breaking other people. If we were living in an ideal world equipped with a time machine, I would redesign "git diff-tree $commit" so that it does not show the commit object name in its output at all, with or without "--quiet". In "git rev-list ... | git diff-tree --stdin" output, the commit object name is absolutely necessary, with or without --quiet, as it serves as the sign that the output switched to talk about a different commit. But the case that feeds a single commit to the command, used as a poor-man's "git show $commit", does not need one---the caller knows exactly which commit the output is about. It is an unfortunate historical accident that a single commit usage is defined to be a degenerate case of feeding a sequence of commits to the command and the length of the sequence happens to be one. But we do not live in an ideal world. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
Sebastian Schuberth writes: > Well, from a user's perspective it does not matter which part of the > internal implementation of diff-tree is responsible for printing that > single line,... That is not "internal implementation", but "logically separate parts". View it more like "'git show -s' does squelch the diff part but does not squelch the log output". After all, a single commit form of 'diff-tree' is a degenerate use case of feeding a single commit to 'diff-tree --stdin' from its standard input, which is a rough plumbing-level equivalent of 'show'. Documenting the behaviour correctly is the best thing you could do at this point, as this is one of the oldest part of the system that existing scripts would rely on. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
On Wed, Jul 22, 2015 at 10:32 PM, Junio C Hamano wrote: >> "--quite" is documented to "Disable all output of the program". Yet >> calling diff-tree with a single commit like >> >> $ git diff-tree --quiet c925fe2 >> >> was logging >> >> c925fe23684455735c3bb1903803643a24a58d8f > > At this point, unfortunately I think we need to call that a > documentation bug. The "output" it refers to is output from the > "diff" portion, not the "poor-man's log" portion, of the program, > where diff-tree was the workhorse behind scripted "git log" that > gave the commit object name as the preamble for each commit it > shows information about. Well, from a user's perspective it does not matter which part of the internal implementation of diff-tree is responsible for printing that single line, a user would just expect "--quiet" to really mean "quiet". As for almost any bug, we could turn it into a feature by "fixing" the docs and claiming it's documented behavior. To me the question simply is whether it makes sense for "--quiet" to not be quiet, and I think it does not make sense. If you run diff-tree this way there is no added value in the given output. My use-case (also see [1]) is that I wanted to checked whether some given commits change nothing but whitespace. So I did if git diff-tree --quiet --ignore-space-change $commit; then echo "$commit only changes whitespace." fi just to see those SHA1s being printed to the console. I probably could instead do if git diff-tree --exit-code --ignore-space-change $commit > /dev/null 2>&1; then echo "$commit only changes whitespace." fi but that defeats the purpose of having "--quiet" in the first place. [1] http://article.gmane.org/gmane.comp.version-control.git/273975 -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
Sebastian Schuberth writes: > "--quite" is documented to "Disable all output of the program". Yet > calling diff-tree with a single commit like > > $ git diff-tree --quiet c925fe2 > > was logging > > c925fe23684455735c3bb1903803643a24a58d8f At this point, unfortunately I think we need to call that a documentation bug. The "output" it refers to is output from the "diff" portion, not the "poor-man's log" portion, of the program, where diff-tree was the workhorse behind scripted "git log" that gave the commit object name as the preamble for each commit it shows information about. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] diff-tree: do not show the sha1 of the given head with --quiet
On 2015-07-22 11:29, Sebastian Schuberth wrote: > "--quite" is documented to "Disable all output of the program". s/--quite/quiet/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
