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@xxxxxxxxxxxxxxx
Attachment:
signature.asc
Description: This is a digitally signed message part.
