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

[Thomas Rast <trast@student.ethz.ch>]

[-- Attachment #1: Type: text/plain, Size: 2904 bytes --]

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).

> > +# 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 :-)

> > +	# 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.

> > +	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


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]