Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
On Thu, Nov 15, 2012 at 09:17:30PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: That is a good question. That confirmation step does come after they have typed their cover letter. However, if they are using --compose, they are dumped in their editor with something like: From Jeff King p...@peff.net # This line is ignored. GIT: Lines beginning in GIT: will be removed. GIT: Consider including an overall diffstat or table of contents GIT: for the patch you are writing. GIT: GIT: Clear the body content if you don't wish to send a summary. From: Jeff King p...@peff.net Subject: In-Reply-To: which I think would count as sufficient notice of the address being used. OK. Tentatively I replaced your old series with these 8 patches including the last one, as I tend to agree with the value the earlier clean-up in the series gives us in the longer term. As you and Felipe discussed, we may want to replace the last one with a simpler don't bother asking patch, but I think that is more or less an orthogonal issue. I'm not sure how orthogonal it is. The latter half of my series is about exposing the user_ident_sufficiently_given() flag. If we go with Felipe's patch, then that exposed information has no users, and it may not be worth it (OTOH, it's possible that some third-party script may want it). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
On Fri, Nov 16, 2012 at 8:08 PM, Jeff King p...@peff.net wrote: On Thu, Nov 15, 2012 at 09:17:30PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: That is a good question. That confirmation step does come after they have typed their cover letter. However, if they are using --compose, they are dumped in their editor with something like: From Jeff King p...@peff.net # This line is ignored. GIT: Lines beginning in GIT: will be removed. GIT: Consider including an overall diffstat or table of contents GIT: for the patch you are writing. GIT: GIT: Clear the body content if you don't wish to send a summary. From: Jeff King p...@peff.net Subject: In-Reply-To: which I think would count as sufficient notice of the address being used. OK. Tentatively I replaced your old series with these 8 patches including the last one, as I tend to agree with the value the earlier clean-up in the series gives us in the longer term. As you and Felipe discussed, we may want to replace the last one with a simpler don't bother asking patch, but I think that is more or less an orthogonal issue. I'm not sure how orthogonal it is. The latter half of my series is about exposing the user_ident_sufficiently_given() flag. If we go with Felipe's patch, then that exposed information has no users, and it may not be worth it (OTOH, it's possible that some third-party script may want it). Well, who is using user_ident_sufficiently_given() in the first place? I think 'git commit' might be suffering from the same problem that prompted you to split it. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
Jeff King p...@peff.net writes: On Thu, Nov 15, 2012 at 09:17:30PM -0800, Junio C Hamano wrote: ... OK. Tentatively I replaced your old series with these 8 patches including the last one, as I tend to agree with the value the earlier clean-up in the series gives us in the longer term. As you and Felipe discussed, we may want to replace the last one with a simpler don't bother asking patch, but I think that is more or less an orthogonal issue. I'm not sure how orthogonal it is. The latter half of my series is about exposing the user_ident_sufficiently_given() flag. If we go with Felipe's patch, then that exposed information has no users, and it may not be worth it (OTOH, it's possible that some third-party script may want it). Oh, no, you are correct. I shouldn't have said the last one, but instead the later ones. The first two we should definitely keep, at least, but the other three are not so clear. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
On Fri, Nov 16, 2012 at 08:57:43PM +0100, Felipe Contreras wrote: I'm not sure how orthogonal it is. The latter half of my series is about exposing the user_ident_sufficiently_given() flag. If we go with Felipe's patch, then that exposed information has no users, and it may not be worth it (OTOH, it's possible that some third-party script may want it). Well, who is using user_ident_sufficiently_given() in the first place? I think 'git commit' might be suffering from the same problem that prompted you to split it. It is just `git commit` now. It does not suffer from the problems that prompted the author/committer split: http://article.gmane.org/gmane.comp.version-control.git/209635 To expand on what I wrote there, we cannot hit case 2 because we always ask for the committer within the same process. Case 1 is not interesting, because we would only fail to show it if is identical to a non-implicit committer (so even if it was implicit, we know that it is a sane value). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
Jeff King wrote: --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -191,15 +191,47 @@ test_expect_success $PREREQ 'Show all headers' ' test_expect_success $PREREQ 'Prompting works' ' clean_fake_sendmail - (echo Example f...@example.com - echo t...@example.com + (echo t...@example.com echo ) | GIT_SEND_EMAIL_NOTTY=1 git send-email \ --smtp-server=$(pwd)/fake.sendmail \ $patches \ 2errors + grep ^From: A U Thor aut...@example.com\$ msgtxt1 + grep ^To: t...@example.com\$ msgtxt1 +' The indentation seems strange here --- are the new grep lines continuations of the git send-email line? It's probably easier to change the structure completely: clean_fake_sendmail echo t...@examples.com prompt.input echo prompt.input GIT_SEND_EMAIL_NOTTY=1 \ git send-email --smtp-server=... $patches prompt.input grep ^From: A U Thor authorid...@example.com\$ msgtxt1 grep ^To: t...@example.com\$ msgtxt1 +test_expect_success $PREREQ,AUTOIDENT 'implicit ident prompts for sender' ' + clean_fake_sendmail + (echo Example f...@example.com + echo t...@example.com + echo + ) | + (sane_unset GIT_AUTHOR_NAME + sane_unset GIT_AUTHOR_EMAIL + sane_unset GIT_COMMITTER_NAME + sane_unset GIT_COMMITTER_EMAIL + GIT_SEND_EMAIL_NOTTY=1 git send-email \ + --smtp-server=$(pwd)/fake.sendmail \ + $patches \ + 2errors grep ^From: Example f...@example.com\$ msgtxt1 grep ^To: t...@example.com\$ msgtxt1 + ) +' Likewise: clean_fake_sendmail echo Example f...@example.com prompt.in echo t...@example.com prompt.in echo prompt.in ( sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL sane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL GIT_SEND_EMAIL_NOTTY=1 \ git send-email --smtp-server=... $patches prompt.in ) grep ^From: Example f...@example.com\$ msgtxt1 grep ^To: t...@example.com\$ msgtxt1 +test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts send-email' ' + clean_fake_sendmail + (sane_unset GIT_AUTHOR_NAME + sane_unset GIT_AUTHOR_EMAIL + sane_unset GIT_COMMITTER_NAME + sane_unset GIT_COMMITTER_EMAIL + GIT_SEND_EMAIL_NOTTY=1 export GIT_SEND_EMAIL_NOTTY + test_must_fail git send-email \ + --smtp-server=$(pwd)/fake.sendmail \ + $patches /dev/null 2errors.out + test_i18ngrep tell me who you are errors.out + ) ' Likewise: clean_fake_sendmail ( sane_unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL sane_unset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL GIT_SEND_EMAIL_NOTTY=1 \ git send-email --smtp-server=... $patches /dev/null 2err ) test_i18ngrep [Tt]ell me who you are err For what it's worth, with or without such changes, Acked-by: Jonathan Nieder jrnie...@gmail.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
On Thu, Nov 15, 2012 at 03:08:42AM +0100, Felipe Contreras wrote:
I don't think there's any need for all that, this does the trick:
diff --git a/git-send-email.perl b/git-send-email.perl
index aea66a0..503e551 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -748,16 +748,11 @@ if (!$force) {
}
}
-my $prompting = 0;
if (!defined $sender) {
$sender = $repoauthor || $repocommitter || '';
- $sender = ask(Who should the emails appear to be from? [$sender] ,
- default = $sender,
- valid_re = qr/\@.*\./, confirm_only = 1);
- print Emails will be sent from: , $sender, \n;
- $prompting++;
}
+my $prompting = 0;
This passes all the current tests and the ones you added.
It may pass on your system, but it will not on a system that meets the
AUTOIDENT prerequisite (it fails the new t9001.19 on my system; I
suspect your system config is such that we skip t9001.19 and run
t9001.20, whereas mine is the opposite).
Which kind of user will get the prompt with your patch, that would
miss it with mine?
One whose system is configured in such a way that git can produce an
automatic ident (i.e., has a non-blank GECOS name and a FQDN).
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
On Thu, Nov 15, 2012 at 9:33 AM, Jeff King p...@peff.net wrote:
On Thu, Nov 15, 2012 at 03:08:42AM +0100, Felipe Contreras wrote:
I don't think there's any need for all that, this does the trick:
diff --git a/git-send-email.perl b/git-send-email.perl
index aea66a0..503e551 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -748,16 +748,11 @@ if (!$force) {
}
}
-my $prompting = 0;
if (!defined $sender) {
$sender = $repoauthor || $repocommitter || '';
- $sender = ask(Who should the emails appear to be from? [$sender] ,
- default = $sender,
- valid_re = qr/\@.*\./, confirm_only = 1);
- print Emails will be sent from: , $sender, \n;
- $prompting++;
}
+my $prompting = 0;
This passes all the current tests and the ones you added.
It may pass on your system, but it will not on a system that meets the
AUTOIDENT prerequisite (it fails the new t9001.19 on my system; I
suspect your system config is such that we skip t9001.19 and run
t9001.20, whereas mine is the opposite).
I tried both:
ok 19 # skip implicit ident prompts for sender (missing AUTOIDENT of
PERL,AUTOIDENT)
ok 20 - broken implicit ident aborts send-email
ok 19 - implicit ident prompts for sender
ok 20 # skip broken implicit ident aborts send-email (missing
!AUTOIDENT of PERL,!AUTOIDENT)
However, it would be much easier if ident learned to check
GIT_TEST_FAKE_HOSTNAME, or something.
But then I realized I had to run 'make' again. Yes, my patch breaks
test 19, but see below.
Which kind of user will get the prompt with your patch, that would
miss it with mine?
One whose system is configured in such a way that git can produce an
automatic ident (i.e., has a non-blank GECOS name and a FQDN).
And doesn't have any of the following:
* configured user.name/user.email
* specified $EMAIL
* configured sendemail.from
* specified --from argument
Very unlikely. And then, what would be the consequences of not
receiving this prompt?
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
On Thu, Nov 15, 2012 at 11:28:46AM +0100, Felipe Contreras wrote: I tried both: ok 19 # skip implicit ident prompts for sender (missing AUTOIDENT of PERL,AUTOIDENT) ok 20 - broken implicit ident aborts send-email ok 19 - implicit ident prompts for sender ok 20 # skip broken implicit ident aborts send-email (missing !AUTOIDENT of PERL,!AUTOIDENT) However, it would be much easier if ident learned to check GIT_TEST_FAKE_HOSTNAME, or something. Yes, it would be. It has two downsides: 1. The regular git code has to be instrumented to respect the variable, so it can potentially affect git in production use outside of the test suite. Since such code is simple, though, it is probably not a big risk. 2. We would not actually exercise the code paths for doing hostname and GECOS lookup. We do not test their resulting values, so the coverage is not great now, but we do at least run the code, which would let a run with --valgrind check it. I guess we could go through the motions of assembling the ident and then replace it at the end with the fake value. I don't have a strong opinion either way. One whose system is configured in such a way that git can produce an automatic ident (i.e., has a non-blank GECOS name and a FQDN). And doesn't have any of the following: * configured user.name/user.email * specified $EMAIL * configured sendemail.from * specified --from argument Very unlikely. That is certainly the opinion you have stated already. I'm not sure I agree. Linus, for example, was an advocate of such a configuration early on in git's history. I don't think he still runs that way, though. And then, what would be the consequences of not receiving this prompt? An email would be sent with the generated identity. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
On Thu, Nov 15, 2012 at 02:43:58AM -0800, Jeff King wrote: And doesn't have any of the following: * configured user.name/user.email * specified $EMAIL * configured sendemail.from * specified --from argument Very unlikely. That is certainly the opinion you have stated already. I'm not sure I agree. Linus, for example, was an advocate of such a configuration early on in git's history. I don't think he still runs that way, though. And then, what would be the consequences of not receiving this prompt? An email would be sent with the generated identity. I suspect you did not need me to answer that question, but were setting it up as a rhetorical trap to mention the final confirmation, which I failed to note in my response. I think a much more compelling argument/commit message for your suggested patch would be: We currently prompt the user for the From address. This is an inconvenience in the common case that the user has configured their identity in the environment, but is meant as a safety check for when git falls back to an implicitly generated identity (which may or may not be valid). That safety check is not really necessary, though, as by default send-email will prompt the user for a final confirmation before sending out any message. The likelihood that a user has both bothered to turn off this default _and_ not configured any identity (nor checked that the automatic identity is valid) is rather low. I could accept that line of reasoning. I see that this argument is buried deep in your commit message, but I will admit to not reading your 9-point list of conditions all that closely, as the first 7 points are, in my opinion, not relevant (and I had already read and disagreed with them in other messages). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
On Thu, Nov 15, 2012 at 03:13:47AM -0800, Jeff King wrote: I think a much more compelling argument/commit message for your suggested patch would be: We currently prompt the user for the From address. This is an inconvenience in the common case that the user has configured their identity in the environment, but is meant as a safety check for when git falls back to an implicitly generated identity (which may or may not be valid). That safety check is not really necessary, though, as by default send-email will prompt the user for a final confirmation before sending out any message. The likelihood that a user has both bothered to turn off this default _and_ not configured any identity (nor checked that the automatic identity is valid) is rather low. If that is the route we want to go, then we should obviously drop my series in favor of your final patch. I think it would also need a test update, no? I think a more concise commit message would help, too. I disagree with a great deal of the reasoning in your existing message, but those parts turn out not to be relevant. The crux of the issue is that the safety check is not necessary because there is already one (i.e., point 8 of your list). Feel free to use any or all of my text above. From my series, there were a few cleanups that might be worth salvaging. Here is a rundown by patch: [1/8]: test-lib: allow negation of prerequisites This stands on its own, and is something I have wanted a few times in the past. However, since there is no immediate user, I don't know if it is worth doing or not. [2/8]: t7502: factor out autoident prerequisite A minor cleanup and possible help to future tests, but since there are no other callers now, not sure if it is worth it. [3/8]: ident: make user_ident_explicitly_given static A cleanup that is worth doing, I think. [4/8]: ident: keep separate explicit flags for author and committer Another cleanup. This is more correct, in that it handles the corner cases I mentioned in the commit message. But no current code cares about those corner cases, because the only real caller is git-commit, and this is a purely internal interface. I could take or leave it. [5/8]: var: accept multiple variables on the command line The tests for this can be split out; we currently don't have git var tests at all, so increasing our test coverage is reasonable. The multiple variables thing is potentially useful, but there are simply not that many callers of git var, and nobody has been asking for such a feature (we could use it to save a process in git-send-email, but it is probably not worth the complexity). [6/8]: var: provide explicit/implicit ident information [7/8]: Git.pm: teach ident to query explicitness These two should probably be dropped. They would lock us into supporting the explicit/implicit variables in git var, for no immediate benefit. [8/8]: send-email: do not prompt for explicit repo ident Obviously drop. I could accept that line of reasoning. I see that this argument is buried deep in your commit message, but I will admit to not reading your 9-point list of conditions all that closely, as the first 7 points are, in my opinion, not relevant (and I had already read and disagreed with them in other messages). If it sounds like I am blaming you here, I am to some degree. But I am also blaming myself. I should have read your commit message more carefully, and I'm sorry for not doing so. I hope we can both try harder to avoid getting side-tracked on arguing about issues that turned out not to be important at all (of course, we cannot know which part of the discussion will turn out to be important, but I think there some obviously unproductive parts of this discussion). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
Jeff King p...@peff.net writes: I think a much more compelling argument/commit message for your suggested patch would be: We currently prompt the user for the From address. This is an inconvenience in the common case that the user has configured their identity in the environment, but is meant as a safety check for when git falls back to an implicitly generated identity (which may or may not be valid). That safety check is not really necessary, though, as by default send-email will prompt the user for a final confirmation before sending out any message. The likelihood that a user has both bothered to turn off this default _and_ not configured any identity (nor checked that the automatic identity is valid) is rather low. This somehow reminds me of the first paragraph of f20f387 (commit: check committer identity more strictly, 2012-07-23). I never use send-email driving format-patch workflow myself, but I suspect there are people among who do so who are using --compose to do the cover letter of their series. Does the confirmation as the last step help them, or would they have to retype their message? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
On Thu, Nov 15, 2012 at 08:56:37AM -0800, Junio C Hamano wrote: I think a much more compelling argument/commit message for your suggested patch would be: We currently prompt the user for the From address. This is an inconvenience in the common case that the user has configured their identity in the environment, but is meant as a safety check for when git falls back to an implicitly generated identity (which may or may not be valid). That safety check is not really necessary, though, as by default send-email will prompt the user for a final confirmation before sending out any message. The likelihood that a user has both bothered to turn off this default _and_ not configured any identity (nor checked that the automatic identity is valid) is rather low. This somehow reminds me of the first paragraph of f20f387 (commit: check committer identity more strictly, 2012-07-23). I never use send-email driving format-patch workflow myself, but I suspect there are people among who do so who are using --compose to do the cover letter of their series. Does the confirmation as the last step help them, or would they have to retype their message? That is a good question. That confirmation step does come after they have typed their cover letter. However, if they are using --compose, they are dumped in their editor with something like: From Jeff King p...@peff.net # This line is ignored. GIT: Lines beginning in GIT: will be removed. GIT: Consider including an overall diffstat or table of contents GIT: for the patch you are writing. GIT: GIT: Clear the body content if you don't wish to send a summary. From: Jeff King p...@peff.net Subject: In-Reply-To: which I think would count as sufficient notice of the address being used. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
Jeff King p...@peff.net writes: That is a good question. That confirmation step does come after they have typed their cover letter. However, if they are using --compose, they are dumped in their editor with something like: From Jeff King p...@peff.net # This line is ignored. GIT: Lines beginning in GIT: will be removed. GIT: Consider including an overall diffstat or table of contents GIT: for the patch you are writing. GIT: GIT: Clear the body content if you don't wish to send a summary. From: Jeff King p...@peff.net Subject: In-Reply-To: which I think would count as sufficient notice of the address being used. OK. Tentatively I replaced your old series with these 8 patches including the last one, as I tend to agree with the value the earlier clean-up in the series gives us in the longer term. As you and Felipe discussed, we may want to replace the last one with a simpler don't bother asking patch, but I think that is more or less an orthogonal issue. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 8/8] send-email: do not prompt for explicit repo ident
If git-send-email is configured with sendemail.from, we will
not prompt the user for the From address of the emails.
If it is not configured, we prompt the user, but provide the
repo author or committer as a default. Even though we
probably have a sensible value for the default, the prompt
is a safety check in case git generated an incorrect
implicit ident string.
Now that Git.pm will tell us whether the ident is implicit or
explicit, we can stop prompting in the explicit case, saving
most users from having to see the prompt at all.
Signed-off-by: Jeff King p...@peff.net
---
git-send-email.perl | 22 +-
t/t9001-send-email.sh | 36 ++--
2 files changed, 47 insertions(+), 11 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 5a7c29d..0c49b32 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -436,9 +436,8 @@ if (0) {
}
}
-my ($repoauthor, $repocommitter);
-($repoauthor) = Git::ident_person(@repo, 'author');
-($repocommitter) = Git::ident_person(@repo, 'committer');
+my ($repoauthor, $author_explicit) = Git::ident_person(@repo, 'author');
+my ($repocommitter, $committer_explicit) = Git::ident_person(@repo,
'committer');
# Verify the user input
@@ -755,12 +754,17 @@ if (!$force) {
my $prompting = 0;
if (!defined $sender) {
- $sender = $repoauthor || $repocommitter || '';
- $sender = ask(Who should the emails appear to be from? [$sender] ,
- default = $sender,
- valid_re = qr/\@.*\./, confirm_only = 1);
- print Emails will be sent from: , $sender, \n;
- $prompting++;
+ ($sender, my $explicit) =
+ defined $repoauthor ? ($repoauthor, $author_explicit) :
+ defined $repocommitter ? ($repocommitter, $committer_explicit) :
+ ('', 0);
+ if (!$explicit) {
+ $sender = ask(Who should the emails appear to be from?
[$sender] ,
+ default = $sender,
+ valid_re = qr/\@.*\./, confirm_only = 1);
+ print Emails will be sent from: , $sender, \n;
+ $prompting++;
+ }
}
if (!@initial_to !defined $to_cmd) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 6c6af7d..0fe0b8e 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -191,15 +191,47 @@ test_expect_success $PREREQ 'Show all headers' '
test_expect_success $PREREQ 'Prompting works' '
clean_fake_sendmail
- (echo Example f...@example.com
-echo t...@example.com
+ (echo t...@example.com
echo
) | GIT_SEND_EMAIL_NOTTY=1 git send-email \
--smtp-server=$(pwd)/fake.sendmail \
$patches \
2errors
+ grep ^From: A U Thor aut...@example.com\$ msgtxt1
+ grep ^To: t...@example.com\$ msgtxt1
+'
+
+test_expect_success $PREREQ,AUTOIDENT 'implicit ident prompts for sender' '
+ clean_fake_sendmail
+ (echo Example f...@example.com
+echo t...@example.com
+echo
+ ) |
+ (sane_unset GIT_AUTHOR_NAME
+sane_unset GIT_AUTHOR_EMAIL
+sane_unset GIT_COMMITTER_NAME
+sane_unset GIT_COMMITTER_EMAIL
+GIT_SEND_EMAIL_NOTTY=1 git send-email \
+ --smtp-server=$(pwd)/fake.sendmail \
+ $patches \
+ 2errors
grep ^From: Example f...@example.com\$ msgtxt1
grep ^To: t...@example.com\$ msgtxt1
+ )
+'
+
+test_expect_success $PREREQ,!AUTOIDENT 'broken implicit ident aborts
send-email' '
+ clean_fake_sendmail
+ (sane_unset GIT_AUTHOR_NAME
+sane_unset GIT_AUTHOR_EMAIL
+sane_unset GIT_COMMITTER_NAME
+sane_unset GIT_COMMITTER_EMAIL
+GIT_SEND_EMAIL_NOTTY=1 export GIT_SEND_EMAIL_NOTTY
+test_must_fail git send-email \
+ --smtp-server=$(pwd)/fake.sendmail \
+ $patches /dev/null 2errors.out
+ test_i18ngrep tell me who you are errors.out
+ )
'
test_expect_success $PREREQ 'tocmd works' '
--
1.8.0.207.gdf2154c
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 8/8] send-email: do not prompt for explicit repo ident
On Thu, Nov 15, 2012 at 1:36 AM, Jeff King p...@peff.net wrote:
diff --git a/git-send-email.perl b/git-send-email.perl
index 5a7c29d..0c49b32 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -436,9 +436,8 @@ if (0) {
}
}
-my ($repoauthor, $repocommitter);
-($repoauthor) = Git::ident_person(@repo, 'author');
-($repocommitter) = Git::ident_person(@repo, 'committer');
+my ($repoauthor, $author_explicit) = Git::ident_person(@repo, 'author');
+my ($repocommitter, $committer_explicit) = Git::ident_person(@repo,
'committer');
# Verify the user input
@@ -755,12 +754,17 @@ if (!$force) {
my $prompting = 0;
if (!defined $sender) {
- $sender = $repoauthor || $repocommitter || '';
- $sender = ask(Who should the emails appear to be from? [$sender] ,
- default = $sender,
- valid_re = qr/\@.*\./, confirm_only = 1);
- print Emails will be sent from: , $sender, \n;
- $prompting++;
+ ($sender, my $explicit) =
+ defined $repoauthor ? ($repoauthor, $author_explicit) :
+ defined $repocommitter ? ($repocommitter,
$committer_explicit) :
+ ('', 0);
+ if (!$explicit) {
+ $sender = ask(Who should the emails appear to be from?
[$sender] ,
+ default = $sender,
+ valid_re = qr/\@.*\./, confirm_only = 1);
+ print Emails will be sent from: , $sender, \n;
+ $prompting++;
+ }
}
if (!@initial_to !defined $to_cmd) {
I don't think there's any need for all that, this does the trick:
diff --git a/git-send-email.perl b/git-send-email.perl
index aea66a0..503e551 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -748,16 +748,11 @@ if (!$force) {
}
}
-my $prompting = 0;
if (!defined $sender) {
$sender = $repoauthor || $repocommitter || '';
- $sender = ask(Who should the emails appear to be from? [$sender] ,
- default = $sender,
- valid_re = qr/\@.*\./, confirm_only = 1);
- print Emails will be sent from: , $sender, \n;
- $prompting++;
}
+my $prompting = 0;
This passes all the current tests and the ones you added.
Which kind of user will get the prompt with your patch, that would
miss it with mine?
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
