* amend warnings with no changes staged @ 2019-08-06 0:28 Lukas Gross 2019-08-06 1:30 ` Jonathan Nieder 0 siblings, 1 reply; 13+ messages in thread From: Lukas Gross @ 2019-08-06 0:28 UTC (permalink / raw) To: git Hi, I have occasionally used git commit --amend without staging any changes or modifying the commit message (--no-edit). Since this is often done unintentionally, could amend warn when it is being used in this way? Thanks! ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: amend warnings with no changes staged 2019-08-06 0:28 amend warnings with no changes staged Lukas Gross @ 2019-08-06 1:30 ` Jonathan Nieder 2019-08-06 1:37 ` Lukas Gross 0 siblings, 1 reply; 13+ messages in thread From: Jonathan Nieder @ 2019-08-06 1:30 UTC (permalink / raw) To: Lukas Gross; +Cc: git Hi, Lukas Gross wrote: > I have occasionally used git commit --amend without staging any > changes or modifying the commit message (--no-edit). Since this is > often done unintentionally, could amend warn when it is being used in > this way? Can you say more about the context? What were you trying to do when you performed this operation? What happened instead? Thanks, Jonathan ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: amend warnings with no changes staged 2019-08-06 1:30 ` Jonathan Nieder @ 2019-08-06 1:37 ` Lukas Gross 2019-08-06 2:01 ` Jonathan Nieder 2019-08-06 2:16 ` Jonathan Nieder 0 siblings, 2 replies; 13+ messages in thread From: Lukas Gross @ 2019-08-06 1:37 UTC (permalink / raw) To: Jonathan Nieder; +Cc: git Hi Jonathan, I had intended to stage commits but forgot to do so. Git responded with a normal commit creation message, so I pushed to the remote to begin a CI build. When the build failed for the same reason, I realized I had forgotten to stage the changes. An additional line in the response to the effect of “Warning: did you mean to amend with no changes?” would be very helpful to shorten this feedback loop. Lukas On 8/5/19, Jonathan Nieder <jrnieder@gmail.com> wrote: > Hi, > > Lukas Gross wrote: > >> I have occasionally used git commit --amend without staging any >> changes or modifying the commit message (--no-edit). Since this is >> often done unintentionally, could amend warn when it is being used in >> this way? > > Can you say more about the context? What were you trying to do when > you performed this operation? What happened instead? > > Thanks, > Jonathan > -- *Lukas Gross* B.S. Computer Science | McCormick School of Engineering Northwestern University | Class of 2020 (224) 522-9067 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: amend warnings with no changes staged 2019-08-06 1:37 ` Lukas Gross @ 2019-08-06 2:01 ` Jonathan Nieder 2019-08-06 2:16 ` Jonathan Nieder 1 sibling, 0 replies; 13+ messages in thread From: Jonathan Nieder @ 2019-08-06 2:01 UTC (permalink / raw) To: Lukas Gross; +Cc: git (administrivia: please don't top-post) Lukas Gross wrote: > I had intended to stage commits but forgot to do so. Git responded > with a normal commit creation message, so I pushed to the remote to > begin a CI build. When the build failed for the same reason, I > realized I had forgotten to stage the changes. An additional line in > the response to the effect of “Warning: did you mean to amend with no > changes?” would be very helpful to shorten this feedback loop. Ah, I see. You passed --no-edit so you didn't see the usual --dry-run style output that "git commit" shows. You forgot to run "git add" before amending, and this is what you'd like commit to assist you with. That said, this is sometimes an operation people perform intentionally. Ideas: * Can the documentation do a better job of steering people away from --no-edit? The hints shown when "git commit --amend" (and --no-amend, for that matter) open a text editor tend to be very helpful for understanding what is going to happen. If any documentation is leading people to forgo that tool, we should fix it. * Should "git commit --no-edit" say a little more about what it did, since it knows that the user didn't get to see the text editor? * Should we have a special option (e.g. "git commit --amend --refresh") for a no-op amend? That ways, when a user doesn't pass that option, it would be more clear that it is a mistake, allowing a warning or even an error. Of these three, the first seems most compelling to me. Others may have other ideas. Thoughts? Thanks, Jonathan ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: amend warnings with no changes staged 2019-08-06 1:37 ` Lukas Gross 2019-08-06 2:01 ` Jonathan Nieder @ 2019-08-06 2:16 ` Jonathan Nieder 2019-08-06 2:47 ` Junio C Hamano 2019-08-06 4:19 ` Jeff King 1 sibling, 2 replies; 13+ messages in thread From: Jonathan Nieder @ 2019-08-06 2:16 UTC (permalink / raw) To: Lukas Gross; +Cc: git Lukas Gross wrote: > I had intended to stage commits but forgot to do so. Git responded > with a normal commit creation message, so I pushed to the remote to > begin a CI build. When the build failed for the same reason, I > realized I had forgotten to stage the changes. An additional line in > the response to the effect of “Warning: did you mean to amend with no > changes?” would be very helpful to shorten this feedback loop. On second thought: $ git commit --amend --no-edit [detached HEAD 33a3db8805] Git 2.23-rc1 Author: Junio C Hamano <gitster@pobox.com> Date: Fri Aug 2 13:12:24 2019 -0700 2 files changed, 2 insertions(+), 1 deletion(-) $ Some non-judgemental descriptive output like $ git commit --amend --no-edit No changes. $ would address this case, without bothering people who are doing it intentionally. So I think there's room for a simple improvement here. Care to take a stab at it? builtin/commit.c would be the place to start. Thanks and sorry for the roller-coaster, Jonathan ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: amend warnings with no changes staged 2019-08-06 2:16 ` Jonathan Nieder @ 2019-08-06 2:47 ` Junio C Hamano 2019-08-06 3:00 ` Jonathan Nieder 2019-08-06 4:19 ` Jeff King 1 sibling, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2019-08-06 2:47 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Lukas Gross, git Jonathan Nieder <jrnieder@gmail.com> writes: > Lukas Gross wrote: > >> I had intended to stage commits but forgot to do so. Git responded >> with a normal commit creation message, so I pushed to the remote to >> begin a CI build. When the build failed for the same reason, I >> realized I had forgotten to stage the changes. An additional line in >> the response to the effect of “Warning: did you mean to amend with no >> changes?” would be very helpful to shorten this feedback loop. > > On second thought: > > $ git commit --amend --no-edit > [detached HEAD 33a3db8805] Git 2.23-rc1 > Author: Junio C Hamano <gitster@pobox.com> > Date: Fri Aug 2 13:12:24 2019 -0700 > 2 files changed, 2 insertions(+), 1 deletion(-) > $ > > Some non-judgemental descriptive output like > > $ git commit --amend --no-edit > No changes. > $ > > would address this case, without bothering people who are doing it > intentionally. So I think there's room for a simple improvement here. I do that to refresh the committer timestamp. Do I now have to say something silly like $ GIT_EDITOR=true git commit --amend to defeat the above (mis)feature, or is there a cleaner workaround? Obviously I'd prefer to see a solution that does not force existing users to work around the new behaviour ;-) > Care to take a stab at it? builtin/commit.c would be the place to > start. I'd suggest not to start it before we know what we want. I am not yet convinced that "--amend --no-edit will become a no-op" is the final solution we want. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: amend warnings with no changes staged 2019-08-06 2:47 ` Junio C Hamano @ 2019-08-06 3:00 ` Jonathan Nieder 2019-08-06 3:29 ` Junio C Hamano 0 siblings, 1 reply; 13+ messages in thread From: Jonathan Nieder @ 2019-08-06 3:00 UTC (permalink / raw) To: Junio C Hamano; +Cc: Lukas Gross, git Junio C Hamano wrote: > Jonathan Nieder <jrnieder@gmail.com> writes: >> Lukas Gross wrote: >>> I had intended to stage commits but forgot to do so. Git responded >>> with a normal commit creation message, so I pushed to the remote to >>> begin a CI build. When the build failed for the same reason, I >>> realized [...] >> $ git commit --amend --no-edit >> [detached HEAD 33a3db8805] Git 2.23-rc1 >> Author: Junio C Hamano <gitster@pobox.com> >> Date: Fri Aug 2 13:12:24 2019 -0700 >> 2 files changed, 2 insertions(+), 1 deletion(-) >> $ >> >> Some non-judgemental descriptive output like >> >> $ git commit --amend --no-edit >> No changes. >> $ >> >> would address this case, without bothering people who are doing it >> intentionally. So I think there's room for a simple improvement here. > > I do that to refresh the committer timestamp. I do, too. The proposal is, paraphrasing, $ git commit --amend --no-edit Ah, I see that you want me to refresh the committer timestamp. Done, as requested. $ In other words: [...] > I am not > yet convinced that "--amend --no-edit will become a no-op" is the > final solution we want. Not this. Hoping that clarifies, Jonathan ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: amend warnings with no changes staged 2019-08-06 3:00 ` Jonathan Nieder @ 2019-08-06 3:29 ` Junio C Hamano 2019-08-06 3:53 ` Junio C Hamano 0 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2019-08-06 3:29 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Lukas Gross, git Jonathan Nieder <jrnieder@gmail.com> writes: >>> Some non-judgemental descriptive output like >>> >>> $ git commit --amend --no-edit >>> No changes. >>> $ >>> >>> would address this case, without bothering people who are doing it >>> intentionally. So I think there's room for a simple improvement here. >> >> I do that to refresh the committer timestamp. > > I do, too. The proposal is, paraphrasing, > > $ git commit --amend --no-edit > Ah, I see that you want me to refresh the committer timestamp. > Done, as requested. > $ Ah, OK then. I somehow misread "No changes." as an error message. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: amend warnings with no changes staged 2019-08-06 3:29 ` Junio C Hamano @ 2019-08-06 3:53 ` Junio C Hamano 2019-08-06 16:32 ` Phillip Wood 0 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2019-08-06 3:53 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Lukas Gross, git Junio C Hamano <gitster@pobox.com> writes: > Jonathan Nieder <jrnieder@gmail.com> writes: > >>>> Some non-judgemental descriptive output like >>>> >>>> $ git commit --amend --no-edit >>>> No changes. >>>> $ >>>> >>>> would address this case, without bothering people who are doing it >>>> intentionally. So I think there's room for a simple improvement here. >>> >>> I do that to refresh the committer timestamp. >> >> I do, too. The proposal is, paraphrasing, >> >> $ git commit --amend --no-edit >> Ah, I see that you want me to refresh the committer timestamp. >> Done, as requested. >> $ > > Ah, OK then. I somehow misread "No changes." as an error message. Well, on second thought, I think "fatal: no changes" that exits with non-zero, with "--force" as an escape hatch for those who want to refresh the committer timestamp, would probably be more in line with the expectation Lukas had when this thread was started, and I further suspect that it might be a bit more end-user friendly. It is a backward incompatible behaviour, but I suspect that if I were inventing "commit --amend" today, unlike 8588452c ("git-commit --amend: allow empty commit.", 2006-03-04), I probably would design it that way. After all, failing and stopping is always a safer option than going ahead with or without a report. I am not sure which one between "go ahead anyway but report" and "fail by default but allow forcing" I would prefer more. At least not yet. But I won't rule the latter out at this point. Thanks. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: amend warnings with no changes staged 2019-08-06 3:53 ` Junio C Hamano @ 2019-08-06 16:32 ` Phillip Wood 0 siblings, 0 replies; 13+ messages in thread From: Phillip Wood @ 2019-08-06 16:32 UTC (permalink / raw) To: Junio C Hamano, Jonathan Nieder; +Cc: Lukas Gross, git, Jeff King On 06/08/2019 04:53, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > >> Jonathan Nieder <jrnieder@gmail.com> writes: >> >>>>> Some non-judgemental descriptive output like >>>>> >>>>> $ git commit --amend --no-edit >>>>> No changes. >>>>> $ >>>>> >>>>> would address this case, without bothering people who are doing it >>>>> intentionally. So I think there's room for a simple improvement here. >>>> >>>> I do that to refresh the committer timestamp. >>> >>> I do, too. The proposal is, paraphrasing, >>> >>> $ git commit --amend --no-edit >>> Ah, I see that you want me to refresh the committer timestamp. >>> Done, as requested. >>> $ >> >> Ah, OK then. I somehow misread "No changes." as an error message. > > Well, on second thought, I think "fatal: no changes" that exits with > non-zero, with "--force" as an escape hatch for those who want to > refresh the committer timestamp, would probably be more in line with > the expectation Lukas had when this thread was started, and I further > suspect that it might be a bit more end-user friendly. I agree that this would be the must user friendly way of doing it. I think refreshing the timestamp is probably a niche use of '--amend' (out of interest what the the motivation for doing that?) although it does seem to popular with the other contributors to this thread. We could always have a transition period with an opt-in config variable. I find the current behavior quite annoying when I've forgotten to stage anything as there is no indication the the commit's content is unchanged. I've been using a wrapper script that errors out if there are no new changes staged with --amend --no-edit and found it very helpful (as is a proper --reword option rather than having to give --amend --only) Best Wishes Phillip > > It is a backward incompatible behaviour, but I suspect that if I > were inventing "commit --amend" today, unlike 8588452c ("git-commit > --amend: allow empty commit.", 2006-03-04), I probably would design > it that way. After all, failing and stopping is always a safer > option than going ahead with or without a report. > > I am not sure which one between "go ahead anyway but report" and > "fail by default but allow forcing" I would prefer more. At least > not yet. But I won't rule the latter out at this point. > > Thanks. > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: amend warnings with no changes staged 2019-08-06 2:16 ` Jonathan Nieder 2019-08-06 2:47 ` Junio C Hamano @ 2019-08-06 4:19 ` Jeff King 2019-08-06 19:11 ` Junio C Hamano 1 sibling, 1 reply; 13+ messages in thread From: Jeff King @ 2019-08-06 4:19 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Lukas Gross, git On Mon, Aug 05, 2019 at 07:16:18PM -0700, Jonathan Nieder wrote: > Lukas Gross wrote: > > > I had intended to stage commits but forgot to do so. Git responded > > with a normal commit creation message, so I pushed to the remote to > > begin a CI build. When the build failed for the same reason, I > > realized I had forgotten to stage the changes. An additional line in > > the response to the effect of “Warning: did you mean to amend with no > > changes?” would be very helpful to shorten this feedback loop. > > On second thought: > > $ git commit --amend --no-edit > [detached HEAD 33a3db8805] Git 2.23-rc1 > Author: Junio C Hamano <gitster@pobox.com> > Date: Fri Aug 2 13:12:24 2019 -0700 > 2 files changed, 2 insertions(+), 1 deletion(-) > $ > > Some non-judgemental descriptive output like > > $ git commit --amend --no-edit > No changes. > $ > > would address this case, without bothering people who are doing it > intentionally. So I think there's room for a simple improvement here. > > Care to take a stab at it? builtin/commit.c would be the place to > start. I'm not clear on the situation from your second change. There are two sets of changes to talk about here: the changes between the new commit and its parent, and the changes between the original commit and the amended version. The output in your first example is showing the differences to the parent. Do you mean in the second example that there are no changes to the parent, and thus we say "No changes"? If not, then what happened to that output? :) And if so, then I don't think it is solving Lukas's problem. I imagine the issue to be (because I have done this myself many times): git commit -m 'buggy commit' echo fix >>file.c git commit --amend ;# oops, should have been "-a" git push But perhaps that gets to the heart of the matter. Could we perhaps be providing a more detailed summary of what happened for an --amend? I.e., to summarize _both_ sets of changes (and if one set is empty, say so)? I'm not quite sure how to make that readable. Something like: $ git commit --amend [master 5787bce] some commit subject Date: Tue Aug 6 00:03:28 2019 -0400 Changes introduced by this commit (diff HEAD^): 1 file changed, 1 insertion(+) create mode 100644 added-by-broken-commit Changes from the amend commit (diff HEAD@{1}): 1 file changed, 1 insertion(+) create mode 100644 added-during-amend is pretty ugly. And anyway, because it's just the diffstat total, it's hard to see whether it contains your changes or not (i.e., would you notice if you forgot to stage 3 lines from some random file). OTOH, if the common case we care about is just "you didn't stage anything as part of the amend", then it would be enough to let you know (without making a judgement about whether it's an error, since it may well be that you were simply rewording the commit message). -Peff ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: amend warnings with no changes staged 2019-08-06 4:19 ` Jeff King @ 2019-08-06 19:11 ` Junio C Hamano 2019-08-08 9:46 ` Jeff King 0 siblings, 1 reply; 13+ messages in thread From: Junio C Hamano @ 2019-08-06 19:11 UTC (permalink / raw) To: Jeff King; +Cc: Jonathan Nieder, Lukas Gross, git Jeff King <peff@peff.net> writes: > git commit -m 'buggy commit' > echo fix >>file.c > git commit --amend ;# oops, should have been "-a" > git push > > But perhaps that gets to the heart of the matter. Could we perhaps be > providing a more detailed summary of what happened for an --amend? I.e., > to summarize _both_ sets of changes (and if one set is empty, say so)? > ... > judgement about whether it's an error, since it may well be that you > were simply rewording the commit message). Perhaps "git range-diff HEAD@{1}...HEAD" being an empty is a sign that either the user intentionally or accidentally did not do anything other than "touch"ing the commit. "git commit --amend --[no-]range-diff" that shows what you changed with the amending may be an interesting possibility; I am not yet ready to seriously encourage anybody to explore it, though, because "git diff HEAD@{1}" is much easier to see what code got changed, but one (and probably only) downside is that it does not cover the change in the log message. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: amend warnings with no changes staged 2019-08-06 19:11 ` Junio C Hamano @ 2019-08-08 9:46 ` Jeff King 0 siblings, 0 replies; 13+ messages in thread From: Jeff King @ 2019-08-08 9:46 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jonathan Nieder, Lukas Gross, git On Tue, Aug 06, 2019 at 12:11:53PM -0700, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > git commit -m 'buggy commit' > > echo fix >>file.c > > git commit --amend ;# oops, should have been "-a" > > git push > > > > But perhaps that gets to the heart of the matter. Could we perhaps be > > providing a more detailed summary of what happened for an --amend? I.e., > > to summarize _both_ sets of changes (and if one set is empty, say so)? > > ... > > judgement about whether it's an error, since it may well be that you > > were simply rewording the commit message). > > Perhaps "git range-diff HEAD@{1}...HEAD" being an empty is a sign > that either the user intentionally or accidentally did not do > anything other than "touch"ing the commit. > > "git commit --amend --[no-]range-diff" that shows what you changed > with the amending may be an interesting possibility; I am not yet > ready to seriously encourage anybody to explore it, though, because > "git diff HEAD@{1}" is much easier to see what code got changed, but > one (and probably only) downside is that it does not cover the > change in the log message. I hadn't even thought of range-diff. Showing the commit message diff would be a big improvement. A range-diff would also show an update to the author (e.g., if you used --reset-author). We don't really need the full power of range diff, though. After all, we know there are exactly two patches to compare, so we don't care about it trying to figure out which ones correlate (and it might even be a bad thing for it to decide that two entries don't match). So I think we'd probably want our own custom thing. -Peff ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-08-08 9:46 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-06 0:28 amend warnings with no changes staged Lukas Gross 2019-08-06 1:30 ` Jonathan Nieder 2019-08-06 1:37 ` Lukas Gross 2019-08-06 2:01 ` Jonathan Nieder 2019-08-06 2:16 ` Jonathan Nieder 2019-08-06 2:47 ` Junio C Hamano 2019-08-06 3:00 ` Jonathan Nieder 2019-08-06 3:29 ` Junio C Hamano 2019-08-06 3:53 ` Junio C Hamano 2019-08-06 16:32 ` Phillip Wood 2019-08-06 4:19 ` Jeff King 2019-08-06 19:11 ` Junio C Hamano 2019-08-08 9:46 ` Jeff King
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).