{"thread":{"id":"61951","subject":"[PATCH 1/1] git jump: support show","startedAt":"2024-08-14T20:07:21Z","lastAt":"2024-08-15T00:42:56Z","messageCount":3,"participants":["Geoffrey Thomas","Eric Sunshine","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"500938","messageId":"20240814200709.53450-1-geofft@ldpreload.com","threadId":"61951","inReplyTo":null,"subject":"[PATCH 1/1] git jump: support show","fromName":"Geoffrey Thomas","fromEmail":"geofft@ldpreload.com","sentAt":"2024-08-14T20:07:09Z","receivedAt":"2024-08-14T20:07:21Z","isPatch":true,"sender":{"key":"geofft@ldpreload.com","avatar":null},"body":"This makes it easy to go to the changes in the latest commit, or a\nprevious named commit, to fix a bug and commit a fixup, to respond to\ncode review feedback, etc.\n\nSigned-off-by: Geoffrey Thomas <geofft@ldpreload.com>\n---\n contrib/git-jump/git-jump | 14 ++++++++++++--\n 1 file changed, 12 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\nindex 47e0c557e6..6cf16e0f32 100755\n--- a/contrib/git-jump/git-jump\n+++ b/contrib/git-jump/git-jump\n@@ -41,8 +41,10 @@ open_editor() {\n \tesac\n }\n \n-mode_diff() {\n-\tgit diff --no-prefix --relative \"$@\" |\n+do_diff() {\n+\tcmd=$1\n+\tshift\n+\tgit \"$cmd\" --no-prefix --relative \"$@\" |\n \tperl -ne '\n \tif (m{^\\+\\+\\+ (.*)}) { $file = $1; next }\n \tdefined($file) or next;\n@@ -56,6 +58,14 @@ mode_diff() {\n \t'\n }\n \n+mode_diff() {\n+\tdo_diff diff \"$@\"\n+}\n+\n+mode_show() {\n+\tdo_diff show \"$@\"\n+}\n+\n mode_merge() {\n \tgit ls-files -u \"$@\" |\n \tperl -pe 's/^.*?\\t//' |\n\nbase-commit: 25673b1c476756ec0587fb0596ab3c22b96dc52a\n-- \n2.39.3 (Apple Git-145)\n\n"},{"id":"500939","messageId":"CAPig+cRTbVPaF2C=u1+mVqYwK5UiPQ7jXB6sw_i4_H6qgYibLA@mail.gmail.com","threadId":"61951","inReplyTo":"20240814200709.53450-1-geofft@ldpreload.com","subject":"Re: [PATCH 1/1] git jump: support show","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2024-08-14T21:10:34Z","receivedAt":"2024-08-14T21:10:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Aug 14, 2024 at 4:07 PM Geoffrey Thomas <geofft@ldpreload.com> wrote:\n> This makes it easy to go to the changes in the latest commit, or a\n> previous named commit, to fix a bug and commit a fixup, to respond to\n> code review feedback, etc.\n>\n> Signed-off-by: Geoffrey Thomas <geofft@ldpreload.com>\n> ---\n> diff --git a/contrib/git-jump/git-jump b/contrib/git-jump/git-jump\n> @@ -41,8 +41,10 @@ open_editor() {\n> -mode_diff() {\n> -       git diff --no-prefix --relative \"$@\" |\n> +do_diff() {\n> +       cmd=$1\n> +       shift\n> +       git \"$cmd\" --no-prefix --relative \"$@\" |\n>         perl -ne '\n>         if (m{^\\+\\+\\+ (.*)}) { $file = $1; next }\n>         defined($file) or next;\n> @@ -56,6 +58,14 @@ mode_diff() {\n> +mode_diff() {\n> +       do_diff diff \"$@\"\n> +}\n> +\n> +mode_show() {\n> +       do_diff show \"$@\"\n> +}\n> +\n\nI'm not a git-jump user (indeed, I've never even looked at it), but\nshould this change be accompanied by corresponding updates to the\ndocumentation in the README and usage statement emitted by the usage()\nfunction in git-jump script?\n"},{"id":"500944","messageId":"20240815004248.GA2629889@coredump.intra.peff.net","threadId":"61951","inReplyTo":"20240814200709.53450-1-geofft@ldpreload.com","subject":"Re: [PATCH 1/1] git jump: support show","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-08-15T00:42:48Z","receivedAt":"2024-08-15T00:42:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 14, 2024 at 04:07:09PM -0400, Geoffrey Thomas wrote:\n\n> This makes it easy to go to the changes in the latest commit, or a\n> previous named commit, to fix a bug and commit a fixup, to respond to\n> code review feedback, etc.\n\nOne trouble with this approach is that you're analyzing a diff whose\nendpoint is something other than the current working tree. So the line\nnumbers in the diff do not necessarily correspond to what you're going\nto open in the editor.\n\nFor something like the changes in the latest commit, I'd usually do \"git\njump diff HEAD^\", which I think is strictly better than a \"show\" on the\nlatest commit.\n\nFor looking at older commits that doesn't work, though. And if they're\nnot _too_ old, then you've got a reasonable chance of ending up\nsomewhere useful. So I'm not opposed to this patch. As Eric mentioned,\nwe'd probably want an update to the README. And I think it should\nmention the caveat that the post-image of the diff you're viewing won't\nnecessarily match the working tree.\n\n-Peff\n"}]}