# Alternative to manual editing with git add --patch 9 messages from 2015-10-14 to 2015-10-15. Participants: Sven Helmberger, Matthieu Moy, Johannes Schindelin, Junio C Hamano. Thread: https://gitlist.dev/t/40558 ## Sven Helmberger, 2015-10-14 15:07 Subject: Alternative to manual editing with git add --patch Message-ID: <561E6FBB.7060302@gmx.de> URL: https://gitlist.dev/e/561E6FBB.7060302%40gmx.de ``` Hello, I hope this hasn't been discussed before. I'm a big fan of cleanliness in commits and therefore often use git add --patch to sort code changes I made into the right commits etc. What I then often encountered was the situation where I happened to have inserted consecutive lines of code that conceptually belong to different commits. Normally I can nicely split patches, but not in this case, making manually editing the patch the only alternative. Shouldn't there be at least a way to quickly say line-by-line if you want to have it added or not? Personally, I find manually editing just annoying, it seems overly arcane, but it also prevents me from really recommending "add --patch" as best practice. I think it's a really good idea for many reasons to do so, but I can't really tell people already struggling with using git that I expect them to edit patches manually. Regards, Sven Helmberger ``` ## Matthieu Moy, 2015-10-14 16:30 Subject: Re: Alternative to manual editing with git add --patch Message-ID: URL: https://gitlist.dev/e/vpqeggx9zul.fsf%40grenoble-inp.fr In-Reply-To: <561E6FBB.7060302@gmx.de> ``` Sven Helmberger writes: > Hello, > > I hope this hasn't been discussed before. > > I'm a big fan of cleanliness in commits and therefore often use git add > --patch to sort code changes I made into the right commits etc. > > What I then often encountered was the situation where I happened to have > inserted consecutive lines of code that conceptually belong to different > commits. Normally I can nicely split patches, but not in this case, > making manually editing the patch the only alternative. > > Shouldn't there be at least a way to quickly say line-by-line if you > want to have it added or not? Many GUI or text-editor plugins for Git allow you to just select lines and stage the selection. Even though I love "git add -p", I find magit (Git integration for Emacs) more convenient when it comes to staging individual lines. That said, a "split hunk line by line" option for "git add -p" could be nice. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ``` ## Johannes Schindelin, 2015-10-14 16:52 Subject: Re: Alternative to manual editing with git add --patch Message-ID: <1326b047519c9398a0a06e27a26295ca@dscho.org> URL: https://gitlist.dev/e/1326b047519c9398a0a06e27a26295ca%40dscho.org In-Reply-To: <561E6FBB.7060302@gmx.de> ``` Hi Sven, On 2015-10-14 17:07, Sven Helmberger wrote: > What I then often encountered was the situation where I happened to have > inserted consecutive lines of code that conceptually belong to different > commits. Normally I can nicely split patches, but not in this case, > making manually editing the patch the only alternative. How about implementing it and then discussing the implementation? That would have two advantages: 1) there would be something tangential to discuss, and 2) even if Junio rejects it, you can carry the patch in your private fork and use it forever. Ciao, Johannes ``` ## Junio C Hamano, 2015-10-14 17:50 Subject: Re: Alternative to manual editing with git add --patch Message-ID: URL: https://gitlist.dev/e/xmqqk2qp8hlj.fsf%40gitster.mtv.corp.google.com In-Reply-To: <561E6FBB.7060302@gmx.de> ``` Sven Helmberger writes: > I hope this hasn't been discussed before. > > I'm a big fan of cleanliness in commits and therefore often use git add > --patch to sort code changes I made into the right commits etc. > > What I then often encountered was the situation where I happened to have > inserted consecutive lines of code that conceptually belong to different > commits. Normally I can nicely split patches, but not in this case, > making manually editing the patch the only alternative. > > Shouldn't there be at least a way to quickly say line-by-line if you > want to have it added or not? I think this has been discussed a few times (and you should actually hope that is the case---it shows that something that allows you to split a hunk that consists of consecutive lines is not an obscure and useless feature wish). But I do not think we saw anybody came up with a convincingly usable design (not the code design but how the user interaction specifies where the hunk is cut in the first place), let alone a prototype for it, so that we can discuss further. At least not yet. As a quick-and-dirty change, you could invent a new variant of 's'plit that breaks a N-line hunk into N hunks with 1-line each, but obviously that would not be a pleasant-enough UI to be called usable when you have a hunk that adds 100 lines. Perhaps "Split this hunk into two by ending the earlier one immediately before the line that has this substring" or something might be an idea? ``` ## Sven Helmberger, 2015-10-14 23:36 Subject: Re: Alternative to manual editing with git add --patch Message-ID: <561EE700.3020002@gmx.de> URL: https://gitlist.dev/e/561EE700.3020002%40gmx.de In-Reply-To: ``` Am 14.10.2015 um 19:50 schrieb Junio C Hamano: > Sven Helmberger writes: > > As a quick-and-dirty change, you could invent a new variant of > 's'plit that breaks a N-line hunk into N hunks with 1-line each, but > obviously that would not be a pleasant-enough UI to be called usable > when you have a hunk that adds 100 lines. Perhaps "Split this hunk > into two by ending the earlier one immediately before the line that > has this substring" or something might be an idea? > If we go by the style of interaction in git add --patch and git add --interactive, I think the most canonical solution would be to implement it like this. If we know when we can't split the current patch any further ( the point at which selecting s changes nothing anymore), why shouldn't add --patch not work similar to add --interactive in that it prints the lines of the diff prefixed with numbers and the user can define a numerical range to "split off". Then they decide whether to add those lines or not and return to the line-numbers until they're trough with the patch. Regards, Sven Helmberger ``` ## Johannes Schindelin, 2015-10-15 10:11 Subject: Re: Alternative to manual editing with git add --patch Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.1510151152470.31610%40s15462909.onlinehome-server.info In-Reply-To: <561EE700.3020002@gmx.de> ``` Hi Sven & Junio, On Thu, 15 Oct 2015, Sven Helmberger wrote: > Am 14.10.2015 um 19:50 schrieb Junio C Hamano: > > Sven Helmberger writes: > > > > As a quick-and-dirty change, you could invent a new variant of > > 's'plit that breaks a N-line hunk into N hunks with 1-line each, but > > obviously that would not be a pleasant-enough UI to be called usable > > when you have a hunk that adds 100 lines. Perhaps "Split this hunk > > into two by ending the earlier one immediately before the line that > > has this substring" or something might be an idea? > > > > If we go by the style of interaction in git add --patch and git add > --interactive, I think the most canonical solution would be to implement > it like this. > > If we know when we can't split the current patch any further ( the point > at which selecting s changes nothing anymore), why shouldn't add --patch > not work similar to add --interactive in that it prints the lines of the > diff prefixed with numbers and the user can define a numerical range to > "split off". Then they decide whether to add those lines or not and > return to the line-numbers until they're trough with the patch. This is all technically sound. From a usability perspective I would wish more for a way to exclude or filter the lines by a pattern. Take for example a diff like this (which is *quite* normal in my workflow): -- snip -- diff --git a/80a8c9a..001a983 b/001a983 index 80a8c9a..001a983 100644 --- a/80a8c9a..001a983 +++ b/001a983 @@ -23,11 +23,15 @@ void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t of hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL, PAGE_WRITECOPY, 0, 0, NULL); +error("hmap: %p", hmap); - if (!hmap) + if (!hmap) { + errno = EINVAL; return MAP_FAILED; + } temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start); +error("temp: %p 0x%x", temp, (unsigned int)GetLastError()); if (!CloseHandle(hmap)) warning("unable to close file mapping handle"); -- snap -- (Yes, I am lazy and reuse the `error()` function for debug log messages.) It is quite obvious what I would like to do in this case: keep the change that sets the errno. I would be really thankful if I had a shortcut in `git add -p` that let me specify, say, "Xerror" to eXclude any + line (and replace the '-' by a ' ' in every - line) that contains the substring 'error'. The way I envisage this command to work would be to present the filtered diff in the next step: -- snip -- @@ -23,11 +23,13 @@ void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t of hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL, PAGE_WRITECOPY, 0, 0, NULL); - if (!hmap) + if (!hmap) { + errno = EINVAL; return MAP_FAILED; + } temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start); if (!CloseHandle(hmap)) warning("unable to close file mapping handle"); -- snap -- Likewise, I could imagine that something like "Ierrno" to keep only + lines with matching substrings (and treating - lines correspondingly). Having said that, I find myself occasionally powering up a full-fledged `git gui` just to stage individual lines. If I had an 'L' command in `git add -p` instead that would present the following hunks, I would be really happy: -- snip -- @@ -23,6 +23,7 @@ void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t of hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL, PAGE_WRITECOPY, 0, 0, NULL); +error("hmap: %p", hmap); if (!hmap) return MAP_FAILED; -- snap -- and then -- snip -- @@ -24,6 +24,5 @@ void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t of hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL, PAGE_WRITECOPY, 0, 0, NULL); - if (!hmap) return MAP_FAILED; temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start); -- snap -- and then -- snip -- @@ -24,6 +24,7 @@ void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t of hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL, PAGE_WRITECOPY, 0, 0, NULL); + if (!hmap) { return MAP_FAILED; temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start); -- snap -- and so on. What do you think? Ciao, Dscho ``` ## Sven Helmberger, 2015-10-15 13:37 Subject: Re: Alternative to manual editing with git add --patch Message-ID: <561FAC12.5020007@gmx.de> URL: https://gitlist.dev/e/561FAC12.5020007%40gmx.de In-Reply-To: ``` Am 15.10.2015 um 12:11 schrieb Johannes Schindelin: > > This is all technically sound. From a usability perspective I would wish > more for a way to exclude or filter the lines by a pattern. Why not do both? The only thing that is unfortunate is that the "/" already is a command. Which would point to either a solution along the lines of "s" being the split-by-line and "s/" being the split-by-regex? Or is this an argument for introducing yet another interaction mode entered when "s" fails to split further -- with simple "/" and "" being the options in that mode. Regards, Sven Helmberger ``` ## Johannes Schindelin, 2015-10-15 15:06 Subject: Re: Alternative to manual editing with git add --patch Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.1510151702420.31610%40s15462909.onlinehome-server.info In-Reply-To: <561FAC12.5020007@gmx.de> ``` Hi Sven, On Thu, 15 Oct 2015, Sven Helmberger wrote: > Am 15.10.2015 um 12:11 schrieb Johannes Schindelin: > > > > This is all technically sound. From a usability perspective I would wish > > more for a way to exclude or filter the lines by a pattern. > > Why not do both? Because sometimes more is less. If users are overwhelmed with many, many options, they are *less* likely to benefit from the few that are easy to use because they won't find out about them. Ciao, Johannes ``` ## Sven Helmberger, 2015-10-15 15:22 Subject: Re: Alternative to manual editing with git add --patch Message-ID: <561FC4CF.9010505@gmx.de> URL: https://gitlist.dev/e/561FC4CF.9010505%40gmx.de In-Reply-To: ``` Am 15.10.2015 um 17:06 schrieb Johannes Schindelin: > > Because sometimes more is less. If users are overwhelmed with many, many > options, they are *less* likely to benefit from the few that are easy to > use because they won't find out about them. > Going from "I want to split at 'x'" to doing so seems just fine. More than "Ok.. let's find the match I want to search for to have the split I want", which seems like the opposite of usable. There are 13 options now not counting help. Not sure it matters much if we increase that to 14 or 15. Regards, Sven ```