{"thread":{"id":"40558","subject":"Alternative to manual editing with git add --patch","startedAt":"2015-10-14T15:07:39Z","lastAt":"2015-10-15T15:22:55Z","messageCount":9,"participants":["Sven Helmberger","Matthieu Moy","Johannes Schindelin","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"271648","messageId":"561E6FBB.7060302@gmx.de","threadId":"40558","inReplyTo":null,"subject":"Alternative to manual editing with git add --patch","fromName":"Sven Helmberger","fromEmail":"sven.helmberger@gmx.de","sentAt":"2015-10-14T15:07:39Z","receivedAt":"2015-10-14T15:07:39Z","isPatch":false,"sender":{"key":"sven.helmberger@gmx.de","avatar":"https://gravatar.com/avatar/6acfa8b9062e6566b78555a6397ea6f37707a2a5ce729c36de672bce5aee2c11?d=mp&s=160"},"body":"Hello,\n\nI hope this hasn't been discussed before.\n\nI'm a big fan of cleanliness in commits and therefore often use git add\n--patch to sort code changes I made into the right commits etc.\n\nWhat I then often encountered was the situation where I happened to have\ninserted consecutive lines of code that conceptually belong to different\ncommits. Normally I can nicely split patches, but not in this case,\nmaking manually editing the patch the only alternative.\n\nShouldn't there be at least a way to quickly say line-by-line if you\nwant to have it added or not?\n\nPersonally, I find manually editing just annoying, it seems overly\narcane, but it also prevents me from really recommending \"add --patch\"\nas best practice. I think it's a really good idea for many reasons to do\nso, but I can't really tell people already struggling with using git\nthat I expect them to edit patches manually.\n\nRegards,\nSven Helmberger\n"},{"id":"271649","messageId":"vpqeggx9zul.fsf@grenoble-inp.fr","threadId":"40558","inReplyTo":"561E6FBB.7060302@gmx.de","subject":"Re: Alternative to manual editing with git add --patch","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-14T16:30:58Z","receivedAt":"2015-10-14T16:30:58Z","isPatch":false,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Sven Helmberger writes:\n\n> Hello,\n>\n> I hope this hasn't been discussed before.\n>\n> I'm a big fan of cleanliness in commits and therefore often use git add\n> --patch to sort code changes I made into the right commits etc.\n>\n> What I then often encountered was the situation where I happened to have\n> inserted consecutive lines of code that conceptually belong to different\n> commits. Normally I can nicely split patches, but not in this case,\n> making manually editing the patch the only alternative.\n>\n> Shouldn't there be at least a way to quickly say line-by-line if you\n> want to have it added or not?\n\nMany GUI or text-editor plugins for Git allow you to just select lines\nand stage the selection. Even though I love \"git add -p\", I find magit\n(Git integration for Emacs) more convenient when it comes to staging\nindividual lines.\n\nThat said, a \"split hunk line by line\" option for \"git add -p\" could be\nnice.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271650","messageId":"1326b047519c9398a0a06e27a26295ca@dscho.org","threadId":"40558","inReplyTo":"561E6FBB.7060302@gmx.de","subject":"Re: Alternative to manual editing with git add --patch","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-14T16:52:07Z","receivedAt":"2015-10-14T16:52:07Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Sven,\n\nOn 2015-10-14 17:07, Sven Helmberger wrote:\n\n> What I then often encountered was the situation where I happened to have\n> inserted consecutive lines of code that conceptually belong to different\n> commits. Normally I can nicely split patches, but not in this case,\n> making manually editing the patch the only alternative.\n\nHow about implementing it and then discussing the implementation? That would have two advantages:\n\n1) there would be something tangential to discuss, and\n2) even if Junio rejects it, you can carry the patch in your private fork and use it forever.\n\nCiao,\nJohannes\n"},{"id":"271655","messageId":"xmqqk2qp8hlj.fsf@gitster.mtv.corp.google.com","threadId":"40558","inReplyTo":"561E6FBB.7060302@gmx.de","subject":"Re: Alternative to manual editing with git add --patch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-14T17:50:32Z","receivedAt":"2015-10-14T17:50:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sven Helmberger writes:\n\n> I hope this hasn't been discussed before.\n>\n> I'm a big fan of cleanliness in commits and therefore often use git add\n> --patch to sort code changes I made into the right commits etc.\n>\n> What I then often encountered was the situation where I happened to have\n> inserted consecutive lines of code that conceptually belong to different\n> commits. Normally I can nicely split patches, but not in this case,\n> making manually editing the patch the only alternative.\n>\n> Shouldn't there be at least a way to quickly say line-by-line if you\n> want to have it added or not?\n\nI think this has been discussed a few times (and you should actually\nhope that is the case---it shows that something that allows you to\nsplit a hunk that consists of consecutive lines is not an obscure\nand useless feature wish). But I do not think we saw anybody came\nup with a convincingly usable design (not the code design but how\nthe user interaction specifies where the hunk is cut in the first\nplace), let alone a prototype for it, so that we can discuss\nfurther. At least not yet.\n\nAs a quick-and-dirty change, you could invent a new variant of\n's'plit that breaks a N-line hunk into N hunks with 1-line each, but\nobviously that would not be a pleasant-enough UI to be called usable\nwhen you have a hunk that adds 100 lines. Perhaps \"Split this hunk\ninto two by ending the earlier one immediately before the line that\nhas this substring\" or something might be an idea?\n"},{"id":"271709","messageId":"561EE700.3020002@gmx.de","threadId":"40558","inReplyTo":"xmqqk2qp8hlj.fsf@gitster.mtv.corp.google.com","subject":"Re: Alternative to manual editing with git add --patch","fromName":"Sven Helmberger","fromEmail":"sven.helmberger@gmx.de","sentAt":"2015-10-14T23:36:32Z","receivedAt":"2015-10-14T23:36:32Z","isPatch":false,"sender":{"key":"sven.helmberger@gmx.de","avatar":"https://gravatar.com/avatar/6acfa8b9062e6566b78555a6397ea6f37707a2a5ce729c36de672bce5aee2c11?d=mp&s=160"},"body":"Am 14.10.2015 um 19:50 schrieb Junio C Hamano:\n> Sven Helmberger writes:\n> \n> As a quick-and-dirty change, you could invent a new variant of\n> 's'plit that breaks a N-line hunk into N hunks with 1-line each, but\n> obviously that would not be a pleasant-enough UI to be called usable\n> when you have a hunk that adds 100 lines. Perhaps \"Split this hunk\n> into two by ending the earlier one immediately before the line that\n> has this substring\" or something might be an idea?\n> \n\nIf we go by the style of interaction in git add --patch and git add\n--interactive, I think the most canonical solution would be to implement\nit like this.\n\nIf we know when we can't split the current patch any further ( the point\nat which selecting s changes nothing anymore), why shouldn't add --patch\nnot work similar to add --interactive in that it prints the lines of the\ndiff prefixed with numbers and the user can define a numerical range to\n\"split off\". Then they decide whether to add those lines or not and\nreturn to the line-numbers until they're trough with the patch.\n\nRegards,\nSven Helmberger\n"},{"id":"271724","messageId":"alpine.DEB.1.00.1510151152470.31610@s15462909.onlinehome-server.info","threadId":"40558","inReplyTo":"561EE700.3020002@gmx.de","subject":"Re: Alternative to manual editing with git add --patch","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-15T10:11:23Z","receivedAt":"2015-10-15T10:11:23Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Sven & Junio,\n\nOn Thu, 15 Oct 2015, Sven Helmberger wrote:\n\n> Am 14.10.2015 um 19:50 schrieb Junio C Hamano:\n> > Sven Helmberger writes:\n> > \n> > As a quick-and-dirty change, you could invent a new variant of\n> > 's'plit that breaks a N-line hunk into N hunks with 1-line each, but\n> > obviously that would not be a pleasant-enough UI to be called usable\n> > when you have a hunk that adds 100 lines. Perhaps \"Split this hunk\n> > into two by ending the earlier one immediately before the line that\n> > has this substring\" or something might be an idea?\n> > \n> \n> If we go by the style of interaction in git add --patch and git add\n> --interactive, I think the most canonical solution would be to implement\n> it like this.\n> \n> If we know when we can't split the current patch any further ( the point\n> at which selecting s changes nothing anymore), why shouldn't add --patch\n> not work similar to add --interactive in that it prints the lines of the\n> diff prefixed with numbers and the user can define a numerical range to\n> \"split off\". Then they decide whether to add those lines or not and\n> return to the line-numbers until they're trough with the patch.\n\nThis is all technically sound. From a usability perspective I would wish\nmore for a way to exclude or filter the lines by a pattern. Take for\nexample a diff like this (which is *quite* normal in my workflow):\n\n-- snip --\ndiff --git a/80a8c9a..001a983 b/001a983\nindex 80a8c9a..001a983 100644\n--- a/80a8c9a..001a983\n+++ b/001a983\n@@ -23,11 +23,15 @@ void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t of\n\n hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL,\n PAGE_WRITECOPY, 0, 0, NULL);\n+error(\"hmap: %p\", hmap);\n\n- if (!hmap)\n+ if (!hmap) {\n+ errno = EINVAL;\n return MAP_FAILED;\n+ }\n\n temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start);\n+error(\"temp: %p 0x%x\", temp, (unsigned int)GetLastError());\n\n if (!CloseHandle(hmap))\n warning(\"unable to close file mapping handle\");\n-- snap --\n\n(Yes, I am lazy and reuse the `error()` function for debug log messages.)\n\nIt is quite obvious what I would like to do in this case: keep the change\nthat sets the errno.\n\nI would be really thankful if I had a shortcut in `git add -p` that let me\nspecify, say, \"Xerror\" to eXclude any + line (and replace the '-' by a ' '\nin every - line) that contains the substring 'error'.\n\nThe way I envisage this command to work would be to present the filtered\ndiff in the next step:\n\n-- snip --\n@@ -23,11 +23,13 @@ void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t of\n\n hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL,\n PAGE_WRITECOPY, 0, 0, NULL);\n\n- if (!hmap)\n+ if (!hmap) {\n+ errno = EINVAL;\n return MAP_FAILED;\n+ }\n\n temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start);\n\n if (!CloseHandle(hmap))\n warning(\"unable to close file mapping handle\");\n-- snap --\n\nLikewise, I could imagine that something like \"Ierrno\" to keep only +\nlines with matching substrings (and treating - lines correspondingly).\n\nHaving said that, I find myself occasionally powering up a full-fledged\n`git gui` just to stage individual lines. If I had an 'L' command in `git\nadd -p` instead that would present the following hunks, I would be really\nhappy:\n\n-- snip --\n@@ -23,6 +23,7 @@ void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t of\n\n hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL,\n PAGE_WRITECOPY, 0, 0, NULL);\n+error(\"hmap: %p\", hmap);\n\n if (!hmap)\n return MAP_FAILED;\n-- snap --\n\nand then\n\n-- snip --\n@@ -24,6 +24,5 @@ void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t of\n hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL,\n PAGE_WRITECOPY, 0, 0, NULL);\n\n- if (!hmap)\n return MAP_FAILED;\n\n temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start);\n-- snap --\n\nand then\n\n-- snip --\n@@ -24,6 +24,7 @@ void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t of\n hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL,\n PAGE_WRITECOPY, 0, 0, NULL);\n\n+ if (!hmap) {\n return MAP_FAILED;\n\n temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start);\n-- snap --\n\nand so on.\n\nWhat do you think?\n\nCiao,\nDscho\n"},{"id":"271733","messageId":"561FAC12.5020007@gmx.de","threadId":"40558","inReplyTo":"alpine.DEB.1.00.1510151152470.31610@s15462909.onlinehome-server.info","subject":"Re: Alternative to manual editing with git add --patch","fromName":"Sven Helmberger","fromEmail":"sven.helmberger@gmx.de","sentAt":"2015-10-15T13:37:22Z","receivedAt":"2015-10-15T13:37:22Z","isPatch":false,"sender":{"key":"sven.helmberger@gmx.de","avatar":"https://gravatar.com/avatar/6acfa8b9062e6566b78555a6397ea6f37707a2a5ce729c36de672bce5aee2c11?d=mp&s=160"},"body":"Am 15.10.2015 um 12:11 schrieb Johannes Schindelin:\n> \n> This is all technically sound. From a usability perspective I would wish\n> more for a way to exclude or filter the lines by a pattern. \n\nWhy not do both?\n\nThe only thing that is unfortunate is that the \"/\" already is a command.\n\nWhich would point to either a solution along the lines of \"s\"\nbeing the split-by-line and \"s/\" being the split-by-regex?\n\nOr is this an argument for introducing yet another interaction mode\nentered when \"s\" fails to split further -- with simple \"/\" and \"\"\nbeing the options in that mode.\n\nRegards,\nSven Helmberger\n"},{"id":"271736","messageId":"alpine.DEB.1.00.1510151702420.31610@s15462909.onlinehome-server.info","threadId":"40558","inReplyTo":"561FAC12.5020007@gmx.de","subject":"Re: Alternative to manual editing with git add --patch","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-10-15T15:06:03Z","receivedAt":"2015-10-15T15:06:03Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Sven,\n\nOn Thu, 15 Oct 2015, Sven Helmberger wrote:\n\n> Am 15.10.2015 um 12:11 schrieb Johannes Schindelin:\n> > \n> > This is all technically sound. From a usability perspective I would wish\n> > more for a way to exclude or filter the lines by a pattern. \n> \n> Why not do both?\n\nBecause sometimes more is less. If users are overwhelmed with many, many\noptions, they are *less* likely to benefit from the few that are easy to\nuse because they won't find out about them.\n\nCiao,\nJohannes\n"},{"id":"271737","messageId":"561FC4CF.9010505@gmx.de","threadId":"40558","inReplyTo":"alpine.DEB.1.00.1510151702420.31610@s15462909.onlinehome-server.info","subject":"Re: Alternative to manual editing with git add --patch","fromName":"Sven Helmberger","fromEmail":"sven.helmberger@gmx.de","sentAt":"2015-10-15T15:22:55Z","receivedAt":"2015-10-15T15:22:55Z","isPatch":false,"sender":{"key":"sven.helmberger@gmx.de","avatar":"https://gravatar.com/avatar/6acfa8b9062e6566b78555a6397ea6f37707a2a5ce729c36de672bce5aee2c11?d=mp&s=160"},"body":"Am 15.10.2015 um 17:06 schrieb Johannes Schindelin:\n\n> \n> Because sometimes more is less. If users are overwhelmed with many, many\n> options, they are *less* likely to benefit from the few that are easy to\n> use because they won't find out about them.\n> \n\nGoing from \"I want to split at 'x'\" to doing so seems just fine. More\nthan \"Ok.. let's find the match I want to search for to have the split I\nwant\", which seems like the opposite of usable.\n\nThere are 13 options now not counting help. Not sure it matters much if\nwe increase that to 14 or 15.\n\nRegards,\nSven\n"}]}