Hi Sven & Junio,

On Thu, 15 Oct 2015, Sven Helmberger wrote:

> Am 14.10.2015 um 19:50 schrieb Junio C Hamano:
> > Sven Helmberger <sven.helmber...@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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to