{"thread":{"id":"45824","subject":"[Suggestion] Alternative way for displaying help menu of patch flow","startedAt":"2017-04-28T05:14:00Z","lastAt":"2017-04-28T07:21:23Z","messageCount":3,"participants":["Kaartic Sivaraam","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"318117","messageId":"b55bd985-be6a-7fd6-1744-7ffb9d8b4ce8@gmail.com","threadId":"45824","inReplyTo":null,"subject":"[Suggestion] Alternative way for displaying help menu of patch flow","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-04-28T05:14:22Z","receivedAt":"2017-04-28T05:14:00Z","isPatch":false,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"Hello community,\n\nI would like to state about a small issue I face when I use the `git add \n-i` command. I guess it's expressed easily with an example.\n\n**** Start ****\n\nImagine a situation in which you accidentally made a reasonable amount \nof change to your code from your last commit. You would like to put them \nin separate commits and hence you go for the `git add -i` command and \nchoose the `patch` option. You choose the file which you want to update \nas patches.\n\nUnfortunately a hunk (of reasonable size) in the file seems to be having \nchanges that you would like to be in separate commits. So, you would \nlike to split the hunk into smaller parts but you forgot the correct \noption for it and you use `?` option. In most cases you wouldn't see the \nhelp menu completely. Moreover, if the hunk's size is reasonable enough \nto take up the whole screen of the terminal you would be seeing the same \nscreen as before as the help menu is hidden above the hunk and you need \nto scroll to see it (only if you knew it's up there in the first place!)\n\n**** End ****\n\nI guess it would be better to display the help menu in a separate flow, \nlike when the user click the `?` option in the `patch` flow then the \nwhole screen is cleared and the help menu is displayed in some \nappropriate place and after the user has viewed the help menu he could \nquit from the help screen (probably by using a key like `q`) to continue \nwhere he left off. I think it would be better because, in general, a \nuser would want to see the help menu when he uses the `?` option in the \npatch flow, not the hunk that gets displayed after it.\n\nIs it a good suggestion or is there anything I missed?\n\nQuote : \"You don’t change the world by doing what you’re told.” — Joi Ito\n\nRegards,\nKaartic\n\n"},{"id":"318118","messageId":"20170428063723.2ni2v6akosm4ldvy@sigill.intra.peff.net","threadId":"45824","inReplyTo":"b55bd985-be6a-7fd6-1744-7ffb9d8b4ce8@gmail.com","subject":"Re: [Suggestion] Alternative way for displaying help menu of patch flow","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-28T06:37:23Z","receivedAt":"2017-04-28T06:37:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Apr 28, 2017 at 10:44:22AM +0530, Kaartic Sivaraam wrote:\n\n> I guess it would be better to display the help menu in a separate flow, like\n> when the user click the `?` option in the `patch` flow then the whole screen\n> is cleared and the help menu is displayed in some appropriate place and\n> after the user has viewed the help menu he could quit from the help screen\n> (probably by using a key like `q`) to continue where he left off. I think it\n> would be better because, in general, a user would want to see the help menu\n> when he uses the `?` option in the patch flow, not the hunk that gets\n> displayed after it.\n\nI think it would be nicer to avoid modality in the interface. I.e.,\nrather than having \"?\" switch you into \"help mode\", stay at the same\nhunk prompt but just don't show the hunk again automatically. In most\ncases it will still be visible on the screen anyway, and there probably\nought to be a \"r\"edisplay command if the user wants to see it from\nscratch.\n\nSomething like the patch at the end of this message.\n\nI think this problem does extend to other output we show in the patch\nloop. For instance, if you hit 'j' and there is no, you get:\n\n  $ git add -p\n  diff --git a/foo b/foo\n  index 3367afd..3e75765 100644\n  --- a/foo\n  +++ b/foo\n  @@ -1 +1 @@\n  -old\n  +new\n  Stage this hunk [y,n,q,a,d,/,e,?]? j\n  No next hunk\n  @@ -1 +1 @@\n  -old\n  +new\n  Stage this hunk [y,n,q,a,d,/,e,?]? \n\nIf the hunk is big, that message scrolls off the screen. We could\nprobably put it after the hunk, but I'd worry that the ordering would be\nconfusing (because it comes between the hunk and the \"this hunk\" prompt.\n\nOr it could just not re-display the hunk all (like what my patch does\nfor help), and you'd get:\n\n  @@ -1 +1 @@\n  -old\n  +new\n  Stage this hunk [y,n,q,a,d,/,e,?]? j\n  No next hunk\n  Stage this hunk [y,n,q,a,d,/,e,?]? \n\nat which point you could \"r\" to redisplay it if you wanted to.\n\nI also suspect that other menus in add--interactive have the same\nissue. E.g., if you have a large number of files what does the\nfile-selection menu \"git add -i\" look like? Perhaps the issue is less\ncommon there, though.\n\n---\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 709a5f6ce..81f62331b 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1237,6 +1237,7 @@ k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n s - split the current hunk into smaller hunks\n e - manually edit the current hunk\n+r - redisplay the current hunk\n ? - print help\n EOF\n }\n@@ -1385,6 +1386,7 @@ my %patch_update_prompt_modes = (\n sub patch_update_file {\n \tmy $quit = 0;\n \tmy ($ix, $num);\n+\tmy $skip_hunk_display;\n \tmy $path = shift;\n \tmy ($head, @hunk) = parse_diff($path);\n \t($head, my $mode, my $deletion) = parse_diff_header($head);\n@@ -1451,8 +1453,13 @@ sub patch_update_file {\n \t\tif ($hunk[$ix]{TYPE} eq 'hunk') {\n \t\t\t$other .= ',e';\n \t\t}\n-\t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n-\t\t\tprint;\n+\n+\t\tif ($skip_hunk_display) {\n+\t\t\t$skip_hunk_display = 0;\n+\t\t} else {\n+\t\t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n+\t\t\t\tprint;\n+\t\t\t}\n \t\t}\n \t\tprint colored $prompt_color,\n \t\t\tsprintf(__($patch_update_prompt_modes{$patch_mode}{$hunk[$ix]{TYPE}}), $other);\n@@ -1608,8 +1615,13 @@ sub patch_update_file {\n \t\t\t\t\tsplice @hunk, $ix, 1, $newhunk;\n \t\t\t\t}\n \t\t\t}\n+\t\t\telsif ($line =~ /^r/) {\n+\t\t\t\t# do nothing; we'll show the hunk when we loop\n+\t\t\t\tnext;\n+\t\t\t}\n \t\t\telse {\n \t\t\t\thelp_patch_cmd($other);\n+\t\t\t\t$skip_hunk_display = 1;\n \t\t\t\tnext;\n \t\t\t}\n \t\t\t# soft increment\n"},{"id":"318119","messageId":"4d6eb11e-f102-7695-7f4a-562407ea13c0@gmail.com","threadId":"45824","inReplyTo":"20170428063723.2ni2v6akosm4ldvy@sigill.intra.peff.net","subject":"Re: [Suggestion] Alternative way for displaying help menu of patch flow","fromName":"Kaartic Sivaraam","fromEmail":"kaarticsivaraam91196@gmail.com","sentAt":"2017-04-28T07:21:14Z","receivedAt":"2017-04-28T07:21:23Z","isPatch":false,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Friday 28 April 2017 12:07 PM, Jeff King wrote:\n>\n> If the hunk is big, that message scrolls off the screen. We could\n> probably put it after the hunk, but I'd worry that the ordering would be\n> confusing (because it comes between the hunk and the \"this hunk\" prompt.\nProbably it could be better to clear the console after each user input, \nbefore\nthe result of that input is displayed. That could probably remove the \nclutter\nand could allow the user to easily differentiate from the current output \nfrom\nprevious output.\n> Or it could just not re-display the hunk all (like what my patch does\n> for help), and you'd get:\n>\n>    @@ -1 +1 @@\n>    -old\n>    +new\n>    Stage this hunk [y,n,q,a,d,/,e,?]? j\n>    No next hunk\n>    Stage this hunk [y,n,q,a,d,/,e,?]?\n>\n> at which point you could \"r\" to redisplay it if you wanted to.\nThis seems good too. But, I guess, the help menu would probably hide the\nhunk and could make that prompt a little absurd.\n> I also suspect that other menus in add--interactive have the same\n> issue. E.g., if you have a large number of files what does the\n> file-selection menu \"git add -i\" look like? Perhaps the issue is less\n> common there, though.\n>\n> ---\n> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n> index 709a5f6ce..81f62331b 100755\n> --- a/git-add--interactive.perl\n> +++ b/git-add--interactive.perl\n> @@ -1237,6 +1237,7 @@ k - leave this hunk undecided, see previous undecided hunk\n>   K - leave this hunk undecided, see previous hunk\n>   s - split the current hunk into smaller hunks\n>   e - manually edit the current hunk\n> +r - redisplay the current hunk\n>   ? - print help\n>   EOF\n>   }\n> @@ -1385,6 +1386,7 @@ my %patch_update_prompt_modes = (\n>   sub patch_update_file {\n>   \tmy $quit = 0;\n>   \tmy ($ix, $num);\n> +\tmy $skip_hunk_display;\n>   \tmy $path = shift;\n>   \tmy ($head, @hunk) = parse_diff($path);\n>   \t($head, my $mode, my $deletion) = parse_diff_header($head);\n> @@ -1451,8 +1453,13 @@ sub patch_update_file {\n>   \t\tif ($hunk[$ix]{TYPE} eq 'hunk') {\n>   \t\t\t$other .= ',e';\n>   \t\t}\n> -\t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n> -\t\t\tprint;\n> +\n> +\t\tif ($skip_hunk_display) {\n> +\t\t\t$skip_hunk_display = 0;\n> +\t\t} else {\n> +\t\t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n> +\t\t\t\tprint;\n> +\t\t\t}\n>   \t\t}\n>   \t\tprint colored $prompt_color,\n>   \t\t\tsprintf(__($patch_update_prompt_modes{$patch_mode}{$hunk[$ix]{TYPE}}), $other);\n> @@ -1608,8 +1615,13 @@ sub patch_update_file {\n>   \t\t\t\t\tsplice @hunk, $ix, 1, $newhunk;\n>   \t\t\t\t}\n>   \t\t\t}\n> +\t\t\telsif ($line =~ /^r/) {\n> +\t\t\t\t# do nothing; we'll show the hunk when we loop\n> +\t\t\t\tnext;\n> +\t\t\t}\n>   \t\t\telse {\n>   \t\t\t\thelp_patch_cmd($other);\n> +\t\t\t\t$skip_hunk_display = 1;\n>   \t\t\t\tnext;\n>   \t\t\t}\n>   \t\t\t# soft increment\nRegards,\nKaartic\n"}]}