Re: [PATCH] git-add--interactive: manual hunk editing mode

Junio C Hamano wrote:

People with minimum Perl installation should still be able to use "add -i"
as long as they do not use 'e' subcommand, shouldn't they?  Shouldn't we
do something like:

[...]

and disable 'e' subcommand unless $can_use_temp?

I can just call the temporary editing file something constant, like
the commit message already is.  I don't care much about that point,
and honestly assumed File::Temp came with every Perl (it's in
perl-base here).

quoted

+# To remove '-' lines, make them ' ' lines (context).
+# To remove '+' lines, delete them.
+# Lines starting with # will be removed.

Don't you want to say "Do not touch lines that begin with ' '"?

Why?  You can make them '-' instead if you really want to :-)

quoted

+	# Reinsert the first hunk header if the user accidentally deleted it

Hmm, perhaps not even giving the "@@ ... @@" lines to the editor would be
a more robust solution?

I originally left it there because Emacs automatically recounts the
header, and it also reminds the user of the function the hunk applies
to.

quoted

+	open $fh, '| git apply --recount --cached --check';

Have to wonder where the potential error message would go, and if it would
confuse the end users...

To the terminal.  If we eat that error message, the user gets
absolutely no indication as to _what_ might be wrong with the edited
patch, so I kind of deliberately left it there.

Then again the message from git-apply is not exactly transparent
either.

I recall that the original "add--interactive" carefully counted numbers in
hunks it reassembles (as it can let you split and then you can choose to
use both parts, which requires it to merge overlapping hunks back), but if
you are going to use --recount anyway, perhaps we can discard that logic?

We briefly talked about that[1], but Jeff thought it should be a
separate patch, and I agree.  Can't sneakily rip out two dozen lines
of otherwise unrelated code in my feature patch, can I?

It may make the patch application less robust, though.  I dunno.

I've become convinced it can't, apart from making it less likely to
trip over bugs in the script itself of course.

An alternative, and probably more robust, approach would be to recount
what we have in @{$mode->{TEXT}}, after letting the user edit some of
them, so that "add--interactive" still knows what it is doing after
applying your patch without having to rely on "apply --recount".

You lost me here.  Why recount the mode part?

If you mean the _hunk_ headers, that's what the first three versions
of this patch did, at the expense of a lot of code complication (and
now that git-apply implements --recount, actually _duplication_).

- Thomas


[1] http://article.gmane.org/gmane.comp.version-control.git/84698

-- 
Thomas Rast
trast@student.ethz.ch