Should "git apply --check" imply verbose?
TL;DR -- "git apply --reject" implies verbose, but the similar "git apply --check" does not, which seems inconsistent.
Background: A common (non-git) workflow can be to use "patch --dry-run" to inspect whether a patch is feasible, and then use patch again a 2nd time (w/o --dry-run) to actually apply it (and then work through the rejects).
You can also do the above in a git repo, but you lose out because "patch" doesn't (yet) capture the patched function names[1] in the rejected hunks, making it hard to double check your work.
My initial thought was to replace the above two steps with "git apply --check ..." and then "git apply --reject ..." so that I could just abandon using patch altogether.
That works great, with just one snag that had me go reading the source. It seems that "git apply --reject" is verbose, and kind of looks like the identical output I'd get if I used patch. But "git apply --check" is quite reserved in its output and doesn't look at all like "patch --dry-run". I initially believed that "--check" was stopping at the 1st failure, based on the output.
Only when I read the source did I realize it was checking all the hunks silently, and adding a "-v" would make it similar to the output from "patch --dry-run".
Not a critical issue by any means, but having the "-v" implied by "--check" (or perhaps having both default to non-verbose?) might save other users from getting confused in the same way.
Thanks,
Paul.
--
Quoted 2 lines · Paul Gortmaker
TL;DR -- "git apply --reject" implies verbose, but the similar "git apply --check" does not, which seems inconsistent.
Hmmm, I am of two minds. From purely idealistic point of view, I can see why defaulting both to non-verbose may look a more attractive way to go, but I have my reservations that is more than the usual change-aversion.
Historically, "check" was primarily meant to see if the patch is applicable cleanly in scripts, and we never thought it would make any sense to make it verbose by default.
On the other hand, the operation of "reject", which was a much later invention, was primarily meant to be observed by humans to see how the patch failed to cleanly apply and where, to help them decide where to look in the target to wiggle the rejected hunk into (even when it is driven from a script). It did not make much sense to squelch its output.
In addition, because "check" is an idempotent operation that does not touch anything in the index or the working tree, running with "check" and then "check verbose" is possible if somebody runs it without verbose and then decides later that s/he wants to see the details. But "reject" does touch the working tree files with applicable hunks, so after a quiet "reject", there is no way to see the verbose output like you can with "check".
Quoted 9 lines · Junio C Hamano
Quoted 2 lines · Paul Gortmaker
TL;DR -- "git apply --reject" implies verbose, but the similar "git apply --check" does not, which seems inconsistent.
Hmmm, I am of two minds. From purely idealistic point of view, I can see why defaulting both to non-verbose may look a more attractive way to go, but I have my reservations that is more than the usual change-aversion.
OK, so given your feedback, how do you feel about a patch to the documentation that indicates to use "-v" in combination with the
"--check" to get equivalent "patch --dry-run" behaviour? If that
had existed, I'd have not gone rummaging around in the source, so that should be good enough to help others avoid the same...
P.
--
Quoted 20 lines
Historically, "check" was primarily meant to see if the patch is applicable cleanly in scripts, and we never thought it would make any sense to make it verbose by default.
On the other hand, the operation of "reject", which was a much later invention, was primarily meant to be observed by humans to see how the patch failed to cleanly apply and where, to help them decide where to look in the target to wiggle the rejected hunk into (even when it is driven from a script). It did not make much sense to squelch its output.
In addition, because "check" is an idempotent operation that does not touch anything in the index or the working tree, running with "check" and then "check verbose" is possible if somebody runs it without verbose and then decides later that s/he wants to see the details. But "reject" does touch the working tree files with applicable hunks, so after a quiet "reject", there is no way to see the verbose output like you can with "check".
Hi Paul,
Quoted 3 lines · Paul Gortmaker
OK, so given your feedback, how do you feel about a patch to the documentation that indicates to use "-v" in combination with the "--check" to get equivalent "patch --dry-run" behaviour?
Sounds like a good idea to me.
I assume you mean a note in the OPTIONS or EXAMPLES section of Documentation/git-apply.txt?
Thanks,
Jonathan
Quoted 4 lines · Jonathan Nieder
Hi Paul,
Paul Gortmaker wrote:
Quoted 8 lines
Quoted 3 lines
OK, so given your feedback, how do you feel about a patch to the documentation that indicates to use "-v" in combination with the "--check" to get equivalent "patch --dry-run" behaviour?
Sounds like a good idea to me.
I assume you mean a note in the OPTIONS or EXAMPLES section of
Documentation/git-apply.txt?
I hadn't looked exactly where yet, but wherever makes sense and wherever appears in TFM.
P.
--
Quoted 4 lines
Thanks,
Jonathan
Quoted 5 lines · Paul Gortmaker
OK, so given your feedback, how do you feel about a patch to the documentation that indicates to use "-v" in combination with the
"--check" to get equivalent "patch --dry-run" behaviour? If that
had existed, I'd have not gone rummaging around in the source, so that should be good enough to help others avoid the same...
I do not think it is necessarily a good idea to assume that people who are learning "git apply" know how GNU patch works.
But I do agree that the description of -v, --verbose has a lot of room for improvement.
Report progress to stderr. By default, only a message about the current patch being applied will be printed. This option will cause additional information to be reported.
It is totally unclear what "additional information" is reported at all.
Thanks.
Quoted 10 lines · On Tue, 20 Aug 2013 12:07:18 -0700 Junio C Hamano <[email protected]> wrote:
Quoted 5 lines · Paul Gortmaker
OK, so given your feedback, how do you feel about a patch to the documentation that indicates to use "-v" in combination with the
"--check" to get equivalent "patch --dry-run" behaviour? If that
had existed, I'd have not gone rummaging around in the source, so that should be good enough to help others avoid the same...I do not think it is necessarily a good idea to assume that people who are learning "git apply" know how GNU patch works.
Linus told me that "git apply" was basically a replacement for patch. Why would you think it would not be a good idea to assume that people would not be familiar with how GNU patch works?
Is it because you expect "git apply" to eventually replace patch all out, and want no dependencies on its knowledge?
-- Steve
Quoted 12 lines
But I do agree that the description of -v, --verbose has a lot of room for improvement.
Report progress to stderr. By default, only a message about the current patch being applied will be printed. This option will cause additional information to be reported.
It is totally unclear what "additional information" is reported at all.
Thanks.
Quoted 6 lines · Steven Rostedt
Quoted 2 lines
I do not think it is necessarily a good idea to assume that people who are learning "git apply" know how GNU patch works.
Linus told me that "git apply" was basically a replacement for patch. Why would you think it would not be a good idea to assume that people would not be familiar with how GNU patch works?
The audience of Git these days are far more widely spread than the kernel circle. I am not opposed to _helping_ those who happen to know "patch", but I was against a description that assumes readers know it, i.e. making it a requirement to know "patch" to understand "apply".
Quoted 9 lines
Quoted 9 lines
But I do agree that the description of -v, --verbose has a lot of room for improvement.
Report progress to stderr. By default, only a message about the current patch being applied will be printed. This option will cause additional information to be reported.
It is totally unclear what "additional information" is reported at all.
In other words, your enhancement to the documentation could go like:
... By default, ... With this option, you will additionally see such and such and such in the output (this is similar to what "patch --dry-run" would give you). See the EXAMPLES section to get a feel of how it looks like.
and I would not be opposed, as long as "such and such and such" are written in such a way that the reader does not have to have a prior experience with GNU patch in order to understand it.
Clear?
Quoted 9 lines · On Tue, 20 Aug 2013 12:45:03 -0700 Junio C Hamano <[email protected]> wrote:
Quoted 6 lines · Steven Rostedt
Quoted 2 lines
I do not think it is necessarily a good idea to assume that people who are learning "git apply" know how GNU patch works.
Linus told me that "git apply" was basically a replacement for patch. Why would you think it would not be a good idea to assume that people would not be familiar with how GNU patch works?
Quoted 5 lines
The audience of Git these days are far more widely spread than the kernel circle. I am not opposed to _helping_ those who happen to know "patch", but I was against a description that assumes readers know it, i.e. making it a requirement to know "patch" to understand "apply".
Patch is used by much more than just the kernel folks ;-) I've been using patch much longer than I've been doing kernel development.
Quoted 18 lines
Quoted 10 lines
Quoted 10 lines
But I do agree that the description of -v, --verbose has a lot of room for improvement.
Report progress to stderr. By default, only a message about the current patch being applied will be printed. This option will cause additional information to be reported.
It is totally unclear what "additional information" is reported at all.
In other words, your enhancement to the documentation could go like:
... By default, ... With this option, you will additionally see such and such and such in the output (this is similar to what "patch --dry-run" would give you). See the EXAMPLES section to get a feel of how it looks like.
Quoted 5 lines
and I would not be opposed, as long as "such and such and such" are written in such a way that the reader does not have to have a prior experience with GNU patch in order to understand it.
Clear?
Looks good to me. Paul, what do you think?
Thanks,
-- Steve
Quoted 40 lines · Steven Rostedt
Quoted 9 lines · On Tue, 20 Aug 2013 12:45:03 -0700 Junio C Hamano <[email protected]> wrote:
Quoted 6 lines · Steven Rostedt
Quoted 2 lines
I do not think it is necessarily a good idea to assume that people who are learning "git apply" know how GNU patch works.
Linus told me that "git apply" was basically a replacement for patch. Why would you think it would not be a good idea to assume that people would not be familiar with how GNU patch works?
Quoted 5 lines
The audience of Git these days are far more widely spread than the kernel circle. I am not opposed to _helping_ those who happen to know "patch", but I was against a description that assumes readers know it, i.e. making it a requirement to know "patch" to understand "apply".
Patch is used by much more than just the kernel folks ;-) I've been using patch much longer than I've been doing kernel development.
Quoted 18 lines
Quoted 10 lines
Quoted 10 lines
But I do agree that the description of -v, --verbose has a lot of room for improvement.
Report progress to stderr. By default, only a message about the current patch being applied will be printed. This option will cause additional information to be reported.
It is totally unclear what "additional information" is reported at all.
In other words, your enhancement to the documentation could go like:
... By default, ... With this option, you will additionally see such and such and such in the output (this is similar to what "patch --dry-run" would give you). See the EXAMPLES section to get a feel of how it looks like.
Quoted 7 lines
Quoted 5 lines
and I would not be opposed, as long as "such and such and such" are written in such a way that the reader does not have to have a prior experience with GNU patch in order to understand it.
Clear?
Looks good to me. Paul, what do you think?
Yep, I'll write something up tomorrow which loosely matches the above.
Thanks,
Paul.
--
Quoted 5 lines
Thanks,
-- Steve
Quoted 3 lines · Paul Gortmaker
Quoted 1 line
Looks good to me. Paul, what do you think?
Yep, I'll write something up tomorrow which loosely matches the above.
Thanks.
Quoted 4 lines · Steven Rostedt
Quoted 4 lines
Quoted 3 lines
Linus told me that "git apply" was basically a replacement for patch. Why would you think it would not be a good idea to assume that people would not be familiar with how GNU patch works?
Quoted 8 lines
Quoted 5 lines
The audience of Git these days are far more widely spread than the kernel circle. I am not opposed to _helping_ those who happen to know "patch", but I was against a description that assumes readers know it, i.e. making it a requirement to know "patch" to understand "apply".
Patch is used by much more than just the kernel folks ;-) I've been using patch much longer than I've been doing kernel development.
Yeah, I was familiar with "patch" when I started Git, too ;-).
But only folks in the kernel circle will be told by Linus the similarity between apply and patch, no?
In any case...
Quoted 10 lines
Quoted 10 lines
In other words, your enhancement to the documentation could go like:
... By default, ... With this option, you will additionally see such and such and such in the output (this is similar to what "patch --dry-run" would give you). See the EXAMPLES section to get a feel of how it looks like.
and I would not be opposed, as long as "such and such and such" are written in such a way that the reader does not have to have a prior experience with GNU patch in order to understand it.
... I forgot to also add: And by mentioning "similar to", people who are familiar with "patch" are also helped by their pre-existing knowledge, so both kinds of people win.
Thanks.
Quoted 2 lines · On Tue, 20 Aug 2013 14:43:56 -0700 Junio C Hamano <[email protected]> wrote:
But only folks in the kernel circle will be told by Linus the similarity between apply and patch, no?
Well, there was a time when Linus was making his rounds showcasing git more than Linux, to people that were not kernel developers.
-- Steve