Re: Alternative to manual editing with git add --patch
Hi Sven & Junio,
On Thu, 15 Oct 2015, Sven Helmberger wrote:
Show 21 quoted lines
> Am 14.10.2015 um 19:50 schrieb Junio C Hamano:
> > Sven Helmberger <sven.helmberger@gmx.de> 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
