git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Alternative to manual editing with git add --patch

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Oct 15, 2015, 10:11 UTC
Message-ID
<alpine.DEB.1.00.1510151152470.31610@s15462909.onlinehome-server.info>
In-Reply-To
<561EE700.3020002@gmx.de>
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
Previous: Sven HelmbergerNext: Sven Helmberger
Message 6 of 9 in “Alternative to manual editing with git add --patch”
  1. Sven HelmbergerOct 14, 2015
  2. Matthieu MoyOct 14, 2015
  3. Johannes SchindelinOct 14, 2015
  4. Junio C HamanoOct 14, 2015
  5. Sven HelmbergerOct 14, 2015
  6. Johannes SchindelinOct 15, 2015
  7. Sven HelmbergerOct 15, 2015
  8. Johannes SchindelinOct 15, 2015
  9. Sven HelmbergerOct 15, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.