{"thread":{"id":"52277","subject":"[RFC PATCH] wt-status: show amended content when verbose","startedAt":"2019-11-16T16:19:05Z","lastAt":"2019-11-20T01:04:36Z","messageCount":6,"participants":["Pratyush Yadav","Junio C Hamano","Aaron Schrab"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"386371","messageId":"20191116161856.28883-1-me@yadavpratyush.com","threadId":"52277","inReplyTo":null,"subject":"[RFC PATCH] wt-status: show amended content when verbose","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-11-16T16:18:56Z","receivedAt":"2019-11-16T16:19:05Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"Hi,\n\nI am working on a simple little feature which shows the \"amended\ncontent\" when running 'git-commit -v'. Currently, only the changes in\nthe _entire_ commit are shown. In a large commit, it is difficult to\nspot a line or two that were amended. So, show just the amended content\nin a different section.\n\nI'm having trouble working with the internal diff API. 'rev' in the\nfunction here is used to diff against HEAD^1. I want to do the exact\nsame thing, but against HEAD instead.\n\nThe diff below works, but it is obviously an ugly hack that just resets\n'rev' and duplicates all the initialization code. I added it here as a\n\"proof of concept\". What would be the cleaner way to do it?\n\nI tried a bunch of things, but they either end up in me hitting\n\n BUG(\"run_diff_index must be passed exactly one tree\");\n\nin 'run_diff_index', or just doing something completely\nunexpected/useless.\n\nSome help/pointers would be appreciated. Thanks.\n\nRegards,\nPratyush Yadav\n\n-- 8< --\n\nSigned-off-by: Pratyush Yadav \n---\n wt-status.c | 21 +++++++++++++++++++++\n 1 file changed, 21 insertions(+)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex cc6f94504d..efa01c7ed6 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1086,6 +1086,27 @@ static void wt_longstatus_print_verbose(struct wt_status *s)\n \t\trev.diffopt.b_prefix = \"w/\";\n \t\trun_diff_files(&rev, 0);\n \t}\n+\n+\tif (s->amend) {\n+\t\trepo_init_revisions(s->repo, &rev, NULL);\n+\t\trev.diffopt.flags.allow_textconv = 1;\n+\t\trev.diffopt.ita_invisible_in_index = 1;\n+\n+\t\tmemset(&opt, 0, sizeof(opt));\n+\t\topt.def = \"HEAD\";\n+\t\tsetup_revisions(0, NULL, &rev, &opt);\n+\n+\t\trev.diffopt.output_format |= DIFF_FORMAT_PATCH;\n+\t\trev.diffopt.detect_rename = s->detect_rename >= 0 ? s->detect_rename : rev.diffopt.detect_rename;\n+\t\trev.diffopt.rename_limit = s->rename_limit >= 0 ? s->rename_limit : rev.diffopt.rename_limit;\n+\t\trev.diffopt.rename_score = s->rename_score >= 0 ? s->rename_score : rev.diffopt.rename_score;\n+\t\trev.diffopt.file = s->fp;\n+\t\trev.diffopt.close_file = 0;\n+\t\trev.diffopt.use_color = 0;\n+\t\tstatus_printf_ln(s, c, \"Changes to amend:\\n\");\n+\n+\t\trun_diff_index(&rev, 1);\n+\t}\n }\n\n static void wt_longstatus_print_tracking(struct wt_status *s)\n--\n2.24.0\n\n"},{"id":"386433","messageId":"xmqqd0dp3lfv.fsf@gitster-ct.c.googlers.com","threadId":"52277","inReplyTo":"20191116161856.28883-1-me@yadavpratyush.com","subject":"Re: [RFC PATCH] wt-status: show amended content when verbose","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-18T03:36:20Z","receivedAt":"2019-11-18T03:36:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pratyush Yadav writes:\n\n> I am working on a simple little feature which shows the \"amended\n> content\" when running 'git-commit -v'. Currently, only the changes in\n> the _entire_ commit are shown. In a large commit, it is difficult to\n> spot a line or two that were amended. So, show just the amended content\n> in a different section.\n\n[jc: even though the diff generation is done before the final commit\nis made, let me refer to the commits with refs _after_ the amend is\ndone].\n\nYou want to show changes between HEAD@{1}..HEAD (which is what the\n\"amend\" did) in addition to changes between HEAD^..HEAD (which is\nwhat the \"amended commit\" does) separately.\n\nThe reason why \"git commit -v\" lets you see the diff since HEAD^ is\nto help you write the commit log message. So it is wrong to show\nonly \"what the amend did\", as the message you would be writing while\namending is to explain the entire \"why the amended commit does what\nit does\" and by definition the log message for \"amend\" should not\ntalk about \"why the amend did what it did\"---the readers would not\neven have access to the older version before the amend.\n\nIt too makes quite a lot of sense to allow readers to see what the\n'amend' did, but that is not something that would help write the log\nmessage. And that is why \"git commit -v --amend\" does not show it.\nIt should be inspected even _before_ the user contemplates to run\n\"git commit --amend\" (e.g. \"git diff HEAD\" before starting to amend).\n\nSo, I am not enthused with this change---it sends a wrong message\n(i.e. what the diff in the editor \"commit -v\" gives the user for).\n\n\n\n\n\n\n\n"},{"id":"386435","messageId":"xmqq4kz13k8p.fsf@gitster-ct.c.googlers.com","threadId":"52277","inReplyTo":"xmqqd0dp3lfv.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] wt-status: show amended content when verbose","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-18T04:02:14Z","receivedAt":"2019-11-18T04:09:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Pratyush Yadav writes:\n>\n>> I am working on a simple little feature which shows the \"amended\n>> content\" when running 'git-commit -v'. Currently, only the changes in\n>> the _entire_ commit are shown. In a large commit, it is difficult to\n>> spot a line or two that were amended. So, show just the amended content\n>> in a different section.\n>\n> [jc: even though the diff generation is done before the final commit\n> is made, let me refer to the commits with refs _after_ the amend is\n> done].\n>\n> You want to show changes between HEAD@{1}..HEAD (which is what the\n> \"amend\" did) in addition to changes between HEAD^..HEAD (which is\n> what the \"amended commit\" does) separately.\n>\n> The reason why \"git commit -v\" lets you see the diff since HEAD^ is\n> to help you write the commit log message. So it is wrong to show\n> only \"what the amend did\", as the message you would be writing while\n> amending is to explain the entire \"why the amended commit does what\n> it does\" and by definition the log message for \"amend\" should not\n> talk about \"why the amend did what it did\"---the readers would not\n> even have access to the older version before the amend.\n>\n> It too makes quite a lot of sense to allow readers to see what the\n> 'amend' did, but that is not something that would help write the log\n> message. And that is why \"git commit -v --amend\" does not show it.\n> It should be inspected even _before_ the user contemplates to run\n> \"git commit --amend\" (e.g. \"git diff HEAD\" before starting to amend).\n>\n> So, I am not enthused with this change---it sends a wrong message\n> (i.e. what the diff in the editor \"commit -v\" gives the user for).\n\nHaving said that, I also wonder two things. Assuming that it may be\na good idea to show \"what the amend does\" in addition to \"what the\namended commit does\",\n\n 1. would it make sense to show a combined diff to show the\n differences among the state being recorded in the amended commit\n as if it were a merge between the state in the original commit\n and the state in the parent commit?\n\n 2. would it make sense to show the differences between\n HEAD^..HEAD@{1} and between HEAD^..HEAD using the range-diff\n machinery.\n\nI think #1 may turn out to be more useful (I haven't tried it,\nthough) because we already show a moral equivalent elsewhere, namely\nin \"git stash show\".\n\nConceptually, it would be similar to showing a stash entry that\nrecords the state where some changes have been already added to the\nindex and some other changes are still in the working tree---the\nbase commit of such a stash entry corresponds to the parent commit\nof the commit being amended, the contents from the index of such a\nstash entry corresponds to the commit being amended, and the\ncontents from the working tree of such a stash entry corresponds to\nthe final contents you are trying to record as an amended commit.\n\n"},{"id":"386518","messageId":"20191119145632.xi6zebglzu4lbgcq@yadavpratyush.com","threadId":"52277","inReplyTo":"xmqq4kz13k8p.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] wt-status: show amended content when verbose","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2019-11-19T14:56:32Z","receivedAt":"2019-11-19T14:56:41Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On 18/11/19 01:02PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n> > Pratyush Yadav writes:\n> >\n> >> I am working on a simple little feature which shows the \"amended\n> >> content\" when running 'git-commit -v'. Currently, only the changes in\n> >> the _entire_ commit are shown. In a large commit, it is difficult to\n> >> spot a line or two that were amended. So, show just the amended content\n> >> in a different section.\n> >\n> > [jc: even though the diff generation is done before the final commit\n> > is made, let me refer to the commits with refs _after_ the amend is\n> > done].\n> >\n> > You want to show changes between HEAD@{1}..HEAD (which is what the\n> > \"amend\" did) in addition to changes between HEAD^..HEAD (which is\n> > what the \"amended commit\" does) separately.\n\nYes, that is what the patch does. It shows _both_ the entire diff and \nthe \"amended diff\".\n\n> > The reason why \"git commit -v\" lets you see the diff since HEAD^ is\n> > to help you write the commit log message. So it is wrong to show\n> > only \"what the amend did\", as the message you would be writing while\n> > amending is to explain the entire \"why the amended commit does what\n> > it does\" and by definition the log message for \"amend\" should not\n> > talk about \"why the amend did what it did\"---the readers would not\n> > even have access to the older version before the amend.\n> >\n> > It too makes quite a lot of sense to allow readers to see what the\n> > 'amend' did, but that is not something that would help write the log\n> > message.\n\nIt would help _amend_ the log message though. This is the use-case which \nmotivated me to write this patch. When I make some changes to a commit \n(like when re-rolling), I often want to update the commit message too. \nIf the commit content is a lot, then it becomes difficult to easily see \nwhat exactly I changed, and in turn makes it difficult to quickly spot \nwhat parts of the log message need updating.\n\n> > And that is why \"git commit -v --amend\" does not show it.\n> > It should be inspected even _before_ the user contemplates to run\n> > \"git commit --amend\" (e.g. \"git diff HEAD\" before starting to amend).\n> >\n> > So, I am not enthused with this change---it sends a wrong message\n> > (i.e. what the diff in the editor \"commit -v\" gives the user for).\n> \n> Having said that, I also wonder two things. Assuming that it may be\n> a good idea to show \"what the amend does\" in addition to \"what the\n> amended commit does\",\n> 1. would it make sense to show a combined diff to show the\n> differences among the state being recorded in the amended commit\n> as if it were a merge between the state in the original commit\n> and the state in the parent commit?\n\nI'm afraid I don't follow what exactly this would do, and how it would \nhelp differentiate between the \"what the amend does\" and \"what the \namended commit does\". Wouldn't the diff of a merge between the original \ncommit and the parent be exactly the diff (iow, the output of 'git \nshow') of the original commit, since the merge is a fast-forward?\n \n> 2. would it make sense to show the differences between\n> HEAD^..HEAD@{1} and between HEAD^..HEAD using the range-diff\n> machinery.\n\nI considered using range-diff, but didn't go with it because of my \npersonal dislike for range-diff. But if you strongly think that \nrange-diff is a better idea, then I can do that too.\n \n> I think #1 may turn out to be more useful (I haven't tried it,\n> though) because we already show a moral equivalent elsewhere, namely\n> in \"git stash show\".\n> \n> Conceptually, it would be similar to showing a stash entry that\n> records the state where some changes have been already added to the\n> index and some other changes are still in the working tree---the\n> base commit of such a stash entry corresponds to the parent commit\n> of the commit being amended, the contents from the index of such a\n> stash entry corresponds to the commit being amended, and the\n> contents from the working tree of such a stash entry corresponds to\n> the final contents you are trying to record as an amended commit.\n> \n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"386580","messageId":"20191120002729.GG4444@pug.qqx.org","threadId":"52277","inReplyTo":"20191119145632.xi6zebglzu4lbgcq@yadavpratyush.com","subject":"Re: [RFC PATCH] wt-status: show amended content when verbose","fromName":"Aaron Schrab","fromEmail":"aaron@schrab.com","sentAt":"2019-11-20T00:27:29Z","receivedAt":"2019-11-20T00:33:20Z","isPatch":true,"sender":{"key":"aaron@schrab.com","avatar":"https://avatars.githubusercontent.com/u/39620?v=4"},"body":"At 20:26 +0530 19 Nov 2019, Pratyush Yadav wrote:\n>> Junio C Hamano writes:\n\n>> > It too makes quite a lot of sense to allow readers to see what the\n>> > 'amend' did, but that is not something that would help write the log\n>> > message.\n>\n>It would help _amend_ the log message though.\n\nIndeed. Another possible use is for sanity checking the amendment. I'll \noften look over the diff in my editor as a final check of what I'm about \nto commit, and when amending a commit I think I would find it helpful to \nbe able to review the changes being amended separately from the full set \nof changes.\n"},{"id":"386595","messageId":"xmqqv9rfe4th.fsf@gitster-ct.c.googlers.com","threadId":"52277","inReplyTo":"20191119145632.xi6zebglzu4lbgcq@yadavpratyush.com","subject":"Re: [RFC PATCH] wt-status: show amended content when verbose","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-20T01:04:26Z","receivedAt":"2019-11-20T01:04:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pratyush Yadav writes:\n\n> I'm afraid I don't follow what exactly this would do, and how it would \n> help differentiate between the \"what the amend does\" and \"what the \n> amended commit does\".\n\nThe resulting history would be\n\n\tO---A\n \\\n B\n\n\nwhere\n\n\tO = HEAD^ = HEAD@{1}^\n\tA = HEAD@{1}\t\t- HEAD before the amend\n\tB = HEAD\t\t- result of the amend\n\nI wonder if\n\n git diff -c B O A\n\n(with possibly different permutations of three revisions) is a\nreasonable way to show what the final state is and where it differs\nfrom the previous one (i.e. HEAD@{1}) and the original one\n(i.e. HEAD^) in the combined diff format.\n\n>> 2. would it make sense to show the differences between\n>> HEAD^..HEAD@{1} and between HEAD^..HEAD using the range-diff\n>> machinery.\n>\n> I considered using range-diff, but didn't go with it because of my \n> personal dislike for range-diff.\n\nFor a single-commit amend, the normal diff between HEAD@{1} and HEAD\nwould be far easier to read than such a range-diff, I would think.\n"}]}