From: Frédéric Heitzmann Date: Wed, 07 Sep 2011 20:02:36 GMT Subject: Re: [PATCH 1/2] git svn dcommit: new option --interactive. Message-ID: <4E67CDDC.1020305@gmail.com> In-Reply-To: <20110906202601.GA11668@dcvr.yhbt.net> Le 06/09/2011 22:26, Eric Wong a écrit : > Frédéric Heitzmann wrote: >> Allow the user to check the patch set before it is commited to SNV. It is then >> possible to accept/discard one patch, accept all, or quit. >> >> This interactive mode is similar with 'git send email' behaviour. However, >> 'git svn dcommit' returns as soon as one patch is discarded. >> >> Part of the code was taken from git-send-email.perl >> Thanks-to: Eric Wong for the initial idea. >> Signed-off-by: Frédéric Heitzmann > I agree with this feature, a few comments inline. > >> I would have preferred not duplicating the code snippets taken from >> git-send-email ('ask' function, Term related code, ...) but I preferred not >> to spoil Git.pm with it. >> Any comment on a better way to factor perl code would be appreciated. > We should put this into Git.pm at some point. > (Somebody should refactor git-svn.perl into separate files too... :x) > >> Documentation/git-svn.txt | 8 +++++ >> git-svn.perl | 71 ++++++++++++++++++++++++++++++++++++++++++++- >> 2 files changed, 78 insertions(+), 1 deletions(-) > Tests and feature should be the same patch >> + return defined $default ? $default : undef >> + unless defined $term->IN and defined fileno($term->IN) and >> + defined $term->OUT and defined fileno($term->OUT); > Things to make life easier for (mainly) C programmers: > > * Use C-style "&&" and "||" for conditionals. "and" and "or" are lower > precedence and better used for control flow (see perlop(1) manpage). > > * Also, use parentheses for defined(foo) to disambiguate multiple > conditions/statements. > My fault : I copied-pasted the 'ask' function from git-send-email. Even if I rewrite it a litlle, it should not prevent anyone to mutalize some code into Git.pm. And, indeed, it will improve readability. I wait a few days to see if anyone else has some comments and I send a V2 patch serie. Thanks for reviewing. -- Fred