{"thread":{"id":"34716","subject":"[RFC] git-send-email: Cache generated message-ids, use them when prompting","startedAt":"2013-08-17T00:58:46Z","lastAt":"2013-08-22T00:20:24Z","messageCount":7,"participants":["Rasmus Villemoes","Junio C Hamano","brian m. carlson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"225363","messageId":"1376701126-5759-1-git-send-email-rv@rasmusvillemoes.dk","threadId":"34716","inReplyTo":null,"subject":"[RFC] git-send-email: Cache generated message-ids, use them when prompting","fromName":"Rasmus Villemoes","fromEmail":"rv@rasmusvillemoes.dk","sentAt":"2013-08-17T00:58:46Z","receivedAt":"2013-08-17T00:58:46Z","isPatch":false,"sender":{"key":"rv@rasmusvillemoes.dk","avatar":"https://avatars.githubusercontent.com/u/4375908?v=4"},"body":"This is mostly a proof of concept/RFC patch. The idea is for\ngit-send-email to store the message-ids it generates, along with the\nSubject and Date headers of the message. When prompting for which\nMessage-ID should be used in In-Reply-To, display a list of recent\nemails (identifed using the Date/Subject pairs; the message-ids\nthemselves are not for human consumption). Choosing from that list\nwill then use the corresponding message-id; otherwise, the behaviour\nis as usual.\n\nWhen composing v2 or v3 of a patch or patch series, this avoids the\nneed to get one's MUA to display the Message-ID of the earlier email\n(which is cumbersome in some MUAs) and then copy-paste that.\n\nIf this idea is accepted, I'm certain I'll get to use the feature\nimmediately, since the patch is not quite ready for inclusion :-)\n\nSigned-off-by: Rasmus Villemoes <rv@rasmusvillemoes.dk>\n---\n git-send-email.perl | 101 +++++++++++++++++++++++++++++++++++++++++++++++++---\n 1 file changed, 96 insertions(+), 5 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex f608d9b..2e3685c 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -26,6 +26,7 @@ use Data::Dumper;\n use Term::ANSIColor;\n use File::Temp qw/ tempdir tempfile /;\n use File::Spec::Functions qw(catfile);\n+use Date::Parse;\n use Error qw(:try);\n use Git;\n \n@@ -203,6 +204,7 @@ my ($validate, $confirm);\n my (@suppress_cc);\n my ($auto_8bit_encoding);\n my ($compose_encoding);\n+my ($msgid_cache_file, $msgid_maxprompt);\n \n my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n \n@@ -214,7 +216,7 @@ my %config_bool_settings = (\n     \"signedoffcc\" => [\\$signed_off_by_cc, undef],      # Deprecated\n     \"validate\" => [\\$validate, 1],\n     \"multiedit\" => [\\$multiedit, undef],\n-    \"annotate\" => [\\$annotate, undef]\n+    \"annotate\" => [\\$annotate, undef],\n );\n \n my %config_settings = (\n@@ -237,6 +239,7 @@ my %config_settings = (\n     \"from\" => \\$sender,\n     \"assume8bitencoding\" => \\$auto_8bit_encoding,\n     \"composeencoding\" => \\$compose_encoding,\n+    \"msgidcachefile\" => \\$msgid_cache_file,\n );\n \n my %config_path_settings = (\n@@ -311,6 +314,7 @@ my $rc = GetOptions(\"h\" => \\$help,\n \t\t    \"8bit-encoding=s\" => \\$auto_8bit_encoding,\n \t\t    \"compose-encoding=s\" => \\$compose_encoding,\n \t\t    \"force\" => \\$force,\n+\t\t    \"msgid-cache-file=s\" => \\$msgid_cache_file,\n \t );\n \n usage() if $help;\n@@ -784,10 +788,31 @@ sub expand_one_alias {\n @bcclist = validate_address_list(sanitize_address_list(@bcclist));\n \n if ($thread && !defined $initial_reply_to) {\n-\t$initial_reply_to = ask(\n-\t\t\"Message-ID to be used as In-Reply-To for the first email (if any)? \",\n-\t\tdefault => \"\",\n-\t\tvalid_re => qr/\\@.*\\./, confirm_only => 1);\n+\tmy @choices;\n+\tif ($msgid_cache_file) {\n+\t\t@choices = msgid_cache_getmatches();\n+\t}\n+\tif (@choices) {\n+\t\tmy $prompt = '';\n+\t\tmy $i = 0;\n+\t\t$prompt .= sprintf \"(%d) [%s] %s\\n\", $i++, $_->{date}, $_->{subject}\n+\t\t    for (@choices);\n+\t\t$prompt .= sprintf \"Answer 0-%d to use the Message-ID of one of the above\\n\", $#choices;\n+\t\t$prompt .= \"Message-ID to be used as In-Reply-To for the first email (if any)? \";\n+\t\t$initial_reply_to = \n+\t\t    ask($prompt,\n+\t\t\tdefault => \"\",\n+\t\t\tvalid_re => qr/\\@.*\\.|^[0-9]+$/, confirm_only => 1);\n+\t\tif ($initial_reply_to =~ /^[0-9]+$/ && $initial_reply_to < @choices) {\n+\t\t\t$initial_reply_to = $choices[$initial_reply_to]{id};\n+\t\t}\n+\t}\n+\telse {\n+\t\t$initial_reply_to = \n+\t\t    ask(\"Message-ID to be used as In-Reply-To for the first email (if any)? \",\n+\t\t\tdefault => \"\",\n+\t\t\tvalid_re => qr/\\@.*\\./, confirm_only => 1);\n+\t}\n }\n if (defined $initial_reply_to) {\n \t$initial_reply_to =~ s/^\\s*<?//;\n@@ -1282,6 +1307,8 @@ X-Mailer: git-send-email $gitversion\n \t\t}\n \t}\n \n+\tmsgid_cache_this($message_id, $subject, $date) if ($msgid_cache_file && !$dry_run);\n+\n \treturn 1;\n }\n \n@@ -1508,6 +1535,8 @@ sub cleanup_compose_files {\n \n $smtp->quit if $smtp;\n \n+msgid_cache_write() if $msgid_cache_file;\n+\n sub unique_email_list {\n \tmy %seen;\n \tmy @emails;\n@@ -1556,3 +1585,65 @@ sub body_or_subject_has_nonascii {\n \t}\n \treturn 0;\n }\n+\n+my @msgid_new_entries;\n+\n+# For now, use a simple tab-separated format:\n+#\n+#    $id\\t$date\\t$subject\\n\n+sub msgid_cache_read {\n+\tmy $fh;\n+\tmy $line;\n+\tmy @entries;\n+\tif (not open ($fh, '<', $msgid_cache_file)) {\n+\t\t# A non-existing cache file is ok, but should we warn if errno != ENOENT?\n+\t\treturn ();\n+\t}\n+\twhile ($line = <$fh>) {\n+\t\tchomp($line);\n+\t\tmy ($id, $date, $subject) = split /\\t/, $line;\n+\t\tmy $epoch = str2time($date);\n+\t\tpush @entries, {id=>$id, date=>$date, epoch=>$epoch, subject=>$subject};\n+\t}\n+\tclose($fh);\n+\treturn @entries;\n+}\n+sub msgid_cache_write {\n+\tmy $fh;\n+\tif (not open($fh, '>>', $msgid_cache_file)) {\n+\t    warn \"cannot open $msgid_cache_file for appending: $!\";\n+\t    return;\n+\t}\n+\tprintf $fh \"%s\\t%s\\t%s\\n\", $_->{id}, $_->{date}, $_->{subject} for (@msgid_new_entries);\n+\tclose($fh);\n+}\n+# Return an array of cached message-ids, ordered by \"relevance\". It\n+# might make sense to take the Subject of the new mail as an extra\n+# argument and do some kind of fuzzy matching against the old\n+# subjects, but for now \"more relevant\" simply means \"newer\".\n+sub msgid_cache_getmatches {\n+\tmy ($maxentries) = @_;\n+\t$maxentries //= 10;\n+\tmy @list = msgid_cache_read();\n+\t@list = sort {$b->{epoch} <=> $a->{epoch}} @list;\n+\t@list = @list[0 .. $maxentries-1] if (@list > $maxentries);\n+\treturn @list;\n+}\n+\n+sub msgid_cache_this {\n+\tmy $msgid = shift;\n+\tmy $subject = shift;\n+\tmy $date = shift;\n+\t# Make sure there are no tabs which will confuse us, and save\n+\t# some valuable horizontal real-estate by removing redundant\n+\t# whitespace.\n+\tif ($subject) {\n+\t\t$subject =~ s/^\\s+|\\s+$//g;\n+\t\t$subject =~ s/\\s+/ /g;\n+\t}\n+\t# Replace undef or the empty string by an actual string. Nobody uses \"0\" as the subject...\n+\t$subject ||= '(none)';\n+\t$date //= format_2822_time(time());\n+\t$date =~ s/\\s+/ /g;\n+\tpush @msgid_new_entries, {id => $msgid, subject => $subject, date => $date};\n+}\n-- \n1.8.4.rc3.2.gb900fc8\n"},{"id":"225432","messageId":"7vvc32n1vz.fsf@alter.siamese.dyndns.org","threadId":"34716","inReplyTo":"1376701126-5759-1-git-send-email-rv@rasmusvillemoes.dk","subject":"Re: [RFC] git-send-email: Cache generated message-ids, use them when prompting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-18T21:08:00Z","receivedAt":"2013-08-18T21:08:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rasmus Villemoes <rv@rasmusvillemoes.dk> writes:\n\n> This is mostly a proof of concept/RFC patch. The idea is for\n> git-send-email to store the message-ids it generates, along with the\n> Subject and Date headers of the message. When prompting for which\n> Message-ID should be used in In-Reply-To, display a list of recent\n> emails (identifed using the Date/Subject pairs; the message-ids\n> themselves are not for human consumption). Choosing from that list\n> will then use the corresponding message-id; otherwise, the behaviour\n> is as usual.\n>\n> When composing v2 or v3 of a patch or patch series, this avoids the\n> need to get one's MUA to display the Message-ID of the earlier email\n> (which is cumbersome in some MUAs) and then copy-paste that.\n>\n> If this idea is accepted, I'm certain I'll get to use the feature\n> immediately, since the patch is not quite ready for inclusion :-)\n>\n> Signed-off-by: Rasmus Villemoes <rv@rasmusvillemoes.dk>\n> ---\n>  git-send-email.perl | 101 +++++++++++++++++++++++++++++++++++++++++++++++++---\n>  1 file changed, 96 insertions(+), 5 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index f608d9b..2e3685c 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -26,6 +26,7 @@ use Data::Dumper;\n>  use Term::ANSIColor;\n>  use File::Temp qw/ tempdir tempfile /;\n>  use File::Spec::Functions qw(catfile);\n> +use Date::Parse;\n\nHmm, is this part of core that we do not have to worry about some\npeople not having it?  It appears that Git/SVN/Log.pm explicitly\navoids using it.\n\n> @@ -203,6 +204,7 @@ my ($validate, $confirm);\n>  my (@suppress_cc);\n>  my ($auto_8bit_encoding);\n>  my ($compose_encoding);\n> +my ($msgid_cache_file, $msgid_maxprompt);\n\nI do not see $msgid_maxprompt used anywhere in the new code.\n\n>  \n>  my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n>  \n> @@ -214,7 +216,7 @@ my %config_bool_settings = (\n>      \"signedoffcc\" => [\\$signed_off_by_cc, undef],      # Deprecated\n>      \"validate\" => [\\$validate, 1],\n>      \"multiedit\" => [\\$multiedit, undef],\n> -    \"annotate\" => [\\$annotate, undef]\n> +    \"annotate\" => [\\$annotate, undef],\n\nIs this related to this patch in any way?\n\n>  );\n>  \n>  my %config_settings = (\n> @@ -237,6 +239,7 @@ my %config_settings = (\n>      \"from\" => \\$sender,\n>      \"assume8bitencoding\" => \\$auto_8bit_encoding,\n>      \"composeencoding\" => \\$compose_encoding,\n> +    \"msgidcachefile\" => \\$msgid_cache_file,\n>  );\n>  \n>  my %config_path_settings = (\n> @@ -311,6 +314,7 @@ my $rc = GetOptions(\"h\" => \\$help,\n>  \t\t    \"8bit-encoding=s\" => \\$auto_8bit_encoding,\n>  \t\t    \"compose-encoding=s\" => \\$compose_encoding,\n>  \t\t    \"force\" => \\$force,\n> +\t\t    \"msgid-cache-file=s\" => \\$msgid_cache_file,\n>  \t );\n\nIs there a standard, recommended location we suggest users to store\nthis?  \n\n>  usage() if $help;\n> @@ -784,10 +788,31 @@ sub expand_one_alias {\n>  @bcclist = validate_address_list(sanitize_address_list(@bcclist));\n>  \n>  if ($thread && !defined $initial_reply_to) {\n> -\t$initial_reply_to = ask(\n> -\t\t\"Message-ID to be used as In-Reply-To for the first email (if any)? \",\n> -\t\tdefault => \"\",\n> -\t\tvalid_re => qr/\\@.*\\./, confirm_only => 1);\n> +\tmy @choices;\n> +\tif ($msgid_cache_file) {\n> +\t\t@choices = msgid_cache_getmatches();\n\nIt is a bit strange that \"filename is specified => we will find the\nlatest 10\" before seeing if we even check to see if that file\nexists.  I would have expected that a two-step \"filename is given =>\ntry to read it\" and \"if we did read something => give choices\"\nprocess would be used.\n\nAlso \"getmatches\" that does not take any clue from what the caller\nknows (the title of the series, for example) seems much less useful\nthan ideal.  The callee is not getting anything to work with to get\nsensible \"matches\".\n\n> @@ -1282,6 +1307,8 @@ X-Mailer: git-send-email $gitversion\n>  \t\t}\n>  \t}\n>  \n> +\tmsgid_cache_this($message_id, $subject, $date) if ($msgid_cache_file && !$dry_run);\n\nIs this caching each and every one, even for things like \"[PATCH 23/47]\"?\n\n> @@ -1508,6 +1535,8 @@ sub cleanup_compose_files {\n>  \n>  $smtp->quit if $smtp;\n>  \n> +msgid_cache_write() if $msgid_cache_file;\n\nIs this done under --dry-run?\n\n> @@ -1556,3 +1585,65 @@ sub body_or_subject_has_nonascii {\n>  \t}\n>  \treturn 0;\n>  }\n> +\n> +my @msgid_new_entries;\n> +\n> +# For now, use a simple tab-separated format:\n> +#\n> +#    $id\\t$date\\t$subject\\n\n> +sub msgid_cache_read {\n> +\tmy $fh;\n> +\tmy $line;\n> +\tmy @entries;\n> +\tif (not open ($fh, '<', $msgid_cache_file)) {\n> +\t\t# A non-existing cache file is ok, but should we warn if errno != ENOENT?\n\nIt should not be a warning but an informational message, \"creating a\nnew cachefile\", when bootstrapping, no?\n\n> +\twhile ($line = <$fh>) {\n> +\t\tchomp($line);\n> +\t\tmy ($id, $date, $subject) = split /\\t/, $line;\n> +\t\tmy $epoch = str2time($date);\n> +\t\tpush @entries, {id=>$id, date=>$date, epoch=>$epoch, subject=>$subject};\n> +\t}\n> +\tclose($fh);\n> +\treturn @entries;\n\nSo all the old ones are read, without dropping ancient and possibly\nuseless ones here...\n\n> +sub msgid_cache_write {\n> +\tmy $fh;\n> +\tif (not open($fh, '>>', $msgid_cache_file)) {\n> +\t    warn \"cannot open $msgid_cache_file for appending: $!\";\n> +\t    return;\n> +\t}\n> +\tprintf $fh \"%s\\t%s\\t%s\\n\", $_->{id}, $_->{date}, $_->{subject} for (@msgid_new_entries);\n> +\tclose($fh);\n\nAnd the new ones are appended to the end without expiring anything.\n\nWhen does the cache shrink, and who is responsible for doing so?\n\n> +# Return an array of cached message-ids, ordered by \"relevance\". It\n> +# might make sense to take the Subject of the new mail as an extra\n> +# argument and do some kind of fuzzy matching against the old\n> +# subjects, but for now \"more relevant\" simply means \"newer\".\n> +sub msgid_cache_getmatches {\n> +\tmy ($maxentries) = @_;\n> +\t$maxentries //= 10;\n\nThe //= operator is mentioned in perl581delta.pod, it seems, and\nnone of our Perl scripted Porcelains seems to use it yet.  Is it\nsafe to assume that everybody has it?\n\n> +\tmy @list = msgid_cache_read();\n> +\t@list = sort {$b->{epoch} <=> $a->{epoch}} @list;\n> +\t@list = @list[0 .. $maxentries-1] if (@list > $maxentries);\n> +\treturn @list;\n> +}\n> +\n> +sub msgid_cache_this {\n> +\tmy $msgid = shift;\n> +\tmy $subject = shift;\n> +\tmy $date = shift;\n> +\t# Make sure there are no tabs which will confuse us, and save\n> +\t# some valuable horizontal real-estate by removing redundant\n> +\t# whitespace.\n> +\tif ($subject) {\n> +\t\t$subject =~ s/^\\s+|\\s+$//g;\n> +\t\t$subject =~ s/\\s+/ /g;\n> +\t}\n> +\t# Replace undef or the empty string by an actual string. Nobody uses \"0\" as the subject...\n\nThat's a strange sloppiness for somebody who uses //=, no?\n\n> +\t$subject ||= '(none)';\n> +\t$date //= format_2822_time(time());\n> +\t$date =~ s/\\s+/ /g;\n> +\tpush @msgid_new_entries, {id => $msgid, subject => $subject, date => $date};\n> +}\n\nI am personally not very interested, but that only means I will not\nbe the one who will be enthusiastically rooting for inclusion; it\ndoes not mean I reject the general notion outright.\n\nBut the behaviour the posted patch seems to implement seems to fall\nfar short of how useful the general notion \"save message ID of what\nwe sent, so that we can reply to them\" could be.\n"},{"id":"225435","messageId":"20130818222359.GF64402@vauxhall.crustytoothpaste.net","threadId":"34716","inReplyTo":"7vvc32n1vz.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC] git-send-email: Cache generated message-ids, use them when prompting","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2013-08-18T22:24:00Z","receivedAt":"2013-08-18T22:24:00Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, Aug 18, 2013 at 02:08:00PM -0700, Junio C Hamano wrote:\n> Rasmus Villemoes <rv@rasmusvillemoes.dk> writes:\n> > +# Return an array of cached message-ids, ordered by \"relevance\". It\n> > +# might make sense to take the Subject of the new mail as an extra\n> > +# argument and do some kind of fuzzy matching against the old\n> > +# subjects, but for now \"more relevant\" simply means \"newer\".\n> > +sub msgid_cache_getmatches {\n> > +\tmy ($maxentries) = @_;\n> > +\t$maxentries //= 10;\n> \n> The //= operator is mentioned in perl581delta.pod, it seems, and\n> none of our Perl scripted Porcelains seems to use it yet.  Is it\n> safe to assume that everybody has it?\n\nThis operator is new in Perl 5.10.  If you want the code to work on\nRHEL/CentOS 5, you need to avoid it, since they only have 5.8 available.\n\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"225626","messageId":"1377111862-13199-1-git-send-email-rv@rasmusvillemoes.dk","threadId":"34716","inReplyTo":"1376701126-5759-1-git-send-email-rv@rasmusvillemoes.dk","subject":"[PATCH v2 0/2] git-send-email: Message-ID caching","fromName":"Rasmus Villemoes","fromEmail":"rv@rasmusvillemoes.dk","sentAt":"2013-08-21T19:04:20Z","receivedAt":"2013-08-21T19:04:20Z","isPatch":true,"sender":{"key":"rv@rasmusvillemoes.dk","avatar":"https://avatars.githubusercontent.com/u/4375908?v=4"},"body":"This is a more polished attempt at implementing the message-id\ncaching. I apologize for the sloppiness (unrelated changes, unused\nvariables etc.) in the first version.\n\nI have split the patch in two. The first half moves the choice-logic\ninside ask(): If ask() decides to return the default immediately\n(because the $term->{IN,OUT} checks fail), there's no reason to print\nall the choices. In my first attempt, I handled this by passing a huge\nprompt-string, but that made everything underlined (the prompt may be\nemphasized in some other way on other terminals). Passing a different\nvalid_re and handling a numeric return is also slightly more\ncumbersome for the caller than making ask() take care of it. There\nmight be other future uses of this 'choices' ability, but if the\nmessage-id-caching is ultimately rejected, [1/2] can of course also be\ndropped.\n\n[2/2] is the new implementation. The two main changes are that old\nentries are expunged when the file grows larger than, by default, 100\nkB, and the old emails are scored according to a simple scheme (which\nmight need improvement). The input to the scoring is {first subject in\nnew batch, old subject, was the old email first in a batch?}. [*]\n\nI also now simply store the unix time stamp instead of storing the\ncontents of the date header. This reduces processing time (no need to\nparse the date header when reading the file), and eliminates the\nDate::Parse dependency. The minor downside is that if the user has\nchanged time zone since the old email was sent, the date in the prompt\nwill not entirely match the Date:-header in the email that was sent.\n\nI had to rearrange the existing code a little to make certain global\nvariables ($time, @files) contain the right thing at the right time,\nbut hopefully I haven't broken anything in the process.\n\n[*] Suggestions for improving that heuristic are welcome. One thing\nthat might be worthwhile is to give words ending in a colon higher\nweight.\n\nRasmus Villemoes (2):\n  git-send-email: add optional 'choices' parameter to the ask sub\n  git-send-email: Cache generated message-ids, use them when prompting\n\n git-send-email.perl |  144 ++++++++++++++++++++++++++++++++++++++++++++++++---\n 1 file changed, 138 insertions(+), 6 deletions(-)\n\n\nThanks, Junio, for the comments. A few replies:\n\nJunio C Hamano <gitster@pobox.com> writes:\n\n> Rasmus Villemoes <rv@rasmusvillemoes.dk> writes:\n>>  my %config_path_settings = (\n>> @@ -311,6 +314,7 @@ my $rc = GetOptions(\"h\" => \\$help,\n>>  \t\t    \"8bit-encoding=s\" => \\$auto_8bit_encoding,\n>>  \t\t    \"compose-encoding=s\" => \\$compose_encoding,\n>>  \t\t    \"force\" => \\$force,\n>> +\t\t    \"msgid-cache-file=s\" => \\$msgid_cache_file,\n>>  \t );\n>\n> Is there a standard, recommended location we suggest users to store\n> this?  \n\nI don't know. It is obviously a local, per-repository, thing. I don't\nknow enough about git's internals to know if something breaks if one\nputs it in .git (say, \".git/msgid.cache\"). If that's a bad idea, then\nI think it should be in the root of the repository, and one would then\nprobably want to add it to .git/info/exclude.\n\nIf storing it under .git is possible, one could consider making the\noption a boolean ('msgid-use-cache' ?) and always use\n\".git/msgid.cache\".\n\n>> +\tmy @choices;\n>> +\tif ($msgid_cache_file) {\n>> +\t\t@choices = msgid_cache_getmatches();\n>\n> It is a bit strange that \"filename is specified => we will find the\n> latest 10\" before seeing if we even check to see if that file\n> exists.  I would have expected that a two-step \"filename is given =>\n> try to read it\" and \"if we did read something => give choices\"\n> process would be used.\n\nI'm not sure I understand how this is different from what I was or am\ndoing. Sure, I don't do the test for existence before calling\nmsgid_cache_getmatches(), but that will just return an empty list if\nthe file does not exist. That will end up doing the same thing as if\nno cache file was given.\n\n> Also \"getmatches\" that does not take any clue from what the caller\n> knows (the title of the series, for example) seems much less useful\n> than ideal.\n\nFixed in the new version.\n\n>> +\tmsgid_cache_this($message_id, $subject, $date) if ($msgid_cache_file && !$dry_run);\n>\n> Is this caching each and every one, even for things like \"[PATCH 23/47]\"?\n\nYes, but now I remember whether it was the first or not. \n\n>> +msgid_cache_write() if $msgid_cache_file;\n>\n> Is this done under --dry-run?\n\nWell, it was, but the msgid_cache_this() was not, so there was an\nempty list of new entries. Of course, better to be explicit and safe,\nso fixed.\n\n>> +\tif (not open ($fh, '<', $msgid_cache_file)) {\n>> +\t\t# A non-existing cache file is ok, but should we warn if errno != ENOENT?\n>\n> It should not be a warning but an informational message, \"creating a\n> new cachefile\", when bootstrapping, no?\n\nIf so, it should probably be when writing the new file. What I'm\ntrying to say is that errno == ENOENT is ok (and expected), but errno\n== EPERM or anything else might be a condition we should warn or even\ndie on.\n\n>> +\twhile ($line = <$fh>) {\n>> +\t\tchomp($line);\n>> +\t\tmy ($id, $date, $subject) = split /\\t/, $line;\n>> +\t\tmy $epoch = str2time($date);\n>> +\t\tpush @entries, {id=>$id, date=>$date, epoch=>$epoch, subject=>$subject};\n>> +\t}\n>> +\tclose($fh);\n>> +\treturn @entries;\n>\n> So all the old ones are read, without dropping ancient and possibly\n> useless ones here...\n>\n>> +sub msgid_cache_write {\n>> +\tmy $fh;\n>> +\tif (not open($fh, '>>', $msgid_cache_file)) {\n>> +\t    warn \"cannot open $msgid_cache_file for appending: $!\";\n>> +\t    return;\n>> +\t}\n>> +\tprintf $fh \"%s\\t%s\\t%s\\n\", $_->{id}, $_->{date}, $_->{subject} for (@msgid_new_entries);\n>> +\tclose($fh);\n>\n> And the new ones are appended to the end without expiring anything.\n>\n> When does the cache shrink, and who is responsible for doing so?\n\nFixed. Whether it should be \"cap when larger than $size\" or \"remove\nentries older than time()-$maxage\" I don't know.\n\n> The //= operator is mentioned in perl581delta.pod, it seems, and\n> none of our Perl scripted Porcelains seems to use it yet.  Is it\n> safe to assume that everybody has it?\n\nNo, fixed.\n\n>> +\tif ($subject) {\n>> +\t\t$subject =~ s/^\\s+|\\s+$//g;\n>> +\t\t$subject =~ s/\\s+/ /g;\n>> +\t}\n>> +\t# Replace undef or the empty string by an actual string. Nobody uses \"0\" as the subject...\n>\n> That's a strange sloppiness for somebody who uses //=, no?\n\nFixed.\n\n\n-- \n1.7.9.5\n"},{"id":"225627","messageId":"1377111862-13199-2-git-send-email-rv@rasmusvillemoes.dk","threadId":"34716","inReplyTo":"1377111862-13199-1-git-send-email-rv@rasmusvillemoes.dk","subject":"[PATCH 1/2] git-send-email: add optional 'choices' parameter to the ask sub","fromName":"Rasmus Villemoes","fromEmail":"rv@rasmusvillemoes.dk","sentAt":"2013-08-21T19:04:21Z","receivedAt":"2013-08-21T19:04:21Z","isPatch":true,"sender":{"key":"rv@rasmusvillemoes.dk","avatar":"https://avatars.githubusercontent.com/u/4375908?v=4"},"body":"Make it possible for callers of ask() to provide a list of\nchoices. Entering an appropriate integer chooses from that list,\notherwise the input is treated as usual.\n\nEach choice can either be a single string, which is used both for the\nprompt and for the return value, or a two-element array ref, where the\nzeroth element is the choice and the first is used for the prompt.\n\nSigned-off-by: Rasmus Villemoes <rv@rasmusvillemoes.dk>\n---\n git-send-email.perl |   11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 2162478..ac3b02d 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -680,11 +680,18 @@ sub ask {\n \tmy $valid_re = $arg{valid_re};\n \tmy $default = $arg{default};\n \tmy $confirm_only = $arg{confirm_only};\n+\tmy $choices = $arg{choices} || [];\n \tmy $resp;\n \tmy $i = 0;\n \treturn defined $default ? $default : undef\n \t\tunless defined $term->IN and defined fileno($term->IN) and\n \t\t       defined $term->OUT and defined fileno($term->OUT);\n+\tfor (@$choices) {\n+\t\tprintf \"(%d) %s\\n\", $i++, ref($_) eq 'ARRAY' ? $_->[1] : $_;\n+\t}\n+\tprintf \"Enter 0-%d to choose from the above list\\n\", $i-1\n+\t\tif (@$choices);\n+\t$i = 0;\n \twhile ($i++ < 10) {\n \t\t$resp = $term->readline($prompt);\n \t\tif (!defined $resp) { # EOF\n@@ -694,6 +701,10 @@ sub ask {\n \t\tif ($resp eq '' and defined $default) {\n \t\t\treturn $default;\n \t\t}\n+\t\tif (@$choices && $resp =~ m/^[0-9]+$/ && $resp < @$choices) {\n+\t\t\tmy $c = $choices->[$resp];\n+\t\t\treturn ref($c) eq 'ARRAY' ? $c->[0] : $c;\n+\t\t}\n \t\tif (!defined $valid_re or $resp =~ /$valid_re/) {\n \t\t\treturn $resp;\n \t\t}\n-- \n1.7.9.5\n"},{"id":"225628","messageId":"1377111862-13199-3-git-send-email-rv@rasmusvillemoes.dk","threadId":"34716","inReplyTo":"1377111862-13199-1-git-send-email-rv@rasmusvillemoes.dk","subject":"[PATCH 2/2] git-send-email: Cache generated message-ids, use them when prompting","fromName":"Rasmus Villemoes","fromEmail":"rv@rasmusvillemoes.dk","sentAt":"2013-08-21T19:04:22Z","receivedAt":"2013-08-21T19:04:22Z","isPatch":true,"sender":{"key":"rv@rasmusvillemoes.dk","avatar":"https://avatars.githubusercontent.com/u/4375908?v=4"},"body":"Allow the user to specify a file (sendemail.msgidcachefile) in which\nto store the message-ids generated by git-send-email, along with time\nand subject information. When prompting for a Message-ID to be used in\nIn-Reply-To, that file can be used to generate a list of options.\n\nWhen composing v2 or v3 of a patch or patch series, this avoids the\nneed to get one's MUA to display the Message-ID of the earlier email\n(which is cumbersome in some MUAs) and then copy-paste that.\n\nListing all previously sent emails is useless, so currently only the\n10 most \"relevant\" emails. \"Relevant\" is based on a simple scoring,\nwhich might need to be revised: Count the words in the old subject\nwhich also appear in the subject of the first email to be sent; add a\nbonus if the old email was first in a batch (that is, [00/74] is more\nlikely to be relevant than [43/74]). Resort to comparing timestamps\n(newer is more relevant) when the scores tie.\n\nTo limit disk usage, the oldest half of the cached entries are\nexpunged when the cache file exceeds sendemail.msgidcachemaxsize\n(default 100kB). This also ensures that we will never have to read,\nscore, and sort 1000s of entries on each invocation of git-send-email.\n\nSigned-off-by: Rasmus Villemoes <rv@rasmusvillemoes.dk>\n---\n git-send-email.perl |  133 ++++++++++++++++++++++++++++++++++++++++++++++++---\n 1 file changed, 127 insertions(+), 6 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex ac3b02d..5094267 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -203,6 +203,7 @@ my ($validate, $confirm);\n my (@suppress_cc);\n my ($auto_8bit_encoding);\n my ($compose_encoding);\n+my ($msgid_cache_file, $msgid_cache_maxsize);\n \n my ($debug_net_smtp) = 0;\t\t# Net::SMTP, see send_message()\n \n@@ -237,6 +238,8 @@ my %config_settings = (\n     \"from\" => \\$sender,\n     \"assume8bitencoding\" => \\$auto_8bit_encoding,\n     \"composeencoding\" => \\$compose_encoding,\n+    \"msgidcachefile\" => \\$msgid_cache_file,\n+    \"msgidcachemaxsize\" => \\$msgid_cache_maxsize,\n );\n \n my %config_path_settings = (\n@@ -796,11 +799,23 @@ sub expand_one_alias {\n @bcclist = expand_aliases(@bcclist);\n @bcclist = validate_address_list(sanitize_address_list(@bcclist));\n \n+if ($compose && $compose > 0) {\n+\t@files = ($compose_filename . \".final\", @files);\n+}\n+\n if ($thread && !defined $initial_reply_to && $prompting) {\n+\tmy @choices = ();\n+\tif ($msgid_cache_file) {\n+\t\tmy $first_subject = get_patch_subject($files[0]);\n+\t\t$first_subject =~ s/^GIT: //;\n+\t\t@choices = msgid_cache_getmatches($first_subject, 10);\n+\t\t@choices = map {[$_->{id}, sprintf \"[%s] %s\", format_2822_time($_->{epoch}), $_->{subject}]} @choices;\n+\t}\n \t$initial_reply_to = ask(\n \t\t\"Message-ID to be used as In-Reply-To for the first email (if any)? \",\n \t\tdefault => \"\",\n-\t\tvalid_re => qr/\\@.*\\./, confirm_only => 1);\n+\t\tvalid_re => qr/\\@.*\\./, confirm_only => 1,\n+\t\tchoices => \\@choices);\n }\n if (defined $initial_reply_to) {\n \t$initial_reply_to =~ s/^\\s*<?//;\n@@ -818,10 +833,6 @@ if (!defined $smtp_server) {\n \t$smtp_server ||= 'localhost'; # could be 127.0.0.1, too... *shrug*\n }\n \n-if ($compose && $compose > 0) {\n-\t@files = ($compose_filename . \".final\", @files);\n-}\n-\n # Variables we set as part of the loop over files\n our ($message_id, %mail, $subject, $reply_to, $references, $message,\n \t$needs_confirm, $message_num, $ask_default);\n@@ -1136,7 +1147,7 @@ sub send_message {\n \tmy $to = join (\",\\n\\t\", @recipients);\n \t@recipients = unique_email_list(@recipients,@cc,@bcclist);\n \t@recipients = (map { extract_valid_address_or_die($_) } @recipients);\n-\tmy $date = format_2822_time($time++);\n+\tmy $date = format_2822_time($time);\n \tmy $gitversion = '@@GIT_VERSION@@';\n \tif ($gitversion =~ m/..GIT_VERSION../) {\n \t    $gitversion = Git::version();\n@@ -1477,6 +1488,11 @@ foreach my $t (@files) {\n \n \tmy $message_was_sent = send_message();\n \n+\tif ($message_was_sent && $msgid_cache_file && !$dry_run) {\n+\t\tmsgid_cache_this($message_id, $message_num == 1 ? 1 : 0, , $time, $subject);\n+\t}\n+\t$time++;\n+\n \t# set up for the next message\n \tif ($thread && $message_was_sent &&\n \t\t($chain_reply_to || !defined $reply_to || length($reply_to) == 0 ||\n@@ -1521,6 +1537,8 @@ sub cleanup_compose_files {\n \n $smtp->quit if $smtp;\n \n+msgid_cache_write() if $msgid_cache_file && !$dry_run;\n+\n sub unique_email_list {\n \tmy %seen;\n \tmy @emails;\n@@ -1569,3 +1587,106 @@ sub body_or_subject_has_nonascii {\n \t}\n \treturn 0;\n }\n+\n+my @msgid_new_entries;\n+sub msgid_cache_this {\n+\tmy $msgid = shift;\n+\tmy $first = shift;\n+\tmy $epoch = shift;\n+\tmy $subject = shift;\n+\t# Make sure there are no tabs which will confuse us, and save\n+\t# some valuable horizontal real-estate by removing redundant\n+\t# whitespace.\n+\tif ($subject) {\n+\t\t$subject =~ s/^\\s+|\\s+$//g;\n+\t\t$subject =~ s/\\s+/ /g;\n+\t}\n+\t# Replace undef or the empty string by an actual string.\n+\t$subject = '(none)' if (!defined $subject || $subject eq '');\n+\n+\tpush @msgid_new_entries, {id => $msgid, first => $first, subject => $subject, epoch => $epoch};\n+}\n+\n+\n+# For now, use a simple tab-separated format:\n+#\n+#    $id\\t$wasfirst\\t$unixtime\\t$subject\\n\n+sub msgid_cache_read {\n+\tmy $fh;\n+\tmy $line;\n+\tmy @entries;\n+\tif (not open ($fh, '<', $msgid_cache_file)) {\n+\t\t# A non-existing cache file is ok, but should we warn if errno != ENOENT?\n+\t\treturn ();\n+\t}\n+\twhile ($line = <$fh>) {\n+\t\tchomp($line);\n+\t\tmy ($id, $first, $epoch, $subject) = split /\\t/, $line;\n+\t\tpush @entries, {id=>$id, first=>$first, epoch=>$epoch, subject=>$subject};\n+\t}\n+\tclose($fh);\n+\treturn @entries;\n+}\n+\n+sub msgid_cache_getmatches {\n+\tmy ($first_subject, $maxentries) = @_;\n+\tmy @list = msgid_cache_read();\n+\n+\t# We need to find the message-ids which are most likely to be\n+\t# useful. There are probably better ways to do this, but for\n+\t# now we simply count how many words in the old subject also\n+\t# appear in $first_subject.\n+\tmy %words = map {$_ => 1} msgid_subject_words($first_subject);\n+\tfor my $item (@list) {\n+\t\t# Emails which were first in a batch are more likely\n+\t\t# to be used for followups (cf. the example in \"man\n+\t\t# git-send-email\"), so give those a head start.\n+\t\tmy $score = $item->{first} ? 3 : 0;\n+\t\tfor (msgid_subject_words($item->{subject})) {\n+\t\t\t$score++ if exists $words{$_};\n+\t\t}\n+\t\t$item->{score} = $score;\n+\t}\n+\t@list = sort {$b->{score} <=> $a->{score} ||\n+\t\t      $b->{epoch} <=> $a->{epoch}} @list;\n+\t@list = @list[0 .. $maxentries-1] if (@list > $maxentries);\n+\treturn @list;\n+}\n+\n+sub msgid_subject_words {\n+\tmy $subject = shift;\n+\t# Ignore initial \"[PATCH 02/47]\"\n+\t$subject =~ s/^\\s*\\[.*?\\]//;\n+\tmy @words = split /\\s+/, $subject;\n+\t# Ignore short words. \n+\t@words = grep { length > 3 } @words;\n+\treturn @words;\n+}\n+\n+sub msgid_cache_write {\n+\tmsgid_cache_do_write(1, \\@msgid_new_entries);\n+\n+\tif (defined $msgid_cache_maxsize && $msgid_cache_maxsize =~ m/^\\s*([0-9]+)\\s*([kKmMgG]?)$/) {\n+\t\tmy %SI = ('' => 1, 'k' => 1e3, 'm' => 1e6, 'g' => 1e9);\n+\t\t$msgid_cache_maxsize = $1 * $SI{lc($2)};\n+\t}\n+\telse {\n+\t\t$msgid_cache_maxsize = 100000;\n+\t}\n+\tif (-s $msgid_cache_file > $msgid_cache_maxsize) {\n+\t\tmy @entries = msgid_cache_read();\n+\t\tsplice @entries, 0, int(@entries/2);\n+\t\tmsgid_cache_do_write(0, \\@entries);\n+\t}\n+}\n+\n+sub msgid_cache_do_write {\n+\tmy $append = shift;\n+\tmy $entries = shift;\n+\tmy $fh;\n+\tif (not open($fh, $append ? '>>' : '>', $msgid_cache_file)) {\n+\t\tdie \"cannot open $msgid_cache_file for writing: $!\";\n+\t}\n+\tprintf $fh \"%s\\t%d\\t%s\\t%s\\n\", $_->{id}, $_->{first}, $_->{epoch}, $_->{subject} for (@$entries);\n+\tclose($fh);\n+}\n-- \n1.7.9.5\n"},{"id":"225664","messageId":"xmqqbo4qshiv.fsf@gitster.dls.corp.google.com","threadId":"34716","inReplyTo":"1377111862-13199-1-git-send-email-rv@rasmusvillemoes.dk","subject":"Re: [PATCH v2 0/2] git-send-email: Message-ID caching","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-22T00:20:24Z","receivedAt":"2013-08-22T00:20:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rasmus Villemoes <rv@rasmusvillemoes.dk> writes:\n\n>> Rasmus Villemoes <rv@rasmusvillemoes.dk> writes:\n>>>  my %config_path_settings = (\n>>> @@ -311,6 +314,7 @@ my $rc = GetOptions(\"h\" => \\$help,\n>>>  \t\t    \"8bit-encoding=s\" => \\$auto_8bit_encoding,\n>>>  \t\t    \"compose-encoding=s\" => \\$compose_encoding,\n>>>  \t\t    \"force\" => \\$force,\n>>> +\t\t    \"msgid-cache-file=s\" => \\$msgid_cache_file,\n>>>  \t );\n>>\n>> Is there a standard, recommended location we suggest users to store\n>> this?  \n>\n> I don't know. It is obviously a local, per-repository, thing. I don't\n> know enough about git's internals to know if something breaks if one\n> puts it in .git (say, \".git/msgid.cache\").\n\nI think $GIT_DIR is OK, when we _know_ we are in a Git controlled\ndirectory.  \"git send-email\" can however be invoked in a random\ndirectory that is _not_ a Git controlled directory, though.\n\nIn any case, if we were to store it inside $GIT_DIR, I'd prefer to\nhave \"send-email\" somewhere in the name of the file, as there are\nother Git programs that deal with things that have \"msgid\" (notably,\n\"am\") that will not have anything to do with this file.\n\n> If storing it under .git is possible, one could consider making the\n> option a boolean ('msgid-use-cache' ?) and always use\n> \".git/msgid.cache\".\n\nAnother possibility is to have it in the output directory specified\nvia the \"format-patch -o $dir\" option.  When you are rerolling a\nseries multiple times, you will only look at the message ID from the\nprevious round; you do not even need to look at old messages in an\nunrelated topic.\n\nI could imagine that\n\n\tgit send-email $dir/0*.txt\n\ncan notice that these input files are all in the same $dir\ndirectory, check to see if $dir/message-id file exists, read it to\noffer it as the suggested initial-reply-to.  Similarly, when sending\nthe _first_ message in such an invocation, it can just write the\ngenerated message-id to that file.  Then we need no choices.  It is\nsufficient to just keep a single message-id of the first message in\nthe previous round and offer it as a possible initial-reply-to in a\nYes/No question.\n\nJust a random thought.\n"}]}