* Alternative to manual editing with git add --patch
@ 2015-10-14 15:07 Sven Helmberger
2015-10-14 16:30 ` Matthieu Moy
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Sven Helmberger @ 2015-10-14 15:07 UTC (permalink / raw)
To: git
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Alternative to manual editing with git add --patch
2015-10-14 15:07 Alternative to manual editing with git add --patch Sven Helmberger
@ 2015-10-14 16:30 ` Matthieu Moy
2015-10-14 16:52 ` Johannes Schindelin
2015-10-14 17:50 ` Junio C Hamano
2 siblings, 0 replies; 9+ messages in thread
From: Matthieu Moy @ 2015-10-14 16:30 UTC (permalink / raw)
To: Sven Helmberger; +Cc: git
Sven Helmberger <sven.helmberger@gmx.de> 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/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Alternative to manual editing with git add --patch
2015-10-14 15:07 Alternative to manual editing with git add --patch Sven Helmberger
2015-10-14 16:30 ` Matthieu Moy
@ 2015-10-14 16:52 ` Johannes Schindelin
2015-10-14 17:50 ` Junio C Hamano
2 siblings, 0 replies; 9+ messages in thread
From: Johannes Schindelin @ 2015-10-14 16:52 UTC (permalink / raw)
To: Sven Helmberger; +Cc: git
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Alternative to manual editing with git add --patch
2015-10-14 15:07 Alternative to manual editing with git add --patch Sven Helmberger
2015-10-14 16:30 ` Matthieu Moy
2015-10-14 16:52 ` Johannes Schindelin
@ 2015-10-14 17:50 ` Junio C Hamano
2015-10-14 23:36 ` Sven Helmberger
2 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2015-10-14 17:50 UTC (permalink / raw)
To: Sven Helmberger; +Cc: git
Sven Helmberger <sven.helmberger@gmx.de> 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?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Alternative to manual editing with git add --patch
2015-10-14 17:50 ` Junio C Hamano
@ 2015-10-14 23:36 ` Sven Helmberger
2015-10-15 10:11 ` Johannes Schindelin
0 siblings, 1 reply; 9+ messages in thread
From: Sven Helmberger @ 2015-10-14 23:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
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.
Regards,
Sven Helmberger
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Alternative to manual editing with git add --patch
2015-10-14 23:36 ` Sven Helmberger
@ 2015-10-15 10:11 ` Johannes Schindelin
2015-10-15 13:37 ` Sven Helmberger
0 siblings, 1 reply; 9+ messages in thread
From: Johannes Schindelin @ 2015-10-15 10:11 UTC (permalink / raw)
To: Sven Helmberger; +Cc: Junio C Hamano, git
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.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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: Alternative to manual editing with git add --patch
2015-10-15 10:11 ` Johannes Schindelin
@ 2015-10-15 13:37 ` Sven Helmberger
2015-10-15 15:06 ` Johannes Schindelin
0 siblings, 1 reply; 9+ messages in thread
From: Sven Helmberger @ 2015-10-15 13:37 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
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<range>"
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 "<range>"
being the options in that mode.
Regards,
Sven Helmberger
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Alternative to manual editing with git add --patch
2015-10-15 13:37 ` Sven Helmberger
@ 2015-10-15 15:06 ` Johannes Schindelin
2015-10-15 15:22 ` Sven Helmberger
0 siblings, 1 reply; 9+ messages in thread
From: Johannes Schindelin @ 2015-10-15 15:06 UTC (permalink / raw)
To: Sven Helmberger; +Cc: Junio C Hamano, git
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Alternative to manual editing with git add --patch
2015-10-15 15:06 ` Johannes Schindelin
@ 2015-10-15 15:22 ` Sven Helmberger
0 siblings, 0 replies; 9+ messages in thread
From: Sven Helmberger @ 2015-10-15 15:22 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-10-15 15:23 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-14 15:07 Alternative to manual editing with git add --patch Sven Helmberger
2015-10-14 16:30 ` Matthieu Moy
2015-10-14 16:52 ` Johannes Schindelin
2015-10-14 17:50 ` Junio C Hamano
2015-10-14 23:36 ` Sven Helmberger
2015-10-15 10:11 ` Johannes Schindelin
2015-10-15 13:37 ` Sven Helmberger
2015-10-15 15:06 ` Johannes Schindelin
2015-10-15 15:22 ` Sven Helmberger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.