{"thread":{"id":"22521","subject":"[PATCH 0/4] gitweb: preliminary notes support","startedAt":"2010-02-04T16:18:54Z","lastAt":"2010-02-07T21:08:05Z","messageCount":47,"participants":["Giuseppe Bilotta","Junio C Hamano","Jakub Narebski","Johan Herland"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"133605","messageId":"1265300338-25021-1-git-send-email-giuseppe.bilotta@gmail.com","threadId":"22521","inReplyTo":null,"subject":"[PATCH 0/4] gitweb: preliminary notes support","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-04T16:18:54Z","receivedAt":"2010-02-04T16:18:54Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"This small patchset introduces notes support in gitweb.\n\nThe feature is designed to be powerful and flexible: any amount of notes\nref spaces can be used, and per-namespace styling is possible.\n\nThe implementation is not exactly high-performance, needing no less than\none extra git call per commit per notes ref space (two if a note exist),\nand is thus disabled by default.\n\nIt's quite likely that appropriate C plumbing in git could speed this\nconsiderably; the current implementation, OTOH, has the advantage of\nworking even when the git core does not support notes itself.\n\nGiuseppe Bilotta (4):\n gitweb: notes feature\n gitweb: show notes in shortlog view\n gitweb: show notes in log\n gitweb: show notes in commit(diff) view\n\n gitweb/gitweb.css | 51 +++++++++++++++++++++++\n gitweb/gitweb.perl | 116 +++++++++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 166 insertions(+), 1 deletions(-)\n"},{"id":"133606","messageId":"1265300338-25021-2-git-send-email-giuseppe.bilotta@gmail.com","threadId":"22521","inReplyTo":"1265300338-25021-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCH 1/4] gitweb: notes feature","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-04T16:18:55Z","receivedAt":"2010-02-04T16:18:55Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Introduce support for notes by collecting them when creating commit\nlists. The list of noterefs to look into is configurable, and can be a(n\narray of) refspec(s), which will be looked for in the refs/notes\nnamespace.\n\nThe feature is disabled by default because it's presently not very\nefficient (one extra git call per configured refspec, plus two extra git\ncalls per commit per noteref).\n---\n gitweb/gitweb.perl | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 62 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex d0c3ff2..9ba5815 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -411,6 +411,22 @@ our %feature = (\n \t\t'override' => 0,\n \t\t'default' => [16]},\n \n+\t# Notes support. When this feature is enabled, the presence of notes\n+\t# for any commit is signaled, and the note content is made available\n+\t# in a way appropriate for the current view.\n+\t# Set this to '*' to enable all notes namespace, or to a shell-glob\n+\t# specification to enable specific namespaces only.\n+\n+\t# To enable system wide have in $GITWEB_CONFIG\n+\t# $feature{'notes'}{'default'} = ['*'];\n+\t# To have project specific config enable override in $GITWEB_CONFIG\n+\t# $feature{'notes'}{'override'} = 1;\n+\t# and in project config gitweb.notes = namespace;\n+\t'notes' => {\n+\t\t'sub' => \\&feature_notes,\n+\t\t'override' => 0,\n+\t\t'default' => []},\n+\n \t# Avatar support. When this feature is enabled, views such as\n \t# shortlog or commit will display an avatar associated with\n \t# the email of the committer(s) and/or author(s).\n@@ -513,6 +529,16 @@ sub feature_patches {\n \treturn ($_[0]);\n }\n \n+sub feature_notes {\n+\tmy @val = (git_get_project_config('notes'));\n+\n+\tif (@val) {\n+\t\treturn @val;\n+\t}\n+\n+\treturn @_;\n+}\n+\n sub feature_avatar {\n \tmy @val = (git_get_project_config('avatar'));\n \n@@ -2786,10 +2812,30 @@ sub parse_commit {\n \treturn %co;\n }\n \n+# return all refs matching refs/notes/ where the globspecs\n+# are taken from the notes feature content.\n+sub get_note_refs {\n+\tmy @globs = gitweb_get_feature('notes');\n+\tmy @note_refs = ();\n+\tforeach my $glob (@globs) {\n+\t\tif (open my $fd, '-|', git_cmd(), 'for-each-ref',\n+\t\t '--format=%(refname)', \"refs/notes/$glob\") {\n+\t\t\twhile (<$fd>) {\n+\t\t\t\tchomp;\n+\t\t\t\tpush @note_refs, $_ if $_;\n+\t\t\t}\n+\t\t\tclose $fd;\n+\t\t}\n+\t}\n+\treturn @note_refs;\n+}\n+\n sub parse_commits {\n \tmy ($commit_id, $maxcount, $skip, $filename, @args) = @_;\n \tmy @cos;\n \n+\tmy @note_refs = get_note_refs();\n+\n \t$maxcount ||= 1;\n \t$skip ||= 0;\n \n@@ -2807,6 +2853,22 @@ sub parse_commits {\n \t\tor die_error(500, \"Open git-rev-list failed\");\n \twhile (my $line = <$fd>) {\n \t\tmy %co = parse_commit_text($line);\n+\t\tmy %notes = () ;\n+\t\tforeach my $note_ref (@note_refs) {\n+\t\t\tmy $obj = \"$note_ref:$co{'id'}\";\n+\t\t\tif (open my $fd, '-|', git_cmd(), 'rev-parse',\n+\t\t\t\t'--verify', '-q', $obj) {\n+\t\t\t\tmy $exists = <$fd>;\n+\t\t\t\tclose $fd;\n+\t\t\t\tif (defined $exists) {\n+\t\t\t\t\tif (open $fd, '-|', git_cmd(), 'show', $obj) {\n+\t\t\t\t\t\t$notes{$note_ref} = scalar <$fd>;\n+\t\t\t\t\t\tclose $fd;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t\t$co{'notes'} = \\%notes;\n \t\tpush @cos, \\%co;\n \t}\n \tclose $fd;\n-- \n1.7.0.rc1.193.ge8618\n"},{"id":"133607","messageId":"1265300338-25021-3-git-send-email-giuseppe.bilotta@gmail.com","threadId":"22521","inReplyTo":"1265300338-25021-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCH 2/4] gitweb: show notes in shortlog view","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-04T16:18:56Z","receivedAt":"2010-02-04T16:18:56Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"The presence of the note is shown by a small icon, hovering on which\nreveals the actual note content.\n---\n gitweb/gitweb.css | 29 +++++++++++++++++++++++++++++\n gitweb/gitweb.perl | 30 +++++++++++++++++++++++++++++-\n 2 files changed, 58 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\nindex 50067f2..7d1836b 100644\n--- a/gitweb/gitweb.css\n+++ b/gitweb/gitweb.css\n@@ -572,3 +572,32 @@ span.match {\n div.binary {\n \tfont-style: italic;\n }\n+\n+span.notes {\n+\tfloat:right;\n+\tposition:relative;\n+}\n+\n+span.notes span.note-container:before {\n+\tcontent: url('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAgAAAAIAgMAAAC5YVYYAAAAAXNSR0IArs4c6QAAAAlQTFRFAABnybuD//+t5rXizQAAAAF0Uk5TAEDm2GYAAAABYktHRACIBR1IAAAAGElEQVQI12MIDWXIWglDQHYIQ1YAQ6gDAFWPBrAKFe0fAAAAAElFTkSuQmCC');\n+}\n+\n+span.notes span.note {\n+\tdisplay:none;\n+}\n+\n+span.notes span.note-container img {\n+\tcontent: normal;\n+}\n+\n+span.notes span.note-container:hover span.note {\n+\tdisplay:block;\n+\tcontent:normal;\n+\tbackground-color:#ffffad;\n+\tborder:1px solid #c9bb83;\n+\tpadding:4px;margin:0;\n+\tposition:absolute;\n+\ttop:0;right:0;\n+\tz-index:10;\n+\toverflow:visible;\n+}\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 9ba5815..1701ed1 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1628,6 +1628,33 @@ sub format_subject_html {\n \t}\n }\n \n+# display notes next to a commit\n+sub format_notes_html {\n+\tmy %notes = %{$_[0]};\n+\tmy $ret = \"\";\n+\twhile (my ($ref, $text) = each %notes) {\n+\t\t# remove 'refs/notes/' and an optional final s\n+\t\t$ref =~ s/^refs\\/notes\\///;\n+\t\t$ref =~ s/s$//;\n+\n+\t\t# double markup is needed to allow pure CSS cross-browser 'popup'\n+\t\t# of the note\n+\t\t$ret .= \"\";\n+\t\t$ret .= \"\";\n+\t\tforeach my $line (split /\\n/, $text) {\n+\t\t\t$ret .= esc_html($line) . \"
\";\n+\t\t}\n+\t\t$ret .= \"\";\n+\t}\n+\tif ($ret) {\n+\t\treturn \"$ret\";\n+\t} else {\n+\t\treturn $ret;\n+\t}\n+\n+\n+}\n+\n # Rather than recomputing the url for an email multiple times, we cache it\n # after the first hit. This gives a visible benefit in views where the avatar\n # for the same email is used repeatedly (e.g. shortlog).\n@@ -4595,6 +4622,7 @@ sub git_shortlog_body {\n \t\tmy %co = %{$commitlist->[$i]};\n \t\tmy $commit = $co{'id'};\n \t\tmy $ref = format_ref_marker($refs, $commit);\n+\t\tmy $notes = format_notes_html($co{'notes'});\n \t\tif ($alternate) {\n \t\t\tprint \"\\n\";\n \t\t} else {\n@@ -4605,7 +4633,7 @@ sub git_shortlog_body {\n \t\tprint \"$co{'age_string_date'}\\n\" .\n \t\t format_author_html('td', \\%co, 10) . \"\";\n \t\tprint format_subject_html($co{'title'}, $co{'title_short'},\n-\t\t href(action=>\"commit\", hash=>$commit), $ref);\n+\t\t href(action=>\"commit\", hash=>$commit), $ref . $notes);\n \t\tprint \"\\n\" .\n \t\t \"\" .\n \t\t $cgi->a({-href => href(action=>\"commit\", hash=>$commit)}, \"commit\") . \" | \" .\n-- \n1.7.0.rc1.193.ge8618\n"},{"id":"133608","messageId":"1265300338-25021-4-git-send-email-giuseppe.bilotta@gmail.com","threadId":"22521","inReplyTo":"1265300338-25021-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCH 3/4] gitweb: show notes in log","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-04T16:18:57Z","receivedAt":"2010-02-04T16:18:57Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"The notes are shown in full to the left of the log message.\n---\n gitweb/gitweb.css | 11 +++++++++++\n gitweb/gitweb.perl | 11 +++++++----\n 2 files changed, 18 insertions(+), 4 deletions(-)\n\ndiff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\nindex 7d1836b..81d66d3 100644\n--- a/gitweb/gitweb.css\n+++ b/gitweb/gitweb.css\n@@ -601,3 +601,14 @@ span.notes span.note-container:hover span.note {\n \tz-index:10;\n \toverflow:visible;\n }\n+\n+div.notes {\n+\tmax-width:150px;\n+\tfloat:left;\n+}\n+\n+div.notes div.note {\n+\tbackground-color:#ffffad;\n+\tborder:1px solid #c9bb83;\n+\tpadding:4px;margin:0;\n+}\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 1701ed1..0d0877e 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -1631,6 +1631,7 @@ sub format_subject_html {\n # display notes next to a commit\n sub format_notes_html {\n \tmy %notes = %{$_[0]};\n+\tmy $tag = $_[1] || 'span' ;\n \tmy $ret = \"\";\n \twhile (my ($ref, $text) = each %notes) {\n \t\t# remove 'refs/notes/' and an optional final s\n@@ -1639,15 +1640,15 @@ sub format_notes_html {\n \n \t\t# double markup is needed to allow pure CSS cross-browser 'popup'\n \t\t# of the note\n-\t\t$ret .= \"\";\n-\t\t$ret .= \"\";\n+\t\t$ret .= \"<$tag title='$ref' class='note-container $ref'>\";\n+\t\t$ret .= \"<$tag title='$ref' class='note $ref'>\";\n \t\tforeach my $line (split /\\n/, $text) {\n \t\t\t$ret .= esc_html($line) . \"
\";\n \t\t}\n-\t\t$ret .= \"\";\n+\t\t$ret .= \"\";\n \t}\n \tif ($ret) {\n-\t\treturn \"$ret\";\n+\t\treturn \"<$tag class='notes'>$ret\";\n \t} else {\n \t\treturn $ret;\n \t}\n@@ -4581,6 +4582,7 @@ sub git_log_body {\n \t\tnext if !%co;\n \t\tmy $commit = $co{'id'};\n \t\tmy $ref = format_ref_marker($refs, $commit);\n+\t\tmy $notes = format_notes_html($co{'notes'}, 'div');\n \t\tmy %ad = parse_date($co{'author_epoch'});\n \t\tgit_print_header_div('commit',\n \t\t \"$co{'age_string'}\" .\n@@ -4598,6 +4600,7 @@ sub git_log_body {\n \t\t git_print_authorship(\\%co, -tag => 'span');\n \t\t print \"
\\n\\n\";\n \n+\t\tprint \"$notes\\n\";\n \t\tprint \"
\\n\";\n \t\tgit_print_log($co{'comment'}, -final_empty_line=> 1);\n \t\tprint \"
\\n\";\n-- \n1.7.0.rc1.193.ge8618\n"},{"id":"133609","messageId":"1265300338-25021-5-git-send-email-giuseppe.bilotta@gmail.com","threadId":"22521","inReplyTo":"1265300338-25021-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCH 4/4] gitweb: show notes in commit(diff) view","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-04T16:18:58Z","receivedAt":"2010-02-04T16:18:58Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"The notes are shown side-by-side along the bottom of the commit message.\n---\n gitweb/gitweb.css | 11 +++++++++++\n gitweb/gitweb.perl | 21 +++++++++++++++++++++\n 2 files changed, 32 insertions(+), 0 deletions(-)\n\ndiff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\nindex 81d66d3..10acab4 100644\n--- a/gitweb/gitweb.css\n+++ b/gitweb/gitweb.css\n@@ -145,6 +145,7 @@ div.list_head {\n \tborder: solid #d9d8d1;\n \tborder-width: 1px 0px 0px;\n \tfont-style: italic;\n+\tclear: both;\n }\n \n .author_date, .author {\n@@ -612,3 +613,13 @@ div.notes div.note {\n \tborder:1px solid #c9bb83;\n \tpadding:4px;margin:0;\n }\n+\n+\n+div.page_body div.notes {\n+\tmax-width:100%;\n+\tfloat:none;\n+}\n+\n+div.page_body div.notes div.note {\n+\tfloat:left;\n+}\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 0d0877e..0d03026 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2837,12 +2837,31 @@ sub parse_commit {\n \t%co = parse_commit_text(<$fd>, 1);\n \tclose $fd;\n \n+\tmy %notes = ();\n+\tforeach my $note_ref (get_note_refs()) {\n+\t\tmy $obj = \"$note_ref:$co{'id'}\";\n+\t\tif (open my $fd, '-|', git_cmd(), 'rev-parse',\n+\t\t\t'--verify', '-q', $obj) {\n+\t\t\tmy $exists = <$fd>;\n+\t\t\tclose $fd;\n+\t\t\tif (defined $exists) {\n+\t\t\t\tif (open $fd, '-|', git_cmd(), 'show', $obj) {\n+\t\t\t\t\t$notes{$note_ref} = scalar <$fd>;\n+\t\t\t\t\tclose $fd;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}\n+\t$co{'notes'} = \\%notes;\n+\n \treturn %co;\n }\n \n # return all refs matching refs/notes/ where the globspecs\n # are taken from the notes feature content.\n sub get_note_refs {\n+\tlocal $/ = \"\";\n+\n \tmy @globs = gitweb_get_feature('notes');\n \tmy @note_refs = ();\n \tforeach my $glob (@globs) {\n@@ -5875,6 +5894,7 @@ sub git_commit {\n \n \tprint \"
\\n\";\n \tgit_print_log($co{'comment'});\n+\tprint format_notes_html($co{'notes'}, 'div');\n \tprint \"
\\n\";\n \n \tgit_difftree_body(\\@difftree, $hash, @$parents);\n@@ -6230,6 +6250,7 @@ sub git_commitdiff {\n \t\t\tgit_print_log($co{'comment'}, -final_empty_line=> 1, -remove_title => 1);\n \t\t\tprint \"\\n\"; # class=\"log\"\n \t\t}\n+\t\tprint format_notes_html($co{'notes'}, 'div');\n \n \t} elsif ($format eq 'plain') {\n \t\tmy $refs = git_get_references(\"tags\");\n-- \n1.7.0.rc1.193.ge8618\n"},{"id":"133611","messageId":"7vpr4lhsfu.fsf@alter.siamese.dyndns.org","threadId":"22521","inReplyTo":"1265300338-25021-2-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-04T16:33:09Z","receivedAt":"2010-02-04T16:33:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Giuseppe Bilotta writes:\n\n> +\t\tmy %notes = () ;\n> +\t\tforeach my $note_ref (@note_refs) {\n> +\t\t\tmy $obj = \"$note_ref:$co{'id'}\";\n\nI think this look-up is wrong (meaning: will stop working anytime in the\nfuture, and needs to be rewritten).\n\nOther parts of this patch looked Ok from a cursory reading, though.\n"},{"id":"133614","messageId":"7v4olxhrti.fsf@alter.siamese.dyndns.org","threadId":"22521","inReplyTo":"7vpr4lhsfu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-04T16:46:33Z","receivedAt":"2010-02-04T16:46:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Giuseppe Bilotta writes:\n>\n>> +\t\tmy %notes = () ;\n>> +\t\tforeach my $note_ref (@note_refs) {\n>> +\t\t\tmy $obj = \"$note_ref:$co{'id'}\";\n>\n> I think this look-up is wrong (meaning: will stop working anytime in the\n> future, and needs to be rewritten).\n\nIOW, the code should be reading output from:\n\n GIT_NOTES_REF=$note_ref git show -s --format=%N $co{'id'}\n\nas the notes tree may not be storing notes in a flat one-level namespace\nlike you are assuming.\n"},{"id":"133621","messageId":"201002041821.22864.jnareb@gmail.com","threadId":"22521","inReplyTo":"7v4olxhrti.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-04T17:21:21Z","receivedAt":"2010-02-04T17:21:21Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 4 Feb 2010, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n>> Giuseppe Bilotta writes:\n>>\n>>> +\t\tmy %notes = () ;\n>>> +\t\tforeach my $note_ref (@note_refs) {\n>>> +\t\t\tmy $obj = \"$note_ref:$co{'id'}\";\n>>\n>> I think this look-up is wrong (meaning: will stop working anytime in the\n>> future, and needs to be rewritten).\n> \n> IOW, the code should be reading output from:\n> \n> GIT_NOTES_REF=$note_ref git show -s --format=%N $co{'id'}\n> \n> as the notes tree may not be storing notes in a flat one-level namespace\n> like you are assuming.\n\nFirst, for some mechanism of deployment (IIRC Apache's mod_perl) changes\nto environment variables from CGI script are not passed to invoked\ncommands (I guess for security reasons). That is why gitweb uses --git-dir\nparameter to git wrapper, and not GIT_DIR environment variable since\n25691fb (gitweb: Use --git-dir parameter instead of setting $ENV{'GIT_DIR'},\n2006-08-28). So for proper support we would need --notes-ref (or similar)\noption to git wrapper\n\n git --notes-ref=$note_ref show -s --format=%N $co{'id'}\n\n\nSecond, parse_commit / parse_commits use\n\n git rev-list -z --parents --header --max-count-X\n\nIf this command automatically shows notes (or it can be modified to\nautomatically show notes) after unindented \"Notes:\" line (as per\ngit-notes documentation), then the only thing that needs to be\nchanged to fill %commit{'notes'} is parse_commit_text subroutine.\n\nThere would be no need for extra subroutine (and extra command, or\neven up to two extra commands per note) to get notes data.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"133637","messageId":"cb7bb73a1002041208q54ff1f57y3202e88ae2f5f44e@mail.gmail.com","threadId":"22521","inReplyTo":"201002041821.22864.jnareb@gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-04T20:08:04Z","receivedAt":"2010-02-04T20:08:04Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Thu, Feb 4, 2010 at 6:21 PM, Jakub Narebski wrote:\n> On Thu, 4 Feb 2010, Junio C Hamano wrote:\n>> Junio C Hamano writes:\n>>\n>>> Giuseppe Bilotta writes:\n>>>\n>>>> +           my %notes = () ;\n>>>> +           foreach my $note_ref (@note_refs) {\n>>>> +                   my $obj = \"$note_ref:$co{'id'}\";\n>>>\n>>> I think this look-up is wrong (meaning: will stop working anytime in the\n>>> future, and needs to be rewritten).\n>>\n>> IOW, the code should be reading output from:\n>>\n>>     GIT_NOTES_REF=$note_ref git show -s --format=%N $co{'id'}\n>>\n>> as the notes tree may not be storing notes in a flat one-level namespace\n>> like you are assuming.\n>\n> First, for some mechanism of deployment (IIRC Apache's mod_perl) changes\n> to environment variables from CGI script are not passed to invoked\n> commands (I guess for security reasons).  That is why gitweb uses --git-dir\n> parameter to git wrapper, and not GIT_DIR environment variable since\n> 25691fb (gitweb: Use --git-dir parameter instead of setting $ENV{'GIT_DIR'},\n> 2006-08-28).  So for proper support we would need --notes-ref (or similar)\n> option to git wrapper\n>\n>  git --notes-ref=$note_ref show -s --format=%N $co{'id'}\n\nAs I mentioned on the cover letter, I was hoping to be able to make\nsomething that could be deployable without requiring core changes and\nthus a specific minimum git version. I do realize however that this is\ninherently not robust (unless the code is updated if and when the\nnotes storage mechanism changes).\n\nThe wrapper is probably needed anyway, I would say, so it makes sense\nto implement it. However, see below for a caveat about its\nimplementation.\n\n> Second, parse_commit / parse_commits use\n>\n>  git rev-list -z --parents --header --max-count-X\n>\n> If this command automatically shows notes (or it can be modified to\n> automatically show notes) after unindented \"Notes:\" line (as per\n> git-notes documentation), then the only thing that needs to be\n> changed to fill %commit{'notes'} is parse_commit_text subroutine.\n>\n> There would be no need for extra subroutine (and extra command, or\n> even up to two extra commands per note) to get notes data.\n\nBut unless the --notes-ref is made to accept multiple refspecs, this\nwill only access _one_ note namespace. It is not hard to envision\nsituations where multiple namespaces are used for the same repo (e.g.\none to store git-svn metadata, one for bugzilla-related entries), in\nwhich case you'd want _all_ of them to be accessible in gitweb.\n\nAnd if we do support multiple refspecs for --notes-ref, we also need\nsome way to identify which note belongs to which namespace, of course,\nin any of the output formats that include notes.\n\nAn alternative approach would be some git notes-list command that\naccepts both multiple namespaces and multiple commits, and is\nspecifically dedicated to display notes-commits relationships\n(together with notes content, obviously)\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133638","messageId":"7vaavo90ic.fsf@alter.siamese.dyndns.org","threadId":"22521","inReplyTo":"cb7bb73a1002041208q54ff1f57y3202e88ae2f5f44e@mail.gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-04T21:03:39Z","receivedAt":"2010-02-04T21:03:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Giuseppe Bilotta writes:\n\n> As I mentioned on the cover letter, I was hoping to be able to make\n> something that could be deployable without requiring core changes and\n> thus a specific minimum git version. I do realize however that this is\n> inherently not robust (unless the code is updated if and when the\n> notes storage mechanism changes).\n\nAFAIU, the note code on the core side already creates a fan-out structure\nwhen notes tree gets large (see recent \"What's cooking\"; the series is\nparked in 'pu' but that is primarily because we are in feature freeze); it\nis not just \"inherently not robust\" but is much closer to \"broken from day\none\" ;-). Otherwise I wouldn't have wasted time to point it out.\n\nYour code is a very good proof-of-concept, though.\n\nRegarding support of multiple notes hierarchies, listing, etc.\n\nSee for example:\n\n http://thread.gmane.org/gmane.comp.version-control.git/138079/focus=138128\n\nI expect more ideas from needs by end-user would come, as we gain\nexperience with using notes in real projects. You will certainly find\nsome other needs of your own, like the \"not an environment but a command\nline option\" which Jakub mentioned, and \"multiple hierarchies\" like both\nyou and I found need for. Share them and let us together make the notes\nmechanism nicer to use.\n\nThanks.\n"},{"id":"133639","messageId":"7vwrys7lrm.fsf@alter.siamese.dyndns.org","threadId":"22521","inReplyTo":"201002041821.22864.jnareb@gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-04T21:07:25Z","receivedAt":"2010-02-04T21:07:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n>> IOW, the code should be reading output from:\n>> \n>> GIT_NOTES_REF=$note_ref git show -s --format=%N $co{'id'}\n>> \n>> as the notes tree may not be storing notes in a flat one-level namespace\n>> like you are assuming.\n>\n> First, for some mechanism of deployment (IIRC Apache's mod_perl) changes\n> to environment variables from CGI script are not passed to invoked\n> commands (I guess for security reasons).\n\nI do not believe you are unable to spawn\n\n\topen $fd, '-|' 'sh', '-c', \"GIT_NOTES_REF=$note_ref git show ...\" \n\nand read from it ;-).\n\nFor possible enhancement to make notes easier to use, see the other\nresponse.\n"},{"id":"133645","messageId":"201002050020.56643.jnareb@gmail.com","threadId":"22521","inReplyTo":"7vwrys7lrm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-04T23:20:53Z","receivedAt":"2010-02-04T23:20:53Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Dnia czwartek 4. lutego 2010 22:07, Junio C Hamano napisał:\n> Jakub Narebski writes:\n> \n>>> IOW, the code should be reading output from:\n>>> \n>>> GIT_NOTES_REF=$note_ref git show -s --format=%N $co{'id'}\n>>> \n>>> as the notes tree may not be storing notes in a flat one-level namespace\n>>> like you are assuming.\n>>\n>> First, for some mechanism of deployment (IIRC Apache's mod_perl) changes\n>> to environment variables from CGI script are not passed to invoked\n>> commands (I guess for security reasons).\n> \n> I do not believe you are unable to spawn\n> \n> \topen $fd, '-|' 'sh', '-c', \"GIT_NOTES_REF=$note_ref git show ...\" \n> \n> and read from it ;-).\n\nYou meant here\n\n\tmy $git_command = quote_command(git_cmd(), 'show', ...);\n\topen my $fd, '-|', 'sh', '-c', \"GIT_NOTES_REF=$note_ref $git_command\" \n\nSo you need to take care to quote parameters ($note_ref fortunately \ndoesn't need to be quoted)... and you have one more process (shell)\nspawned.\n\n\nTherefore I think it would be nice to have --notes-ref option to git \nwrapper,... especially that it should be easy to set it up in such way\nthat it would be possible to pass --notes-ref multiple times, e.g.:\n\n\tgit --notes-ref=commits --notes-ref=git-svn show ...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"133647","messageId":"cb7bb73a1002041538m64c6a6b3p5ee8bbaf0d78457@mail.gmail.com","threadId":"22521","inReplyTo":"7vaavo90ic.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-04T23:38:51Z","receivedAt":"2010-02-04T23:38:51Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Thu, Feb 4, 2010 at 10:03 PM, Junio C Hamano wrote:\n>\n> AFAIU, the note code on the core side already creates a fan-out structure\n> when notes tree gets large (see recent \"What's cooking\"; the series is\n> parked in 'pu' but that is primarily because we are in feature freeze); it\n> is not just \"inherently not robust\" but is much closer to \"broken from day\n> one\" ;-).  Otherwise I wouldn't have wasted time to point it out.\n\nOuch, I hadn't considered that, indeed.\n\n> Your code is a very good proof-of-concept, though.\n\nThanks. I guess at this point a proper implementation can wait for the\nnecessary core support functions.\n\n> Regarding support of multiple notes hierarchies, listing, etc.\n>\n> See for example:\n>\n>  http://thread.gmane.org/gmane.comp.version-control.git/138079/focus=138128\n>\n> I expect more ideas from needs by end-user would come, as we gain\n> experience with using notes in real projects.  You will certainly find\n> some other needs of your own, like the \"not an environment but a command\n> line option\" which Jakub mentioned, and \"multiple hierarchies\" like both\n> you and I found need for.  Share them and let us together make the notes\n> mechanism nicer to use.\n\nCollecting those ideas together would also help define some sort of\nroadmap, or at least have a clear idea of what's needed, to help drive\nthe design of the features themselves. Maybe we could start a TODO\npage on the wiki collecting these ideas?\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133650","messageId":"201002050144.50538.jnareb@gmail.com","threadId":"22521","inReplyTo":"201002041821.22864.jnareb@gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-05T00:44:49Z","receivedAt":"2010-02-05T00:44:49Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jakub Narebski wrote:\n> On Thu, 4 Feb 2010, Junio C Hamano wrote:\n>> Junio C Hamano writes:\n>> \n>>> Giuseppe Bilotta writes:\n>>>\n>>>> +\t\tmy %notes = () ;\n>>>> +\t\tforeach my $note_ref (@note_refs) {\n>>>> +\t\t\tmy $obj = \"$note_ref:$co{'id'}\";\n[...]\n \n> Second, parse_commit / parse_commits use\n> \n> git rev-list -z --parents --header --max-count-X\n> \n> If this command automatically shows notes (or it can be modified to\n> automatically show notes) after unindented \"Notes:\" line (as per\n> git-notes documentation), then the only thing that needs to be\n> changed to fill %commit{'notes'} is parse_commit_text subroutine.\n\nThis command automatically shows notes, even in absence of GIT_NOTES_REF\nenvironment variable or core.notesRef (if core.notesRef is not unset),\nso unless we want gitweb to display notes flattened into commit message\n(which I think was intended behavior - design decision for notes \ndisplay), we would need to modify parse_commit_text to gather notes \ninto %commin{'notes'} (or something).\n\n \n> There would be no need for extra subroutine (and extra command, or\n> even up to two extra commands per note) to get notes data.\n\nYou are right that we would need it if we want to display notes from\nnon-default namespace.\n\nStill, 1 or 2 git commands per commit is a bit too much (with shortlog\ndisplaying 100 commits per page): that is what \"git cat-file --batch\"\nwas invented ;-)\n\n-- \nJakub Narebski\nPoland\n"},{"id":"133652","messageId":"7vaavosdqq.fsf@alter.siamese.dyndns.org","threadId":"22521","inReplyTo":"201002050144.50538.jnareb@gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-05T00:55:09Z","receivedAt":"2010-02-05T00:55:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> Still, 1 or 2 git commands per commit is a bit too much (with shortlog\n> displaying 100 commits per page)\n\nBut who said you need to display notes for all commits by default?\n\nIt could be a thing that you would get _on demand_, just like patch text\nor diffstat is given only on demand by going to the commitdiff page.\n"},{"id":"133673","messageId":"cb7bb73a1002050042w20c8ca58hfac4283bee0a3676@mail.gmail.com","threadId":"22521","inReplyTo":"7vaavosdqq.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-05T08:42:22Z","receivedAt":"2010-02-05T08:42:22Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Fri, Feb 5, 2010 at 1:55 AM, Junio C Hamano wrote:\n> Jakub Narebski writes:\n>\n>> Still, 1 or 2 git commands per commit is a bit too much (with shortlog\n>> displaying 100 commits per page)\n>\n> But who said you need to display notes for all commits by default?\n>\n> It could be a thing that you would get _on demand_, just like patch text\n> or diffstat is given only on demand by going to the commitdiff page.\n\nBut while diffstat or patch text always make sense for _any_ commit,\nsince you always have them; notes don't, in the sense that you might\nnot have notes in most of the commits, and you might actually be\nlooking for commits with notes.\n\nAt the very least, a view like shortlog should give an indicator that\na commit has notes, and possible the namespaces where its notes are\n(e.g., you're only browsing for bugzilla notes).\n\nAnyway, since git cat-file --batch, or --batch-check if we only want\nthe indicator, (didn't know about these options, btw, thanks a bunch)\nmeans that we can get all the notes with a single extra call, what's\nthe problem with displaying them? ;-)\n\n\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133676","messageId":"201002051136.43738.johan@herland.net","threadId":"22521","inReplyTo":"cb7bb73a1002041538m64c6a6b3p5ee8bbaf0d78457@mail.gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2010-02-05T10:36:43Z","receivedAt":"2010-02-05T10:36:43Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Friday 05 February 2010, Giuseppe Bilotta wrote:\n> On Thu, Feb 4, 2010 at 10:03 PM, Junio C Hamano wrote:\n> > I expect more ideas from needs by end-user would come, as we gain\n> > experience with using notes in real projects. You will certainly find\n> > some other needs of your own, like the \"not an environment but a\n> > command line option\" which Jakub mentioned, and \"multiple hierarchies\"\n> > like both you and I found need for. Share them and let us together\n> > make the notes mechanism nicer to use.\n> \n> Collecting those ideas together would also help define some sort of\n> roadmap, or at least have a clear idea of what's needed, to help drive\n> the design of the features themselves. Maybe we could start a TODO\n> page on the wiki collecting these ideas?\n\nI already maintain a TODO list at the end of the cover letter to the notes \nseries. Here is a preview of it (I plan to send the next iteration of \njh/notes as soon as v1.7.0 is released):\n\n\n- Suggestion by Matthieu Moy and Sverre Rabbelier:\n Add notes support to git-format-patch, where note contents in\n refs/notes/format-patch are added to the \"comments section\"\n (i.e. following the '---' separator) of generated patches.\n\n- Better integration with rebase/amend/cherry-pick. Optionally bring\n notes across a commit rewrite. Controlled by command-line options\n and/or config variables. Add \"git notes move\" and \"git notes copy\"\n to suit. Junio says:\n I used to fix minor issues (styles, decl-after-stmt, etc.) using\n rebase-i long after running \"am\" in bulk, but these days I find\n myself going back to my \"inbox\" and fix them in MUA; this is\n only because I know these notes do not propagate across rebases\n and amends -- adjusting the workflow to the tool's limitation is\n not very good.\n\n- Junio says:\n The interface to tell tools to use which notes ref to use should be\n able to say \"these refs\", not just \"this ref\" i.e. GIT_NOTES_REF=a:b\n just like PATH=a:b:c...); I am fairly certain that we would want to\n store different kind of information in separate notes trees and\n aggregate them, as we gain experience with notes.\n\n- Junio says:\n There should be an interface to tell tools to use which notes refs via\n command line options; \"!alias\" does not TAB-complete, and \"git lgm\"\n above doesn't, either. \"git log --notes=notes/amlog --notes=notes/other\"\n would probably be the way to go.\n\n- Add a \"git notes grep\" subcommand: Junio says:\n While reviewing the \"inbox\", I sometimes wonder if I applied a message\n to somewhere already, but there is no obvious way to grep in the notes\n tree and get the object name that a note is attached to. Of course I\n know I can \"git grep -c johan@herland.net notes/amlog\" and it will give\n me something like:\n\n notes/amlog:65807ee697a28cb30b8ad38ebb8b84cebd3f255d:1\n notes/amlog:c789176020d6a008821e01af8b65f28abc138d4b:1\n\n but this won't scale and needs scripting to mechanize, once we start\n rebalancing the notes tree with different fan-outs. The end user (me\n in this case) is interested in \"set of objects that match this grep\n criteria\", not \"the pathnames the notes tree's implementation happens\n to use to store notes for them in the hierarchy\".\n\n- Handle note objects that are not blobs, but trees\n\n(- Rewrite fast-import notes code to use new notes API with non-note \nsupport)\n\n\nHave fun! :)\n\n...Johan\n\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"133688","messageId":"7veikzfysv.fsf@alter.siamese.dyndns.org","threadId":"22521","inReplyTo":"201002051136.43738.johan@herland.net","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-05T16:10:56Z","receivedAt":"2010-02-05T16:10:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johan Herland writes:\n\n> I already maintain a TODO list at the end of the cover letter to the notes \n> series. Here is a preview of it (I plan to send the next iteration of \n> jh/notes as soon as v1.7.0 is released):\n> ...\n\nRandom additional thoughts.\n\n* Futureproofing\n\nWe have to admit that our notes support, especially on the output side, is\nstill in its infancy. We may want to advertise it as such -- highly\nexperimental and subject to change.\n\n* format-patch\n\nTo add notes to format-patch output, we might want to do something like:\n\n $ git format-patch --notes-ref=commits --notes-ref=amlog -1\n\nand produce:\n\n From 8bff7c5383ed833bd1df9c8d85c00a27af3e5b02 Mon Sep 17 00:00:00 2001\n From: Andrew Myrick \n Date: Sat, 30 Jan 2010 03:14:22 +0000\n Subject: [PATCH] git-svn: persistent memoization\n X-Notes: pulled on Fri Feb 5 07:36:12 2010 -0800\n from git://git.bogomips.org/git-svn.git/\n X-Notes-amlog: <1264821262-28322-1-git-send-email-amyrick@apple.com>\n\n Make memoization of the svn:mergeinfo processing functions persistent with\n ...\n\nPoints to notice:\n\n - There is no point forcing users to spell \"--notes-ref\" parameter\n starting from refs/notes/; we should DWIM if they are missing;\n\n - We would want to allow more than one notes hierarchy specified. This\n would affect format_note() function---take list of struct notes_tree,\n perhaps;\n\n - Allow callers of tell format_note() to add the name of the notes\n hierarchy the note came from (or just always add it if it is not the\n default \"refs/notes/commits\").\n\n - For format-patch that produces a mbox output, the email header part may\n be a better place to put notes (obeying the usual \"indent by one space\n to continue the line\" convention).\n\n* \"log --format=%N\" and \"log --show-notes\"\n\nCurrently %N expands to the hardcoded \"log --show-notes\" default format.\nWe can probably keep it that way. When the user asked for a non default\nnotes hierarchy (i.e. other than refs/notes/commits), we may want to\nadjust \"Notes:\" string to use \"Notes-%s:\" to show which hierarchy it came\nfrom, and concatenate them together.\n\nFor \"log --show-notes\" output, we also might want to move the notes to the\nheader part like I illustrated above in format-patch output, instead of\n\"start with unindented Notes: and indented body at the end\". I.e. instead\nof showing this:\n\n $ git log --notes-ref=amlog -1 4d0cc22\n commit 4d0cc2243778b38c3759c6a08f4f1ed64155a070\n Author: Junio C Hamano \n Date: Thu Feb 4 11:10:44 2010 -0800\n\n fast-import: count --max-pack-size in bytes\n\n Similar in spirit to 07cf0f2 (make --max-pack-size argument to 'git\n ...\n Acked-by: Nicolas Pitre \n\n Notes:\n <7v4olwbyvf.fsf_-_@alter.siamese.dyndns.org>\n\n\nshow it like this:\n\n $ git log --notes-ref=amlog -1 4d0cc22\n commit 4d0cc2243778b38c3759c6a08f4f1ed64155a070\n Author: Junio C Hamano \n Date: Thu Feb 4 11:10:44 2010 -0800\n Notes-amlog: <7v4olwbyvf.fsf_-_@alter.siamese.dyndns.org>\n\n fast-import: count --max-pack-size in bytes\n\n Similar in spirit to 07cf0f2 (make --max-pack-size argument to 'git\n ...\n Acked-by: Nicolas Pitre \n"},{"id":"133731","messageId":"cb7bb73a1002051331q75429a4ey442fe8382523d2e8@mail.gmail.com","threadId":"22521","inReplyTo":"7veikzfysv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-05T21:31:14Z","receivedAt":"2010-02-05T21:31:14Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Fri, Feb 5, 2010 at 5:10 PM, Junio C Hamano wrote:\n> * format-patch\n>\n> To add notes to format-patch output, we might want to do something like:\n>\n>    $ git format-patch --notes-ref=commits --notes-ref=amlog -1\n>\n> and produce:\n>\n>    From 8bff7c5383ed833bd1df9c8d85c00a27af3e5b02 Mon Sep 17 00:00:00 2001\n>    From: Andrew Myrick \n>    Date: Sat, 30 Jan 2010 03:14:22 +0000\n>    Subject: [PATCH] git-svn: persistent memoization\n>    X-Notes: pulled on Fri Feb 5 07:36:12 2010 -0800\n>     from git://git.bogomips.org/git-svn.git/\n>    X-Notes-amlog: <1264821262-28322-1-git-send-email-amyrick@apple.com>\n\n+1 for header style note exporting. X-Git-Notes might be better.\n\n> Points to notice:\n>\n>  - There is no point forcing users to spell \"--notes-ref\" parameter\n>   starting from refs/notes/; we should DWIM if they are missing;\n\nAgreed.\n\n>  - We would want to allow more than one notes hierarchy specified. This\n>   would affect format_note() function---take list of struct notes_tree,\n>   perhaps;\n\nI get the impression everybody wants this ;-)\n\n>  - Allow callers of tell format_note() to add the name of the notes\n>   hierarchy the note came from (or just always add it if it is not the\n>   default \"refs/notes/commits\").\n\nThis is probably the most bothering issue: find, for each output\nformat that involves notes, the smart way of also outputting the\nnamespace it came from.\n\n>  - For format-patch that produces a mbox output, the email header part may\n>   be a better place to put notes (obeying the usual \"indent by one space\n>   to continue the line\" convention).\n\nHow would you cope with multi-line notes? One X-Git-Notes header per line?\n\n> * \"log --format=%N\" and \"log --show-notes\"\n>\n> Currently %N expands to the hardcoded \"log --show-notes\" default format.\n> We can probably keep it that way.  When the user asked for a non default\n> notes hierarchy (i.e. other than refs/notes/commits), we may want to\n> adjust \"Notes:\" string to use \"Notes-%s:\" to show which hierarchy it came\n> from, and concatenate them together.\n\nWe might want to do without the dash in standard log output: Notes:\nand Notes :\n\n> For \"log --show-notes\" output, we also might want to move the notes to the\n> header part like I illustrated above in format-patch output, instead of\n> \"start with unindented Notes: and indented body at the end\".  I.e. instead\n> of showing this:\n>\n>    $ git log --notes-ref=amlog -1 4d0cc22\n>    commit 4d0cc2243778b38c3759c6a08f4f1ed64155a070\n>    Author: Junio C Hamano \n>    Date:   Thu Feb 4 11:10:44 2010 -0800\n>\n>        fast-import: count --max-pack-size in bytes\n>\n>        Similar in spirit to 07cf0f2 (make --max-pack-size argument to 'git\n>        ...\n>        Acked-by: Nicolas Pitre \n>\n>    Notes:\n>        <7v4olwbyvf.fsf_-_@alter.siamese.dyndns.org>\n>\n>\n> show it like this:\n>\n>    $ git log --notes-ref=amlog -1 4d0cc22\n>    commit 4d0cc2243778b38c3759c6a08f4f1ed64155a070\n>    Author: Junio C Hamano \n>    Date:   Thu Feb 4 11:10:44 2010 -0800\n>    Notes-amlog: <7v4olwbyvf.fsf_-_@alter.siamese.dyndns.org>\n>\n>        fast-import: count --max-pack-size in bytes\n>\n>        Similar in spirit to 07cf0f2 (make --max-pack-size argument to 'git\n>        ...\n>        Acked-by: Nicolas Pitre \n\nThe footer approach has the benefit of allowing multi-line notes to\njust be printed the same way as multi-line commit messages, whereas\nthe header output would require one header line per commit line.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133737","messageId":"7vsk9fwbzj.fsf@alter.siamese.dyndns.org","threadId":"22521","inReplyTo":"7veikzfysv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-05T22:31:44Z","receivedAt":"2010-02-05T22:31:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> * \"log --format=%N\" and \"log --show-notes\"\n> \n> Currently %N expands to the hardcoded \"log --show-notes\" default format.\n> We can probably keep it that way. When the user asked for a non default\n> notes hierarchy (i.e. other than refs/notes/commits), we may want to\n> adjust \"Notes:\" string to use \"Notes-%s:\" to show which hierarchy it came\n> from, and concatenate them together.\n>\n> For \"log --show-notes\" output, we also might want to move the notes to the\n> header part like I illustrated above in format-patch output, instead of\n> \"start with unindented Notes: and indented body at the end\". I.e. instead\n> of showing this:\n\nGiuseppe was wondering about multi-line thing, so the illustration should\nbe adjusted to match the \"format-patch\" example to show a multi-line note,\nI think. Here is what I meant.\n\nInstead of showing:\n\n $ git log --notes-ref=amlog -1 4d0cc22\n commit 4d0cc2243778b38c3759c6a08f4f1ed64155a070\n Author: Junio C Hamano \n Date: Thu Feb 4 11:10:44 2010 -0800\n\n fast-import: count --max-pack-size in bytes\n\n Similar in spirit to 07cf0f2 (make --max-pack-size argument to 'git\n ...\n\n Notes:\n pulled on Fri Feb 5 07:36:12 2010 -0800\n from git://git.bogomips.org/git-svn.git/\n Notes-amlog:\n <7v4olwbyvf.fsf_-_@alter.siamese.dyndns.org>\n\nwhich is what 1.6.6 added, showing it like this:\n\n $ git log --notes-ref=amlog -1 4d0cc22\n commit 4d0cc2243778b38c3759c6a08f4f1ed64155a070\n Author: Junio C Hamano \n Date: Thu Feb 4 11:10:44 2010 -0800\n Notes: pulled on Fri Feb 5 07:36:12 2010 -0800\n from git://git.bogomips.org/git-svn.git/\n Notes-amlog: <7v4olwbyvf.fsf_-_@alter.siamese.dyndns.org>\n\n fast-import: count --max-pack-size in bytes\n\n Similar in spirit to 07cf0f2 (make --max-pack-size argument to 'git\n ...\n\nmight be easier to see. After all, notes are metainformation on commits,\nand people who are interested will look at the header, and those who are\nnot will skim over the block of text at the beginning, knowing that is\nwhere all the metainformation is.\n\nBut this is just \"might\", not \"should---I strongly believe\".\n"},{"id":"133758","messageId":"201002060044.11225.jnareb@gmail.com","threadId":"22521","inReplyTo":"1265300338-25021-2-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-05T23:44:09Z","receivedAt":"2010-02-05T23:44:09Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:\n\nBTW. shouldn't this series be marked as RFC?\n\n> Introduce support for notes by collecting them when creating commit\n> lists. The list of noterefs to look into is configurable, and can be a(n\n> array of) refspec(s), which will be looked for in the refs/notes\n> namespace.\n> \n> The feature is disabled by default because it's presently not very\n> efficient (one extra git call per configured refspec, plus two extra git\n> calls per commit per noteref).\n\nSignoff?\n> ---\n> gitweb/gitweb.perl | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++\n> 1 files changed, 62 insertions(+), 0 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index d0c3ff2..9ba5815 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -411,6 +411,22 @@ our %feature = (\n> \t\t'override' => 0,\n> \t\t'default' => [16]},\n> \n> +\t# Notes support. When this feature is enabled, the presence of notes\n> +\t# for any commit is signaled, and the note content is made available\n> +\t# in a way appropriate for the current view.\n> +\t# Set this to '*' to enable all notes namespace, or to a shell-glob\n> +\t# specification to enable specific namespaces only.\n\nIt is not obvious from this description that you can provide _list_ of\nnotes namespaces (or list of shell-globs).\n\n> +\n> +\t# To enable system wide have in $GITWEB_CONFIG\n> +\t# $feature{'notes'}{'default'} = ['*'];\n> +\t# To have project specific config enable override in $GITWEB_CONFIG\n> +\t# $feature{'notes'}{'override'} = 1;\n> +\t# and in project config gitweb.notes = namespace;\n\nHow you can provide list of notes here? Is overriding limited to single\nname or shell-glob?\n\nSee feature_snapshot for example implementation.\n\n> +\t'notes' => {\n> +\t\t'sub' => \\&feature_notes,\n> +\t\t'override' => 0,\n> +\t\t'default' => []},\n> +\n> \t# Avatar support. When this feature is enabled, views such as\n> \t# shortlog or commit will display an avatar associated with\n> \t# the email of the committer(s) and/or author(s).\n> @@ -513,6 +529,16 @@ sub feature_patches {\n> \treturn ($_[0]);\n> }\n> \n> +sub feature_notes {\n> +\tmy @val = (git_get_project_config('notes'));\n> +\n> +\tif (@val) {\n> +\t\treturn @val;\n> +\t}\n> +\n> +\treturn @_;\n> +}\n\nFirst, this I think limits overriding in repository config to single value.\n\nSecond, perhaps it is time to refactor all those similar feature_xxx\nsubroutines (just a possible suggestion)?\n\n> +\n> sub feature_avatar {\n> \tmy @val = (git_get_project_config('avatar'));\n> \n> @@ -2786,10 +2812,30 @@ sub parse_commit {\n> \treturn %co;\n> }\n> \n> +# return all refs matching refs/notes/ where the globspecs\n> +# are taken from the notes feature content.\n> +sub get_note_refs {\n> +\tmy @globs = gitweb_get_feature('notes');\n> +\tmy @note_refs = ();\n> +\tforeach my $glob (@globs) {\n> +\t\tif (open my $fd, '-|', git_cmd(), 'for-each-ref',\n> +\t\t '--format=%(refname)', \"refs/notes/$glob\") {\n\n \t\topen my $fd, '-|', git_cmd(), 'for-each-ref',\n \t\t\t'--format=%(refname)', \"refs/notes/$glob\"\n \t\t\tor return;\n\nwould reduce indent level a bit.\n\n> +\t\t\twhile (<$fd>) {\n> +\t\t\t\tchomp;\n> +\t\t\t\tpush @note_refs, $_ if $_;\n> +\t\t\t}\n\nWhy not simply\n\n \t\tchomp(@note_refs = <$fd>);\n\n> +\t\t\tclose $fd;\n> +\t\t}\n> +\t}\n> +\treturn @note_refs;\n> +}\n> +\n> sub parse_commits {\n> \tmy ($commit_id, $maxcount, $skip, $filename, @args) = @_;\n> \tmy @cos;\n> \n> +\tmy @note_refs = get_note_refs();\n> +\n> \t$maxcount ||= 1;\n> \t$skip ||= 0;\n> \n> @@ -2807,6 +2853,22 @@ sub parse_commits {\n> \t\tor die_error(500, \"Open git-rev-list failed\");\n> \twhile (my $line = <$fd>) {\n> \t\tmy %co = parse_commit_text($line);\n> +\t\tmy %notes = () ;\n> +\t\tforeach my $note_ref (@note_refs) {\n> +\t\t\tmy $obj = \"$note_ref:$co{'id'}\";\n> +\t\t\tif (open my $fd, '-|', git_cmd(), 'rev-parse',\n> +\t\t\t\t'--verify', '-q', $obj) {\n> +\t\t\t\tmy $exists = <$fd>;\n> +\t\t\t\tclose $fd;\n> +\t\t\t\tif (defined $exists) {\n> +\t\t\t\t\tif (open $fd, '-|', git_cmd(), 'show', $obj) {\n> +\t\t\t\t\t\t$notes{$note_ref} = scalar <$fd>;\n> +\t\t\t\t\t\tclose $fd;\n> +\t\t\t\t\t}\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t}\n\nFirst, there are '--batch' and '--batch-check' options to git-cat-file.\nWith these I think you can get all notes with just single git command,\nalthough using it is a bit complicated (requires open2 from IPC::Open2\nfor bidi communication).\n\nSecond, if not using 'git cat-file --batch', perhaps it would be easier\nto read each $note_ref tree using 'git ls-tree'/'git ls-tree -r', and\nparse its output to check for which commits/objects there are notes\navailable, and only then call 'git show' (or reuse connection to\n'git cat-file --batch').\n\nThe second solution, with a bit more work, could work even in presence\nof fan-out schemes for notes, I think.\n\n> +\t\t$co{'notes'} = \\%notes;\n> \t\tpush @cos, \\%co;\n> \t}\n> \tclose $fd;\n> -- \n> 1.7.0.rc1.193.ge8618\n> \n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"133764","messageId":"201002060118.21137.jnareb@gmail.com","threadId":"22521","inReplyTo":"1265300338-25021-3-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCH 2/4] gitweb: show notes in shortlog view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-06T00:18:19Z","receivedAt":"2010-02-06T00:18:19Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:\n\n> Subject: [PATCH 2/4] gitweb: show notes in shortlog view\n\nIs it RFC?\n\nWhy it is only for 'shortlog' view, and not also for 'history' which is\nalso shortlog-like view? Or is there reason why it is not present for\n'history' view?\n\n> The presence of the note is shown by a small icon, hovering on which\n> reveals the actual note content.\n\nSignoff?\n\n> ---\n> gitweb/gitweb.css | 29 +++++++++++++++++++++++++++++\n> gitweb/gitweb.perl | 30 +++++++++++++++++++++++++++++-\n> 2 files changed, 58 insertions(+), 1 deletions(-)\n> \n> diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css\n> index 50067f2..7d1836b 100644\n> --- a/gitweb/gitweb.css\n> +++ b/gitweb/gitweb.css\n> @@ -572,3 +572,32 @@ span.match {\n> div.binary {\n> \tfont-style: italic;\n> }\n> +\n> +span.notes {\n> +\tfloat:right;\n> +\tposition:relative;\n> +}\n> +\n> +span.notes span.note-container:before {\n> +\tcontent: url('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAgAAAAIAgMAAAC5YVYYAAAAAXNSR0IArs4c6QAAAAlQTFRFAABnybuD//+t5rXizQAAAAF0Uk5TAEDm2GYAAAABYktHRACIBR1IAAAAGElEQVQI12MIDWXIWglDQHYIQ1YAQ6gDAFWPBrAKFe0fAAAAAElFTkSuQmCC');\n> +}\n\nNot all web browsers support ':before' pseudo-element, and 'content'\n(pseudo-)property.\n\nNot all web browsers support 'data:' URI schema in CSS; also such image\ncannot be cached (on the other hand it doesn't require extra TCP \nconnection on first access, and CSS file is cached anyway).\n\nOn the other hand adding extra images to gitweb would probably require\nadditional (yet another) build time parameter to tell where static\nimages are (besides logo and favicon).\n\nSo perhaps it is good solution, at least for a first attempt.\n\n[...]\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 9ba5815..1701ed1 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -1628,6 +1628,33 @@ sub format_subject_html {\n> \t}\n> }\n> \n> +# display notes next to a commit\n> +sub format_notes_html {\n> +\tmy %notes = %{$_[0]};\n\nWhy not use 'my $notes = shift;', and later '%$notes'?\n\n> +\tmy $ret = \"\";\n\nPerhaps $return or $result would be a better name, to better distinguish\nit from visually similar $ref (see $ref vs $res);\n\n> +\twhile (my ($ref, $text) = each %notes) {\n> +\t\t# remove 'refs/notes/' and an optional final s\n> +\t\t$ref =~ s/^refs\\/notes\\///;\n\nYou can use different delimiter than / to avoid 'leaning toothpick'\nsyndrome, e.g.: $ref =~ s!^refs/notes/!!;\n\n> +\t\t$ref =~ s/s$//;\n> +\n> +\t\t# double markup is needed to allow pure CSS cross-browser 'popup'\n> +\t\t# of the note\n> +\t\t$ret .= \"\";\n> +\t\t$ret .= \"\";\n> +\t\tforeach my $line (split /\\n/, $text) {\n> +\t\t\t$ret .= esc_html($line) . \"
\";\n\nProbably would want\n\n \t\t\t$ret .= esc_html($line) . \"
\\n\";\n\nhere. Or do we want single string here?\n\n\nAlso, do you want/need final
? If not, perhaps\n\n \t\tjoin(\"
\", map { esc_html($_) } split(/\\n/, $text);\n\nwould be a better solution (you can always add final \"
\" later)?\n\n> +\t\t}\n> +\t\t$ret .= \"\";\n> +\t}\n> +\tif ($ret) {\n> +\t\treturn \"$ret\";\n> +\t} else {\n> +\t\treturn $ret;\n> +\t}\n> +\n> +\n> +}\n> +\n> # Rather than recomputing the url for an email multiple times, we cache it\n> # after the first hit. This gives a visible benefit in views where the avatar\n> # for the same email is used repeatedly (e.g. shortlog).\n> @@ -4595,6 +4622,7 @@ sub git_shortlog_body {\n> \t\tmy %co = %{$commitlist->[$i]};\n> \t\tmy $commit = $co{'id'};\n> \t\tmy $ref = format_ref_marker($refs, $commit);\n> +\t\tmy $notes = format_notes_html($co{'notes'});\n> \t\tif ($alternate) {\n> \t\t\tprint \"\\n\";\n> \t\t} else {\n> @@ -4605,7 +4633,7 @@ sub git_shortlog_body {\n> \t\tprint \"$co{'age_string_date'}\\n\" .\n> \t\t format_author_html('td', \\%co, 10) . \"\";\n> \t\tprint format_subject_html($co{'title'}, $co{'title_short'},\n> -\t\t href(action=>\"commit\", hash=>$commit), $ref);\n> +\t\t href(action=>\"commit\", hash=>$commit), $ref . $notes);\n> \t\tprint \"\\n\" .\n> \t\t \"\" .\n> \t\t $cgi->a({-href => href(action=>\"commit\", hash=>$commit)}, \"commit\") . \" | \" .\n\nNice.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"133783","messageId":"cb7bb73a1002060016p179a3ffdt134897d52f54d41c@mail.gmail.com","threadId":"22521","inReplyTo":"7vsk9fwbzj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-06T08:16:56Z","receivedAt":"2010-02-06T08:16:56Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Fri, Feb 5, 2010 at 11:31 PM, Junio C Hamano wrote:\n>\n> Giuseppe was wondering about multi-line thing, so the illustration should\n> be adjusted to match the \"format-patch\" example to show a multi-line note,\n> I think.  Here is what I meant.\n>\n> Instead of showing:\n>\n>    $ git log --notes-ref=amlog -1 4d0cc22\n>    commit 4d0cc2243778b38c3759c6a08f4f1ed64155a070\n>    Author: Junio C Hamano \n>    Date:   Thu Feb 4 11:10:44 2010 -0800\n>\n>        fast-import: count --max-pack-size in bytes\n>\n>        Similar in spirit to 07cf0f2 (make --max-pack-size argument to 'git\n>        ...\n>\n>    Notes:\n>        pulled on Fri Feb 5 07:36:12 2010 -0800\n>        from git://git.bogomips.org/git-svn.git/\n>    Notes-amlog:\n>        <7v4olwbyvf.fsf_-_@alter.siamese.dyndns.org>\n>\n> which is what 1.6.6 added, showing it like this:\n>\n>    $ git log --notes-ref=amlog -1 4d0cc22\n>    commit 4d0cc2243778b38c3759c6a08f4f1ed64155a070\n>    Author: Junio C Hamano \n>    Date:   Thu Feb 4 11:10:44 2010 -0800\n>    Notes: pulled on Fri Feb 5 07:36:12 2010 -0800\n>     from git://git.bogomips.org/git-svn.git/\n>    Notes-amlog: <7v4olwbyvf.fsf_-_@alter.siamese.dyndns.org>\n>\n>        fast-import: count --max-pack-size in bytes\n>\n>        Similar in spirit to 07cf0f2 (make --max-pack-size argument to 'git\n>        ...\n>\n> might be easier to see.  After all, notes are metainformation on commits,\n> and people who are interested will look at the header, and those who are\n> not will skim over the block of text at the beginning, knowing that is\n> where all the metainformation is.\n>\n> But this is just \"might\", not \"should---I strongly believe\".\n\n\nI'm not convinced. I believe format-patch output should be as close as\npossible to RFC2822 compliance, given how it can be used as basis for\nactual email.\n\nAccording to the RFC, lines should be no more than 78 characters and\nmust be less than 998. The 'one space indent on newline', if I'm not\nmistaken, is a way to indicate a _wrapped_ single line header, rather\nthan a multi-line one.\n\nA scenario such as the following is thus possible: format-patch\ncreates a single-line header which is longer than 78 characters\n(X-Git-Notes-somerefname: 70ish characters line). The patch gets sent\nby email, and a properly configured send-email sends the X-Git-Notes*\nheaders through (BTW, this should probably be added to the\nnotes/send-email TODO). Some intermediate forwarder or user agent\ndecides to rewrap the longish line so that it isn't longer than 78\ncharacters. The user saves the raw email and uses git am to apply it\n-> oops, the single line note has become a two-liner.\n\nIf we use the convention (at least in format-patch, not necessarily in\nlog) of one header line per notes line we should be more on the safe\nside. It would also allow us to keep the headers wrapped at < 78\ncharacters which is nice for legibility on terminals.\n\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133785","messageId":"cb7bb73a1002060102n5544f825x3d01860b1228cee@mail.gmail.com","threadId":"22521","inReplyTo":"201002060044.11225.jnareb@gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-06T09:02:21Z","receivedAt":"2010-02-06T09:02:21Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"2010/2/6 Jakub Narebski :\n> On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:\n>\n> BTW. shouldn't this series be marked as RFC?\n>\n\n[snip]\n\n>\n> Signoff?\n\nY'know, one would figure that, this not being my first contribution\nand what, I'd have learned to do this properly 8-/.\n\n>> +     # Notes support. When this feature is enabled, the presence of notes\n>> +     # for any commit is signaled, and the note content is made available\n>> +     # in a way appropriate for the current view.\n>> +     # Set this to '*' to enable all notes namespace, or to a shell-glob\n>> +     # specification to enable specific namespaces only.\n>\n> It is not obvious from this description that you can provide _list_ of\n> notes namespaces (or list of shell-globs).\n\nI'm starting to think it might make sense to not have a list here, but\nrather a single value only. First of all, multiple refs can be\nindicated à la shell with {ref1,ref2,ref3}. Or, we can also use the\nintended command-line syntax ref1:ref2:ref3, which would help\nconsistency. It also makes things easier for project overrides, as per\nyour subsequent comment:\n\n>> +\n>> +     # To enable system wide have in $GITWEB_CONFIG\n>> +     # $feature{'notes'}{'default'} = ['*'];\n>> +     # To have project specific config enable override in $GITWEB_CONFIG\n>> +     # $feature{'notes'}{'override'} = 1;\n>> +     # and in project config gitweb.notes = namespace;\n>\n> How you can provide list of notes here?  Is overriding limited to single\n> name or shell-glob?\n>\n> See feature_snapshot for example implementation.\n\nAs mentioned above, I'd rather use the same syntax deployed on the\ncommand line, either shell-like or PATH-like multiple paths.\n\n> Second, perhaps it is time to refactor all those similar feature_xxx\n> subroutines (just a possible suggestion)?\n\nfeature_notes looks remarkably like feature_avatar, indeed.\n\n>> +# return all refs matching refs/notes/ where the globspecs\n>> +# are taken from the notes feature content.\n>> +sub get_note_refs {\n>> +     my @globs = gitweb_get_feature('notes');\n>> +     my @note_refs = ();\n>> +     foreach my $glob (@globs) {\n>> +             if (open my $fd, '-|', git_cmd(), 'for-each-ref',\n>> +                 '--format=%(refname)', \"refs/notes/$glob\") {\n>\n>                open my $fd, '-|', git_cmd(), 'for-each-ref',\n>                        '--format=%(refname)', \"refs/notes/$glob\"\n>                        or return;\n>\n> would reduce indent level a bit.\n\nGood idea, thanks.\n\n>> +                     while (<$fd>) {\n>> +                             chomp;\n>> +                             push @note_refs, $_ if $_;\n>> +                     }\n>\n> Why not simply\n>\n>                chomp(@note_refs = <$fd>);\n\nBecause I didn't know chomp worked on lists. Thanks for the idea.\n\n>> +             my %notes = () ;\n>> +             foreach my $note_ref (@note_refs) {\n>> +                     my $obj = \"$note_ref:$co{'id'}\";\n>> +                     if (open my $fd, '-|', git_cmd(), 'rev-parse',\n>> +                             '--verify', '-q', $obj) {\n>> +                             my $exists = <$fd>;\n>> +                             close $fd;\n>> +                             if (defined $exists) {\n>> +                                     if (open $fd, '-|', git_cmd(), 'show', $obj) {\n>> +                                             $notes{$note_ref} = scalar <$fd>;\n>> +                                             close $fd;\n>> +                                     }\n>> +                             }\n>> +                     }\n>> +             }\n>\n> First, there are '--batch' and '--batch-check' options to git-cat-file.\n> With these I think you can get all notes with just single git command,\n> although using it is a bit complicated (requires open2 from IPC::Open2\n> for bidi communication).\n\nHm. The IPC::Open2 doc makes it sound horribly scary, but still doable.\n\n> Second, if not using 'git cat-file --batch', perhaps it would be easier\n> to read each $note_ref tree using 'git ls-tree'/'git ls-tree -r', and\n> parse its output to check for which commits/objects there are notes\n> available, and only then call 'git show' (or reuse connection to\n> 'git cat-file --batch').\n>\n> The second solution, with a bit more work, could work even in presence\n> of fan-out schemes for notes, I think.\n\nAn interesting approach. Without fan-out, git ls-tree -r\nrefs/notes/whatever [hash ...] gives us the blobs we're interested in.\nIn case of fan-out schemes, the efficiency of this approach probably\ndepends on the kind of fan-out we have, and would require some\nheavy-duty grepping. A git ls-notes plumbing with a similar syntax and\noutput would be a nice thing to have.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133786","messageId":"cb7bb73a1002060124s7f9bc4f7y9c2cb418e4802f64@mail.gmail.com","threadId":"22521","inReplyTo":"201002060118.21137.jnareb@gmail.com","subject":"Re: [PATCH 2/4] gitweb: show notes in shortlog view","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-06T09:24:39Z","receivedAt":"2010-02-06T09:24:39Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"2010/2/6 Jakub Narebski :\n> On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:\n>\n>> Subject: [PATCH 2/4] gitweb: show notes in shortlog view\n>\n> Is it RFC?\n\nSee reply to comments on 1/4 8-/\n\n> Why it is only for 'shortlog' view, and not also for 'history' which is\n> also shortlog-like view?  Or is there reason why it is not present for\n> 'history' view?\n\nI always forget about history view, probably because I never use it.\n\n>> The presence of the note is shown by a small icon, hovering on which\n>> reveals the actual note content.\n>\n> Signoff?\n\nA-hem. (whistles innocently)\n\n>> +\n>> +span.notes span.note-container:before {\n>> +     content: url('data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAgAAAAIAgMAAAC5YVYYAAAAAXNSR0IArs4c6QAAAAlQTFRFAABnybuD//+t5rXizQAAAAF0Uk5TAEDm2GYAAAABYktHRACIBR1IAAAAGElEQVQI12MIDWXIWglDQHYIQ1YAQ6gDAFWPBrAKFe0fAAAAAElFTkSuQmCC');\n>> +}\n>\n> Not all web browsers support ':before' pseudo-element, and 'content'\n> (pseudo-)property.\n\nI know it's neither good form nor good webdesigner attitude, but I\nstopped caring about IE a long time ago. I understand however that\nsome ancient versions of Mozilla browsers might have the same issue\ntoo.\n\n> Not all web browsers support 'data:' URI schema in CSS; also such image\n> cannot be cached (on the other hand it doesn't require extra TCP\n> connection on first access, and CSS file is cached anyway).\n>\n> On the other hand adding extra images to gitweb would probably require\n> additional (yet another) build time parameter to tell where static\n> images are (besides logo and favicon).\n>\n> So perhaps it is good solution, at least for a first attempt.\n\nA possible alternative could maybe do without images and just use\nborders and backgrounds of an 8x8 fixed-size element. Wouldn't look as\nnice, probably, but should render decently in everything that supports\nCSS1.\n\n>> +# display notes next to a commit\n>> +sub format_notes_html {\n>> +     my %notes = %{$_[0]};\n>\n> Why not use 'my $notes = shift;', and later '%$notes'?\n\nNo particular reason. I didn't check for syntax preferences regarding\nthis in gitweb, or I would have noticed there was a preference for the\none you mention.\n\n>> +     my $ret = \"\";\n>\n> Perhaps $return or $result would be a better name, to better distinguish\n> it from visually similar $ref (see $ref vs $res);\n\nYep, good point.\n\n>> +     while (my ($ref, $text) = each %notes) {\n>> +             # remove 'refs/notes/' and an optional final s\n>> +             $ref =~ s/^refs\\/notes\\///;\n>\n> You can use different delimiter than / to avoid 'leaning toothpick'\n> syndrome, e.g.: $ref =~ s!^refs/notes/!!;\n\nIndeed I should.\n\n>> +             $ref =~ s/s$//;\n>> +\n>> +             # double markup is needed to allow pure CSS cross-browser 'popup'\n>> +             # of the note\n>> +             $ret .= \"\";\n>> +             $ret .= \"\";\n>> +             foreach my $line (split /\\n/, $text) {\n>> +                     $ret .= esc_html($line) . \"
\";\n>\n> Probably would want\n>\n>                        $ret .= esc_html($line) . \"
\\n\";\n>\n> here.  Or do we want single string here?\n\nIt's within a span element so I was trying to stick to single line in\nthe HTML source.\n\n> Also, do you want/need final
?  If not, perhaps\n>\n>                join(\"
\", map { esc_html($_) } split(/\\n/, $text);\n>\n> would be a better solution (you can always add final \"
\" later)?\n\nI did notice that the final br didn't seem to affect the box height,\nso I didn't bother looking at ways to do without it, but it's probably\nnicer to not have it.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133795","messageId":"201002061357.59245.jnareb@gmail.com","threadId":"22521","inReplyTo":"1265300338-25021-4-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCH 3/4] gitweb: show notes in log","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-06T12:57:57Z","receivedAt":"2010-02-06T12:57:57Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:\n\n> The notes are shown in full to the left of the log message.\n\nThats all good if you have wide (high resolution) screen, and your \nproject follows common commit message conventions of keeping lines in \ncommit message no longer than at most 80 characters, and you don't need \nto use large size fonts.\n\nWhat happens if screen size is too small to contain both commit message \nand notes? Does it do the sensible thing of putting notes _below_ \ncommit message in such situation? I do not know CSS+HTML enogh to \nanswer this question myself.\n\nBTW. signoff?\n\n\nP.S. We would probably want some support for notes also in feeds (Atom \nand RSS feed), but this can be left for the future commit.\n\n\n> @@ -1631,6 +1631,7 @@ sub format_subject_html {\n> # display notes next to a commit\n> sub format_notes_html {\n> \tmy %notes = %{$_[0]};\n> +\tmy $tag = $_[1] || 'span' ;\n\nThis could be\n\n \tmy $notes = shift;\n \tmy $tag = shift || 'span' ;\n\nand then use %$notes.\n\n> \tmy $ret = \"\";\n> \twhile (my ($ref, $text) = each %notes) {\n> \t\t# remove 'refs/notes/' and an optional final s\n> @@ -1639,15 +1640,15 @@ sub format_notes_html {\n> \n> \t\t# double markup is needed to allow pure CSS cross-browser 'popup'\n> \t\t# of the note\n> -\t\t$ret .= \"\";\n> -\t\t$ret .= \"\";\n> +\t\t$ret .= \"<$tag title='$ref' class='note-container $ref'>\";\n> +\t\t$ret .= \"<$tag title='$ref' class='note $ref'>\";\n> \t\tforeach my $line (split /\\n/, $text) {\n> \t\t\t$ret .= esc_html($line) . \"
\";\n> \t\t}\n> -\t\t$ret .= \"\";\n> +\t\t$ret .= \"\";\n> \t}\n> \tif ($ret) {\n> -\t\treturn \"$ret\";\n> +\t\treturn \"<$tag class='notes'>$ret\";\n> \t} else {\n> \t\treturn $ret;\n> \t}\n\nNice trick, but is this distinction really necessary?\n\n> @@ -4581,6 +4582,7 @@ sub git_log_body {\n> \t\tnext if !%co;\n> \t\tmy $commit = $co{'id'};\n> \t\tmy $ref = format_ref_marker($refs, $commit);\n> +\t\tmy $notes = format_notes_html($co{'notes'}, 'div');\n> \t\tmy %ad = parse_date($co{'author_epoch'});\n> \t\tgit_print_header_div('commit',\n> \t\t \"$co{'age_string'}\" .\n> @@ -4598,6 +4600,7 @@ sub git_log_body {\n> \t\t git_print_authorship(\\%co, -tag => 'span');\n> \t\t print \"
\\n\\n\";\n> \n> +\t\tprint \"$notes\\n\";\n> \t\tprint \"
\\n\";\n> \t\tgit_print_log($co{'comment'}, -final_empty_line=> 1);\n> \t\tprint \"
\\n\";\n\nWith respect to the question about what happens if the screen is not \nwide enough, shouldn't notes be put in HTML source below body (commit \nmessage)?\n\n-- \nJakub Narebski\nPoland\n"},{"id":"133796","messageId":"cb7bb73a1002060514p33a4bcbdqb13d636cd6448998@mail.gmail.com","threadId":"22521","inReplyTo":"201002061357.59245.jnareb@gmail.com","subject":"Re: [PATCH 3/4] gitweb: show notes in log","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-06T13:14:23Z","receivedAt":"2010-02-06T13:14:23Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"2010/2/6 Jakub Narebski :\n> On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:\n>\n>> The notes are shown in full to the left of the log message.\n>\n> Thats all good if you have wide (high resolution) screen, and your\n> project follows common commit message conventions of keeping lines in\n> commit message no longer than at most 80 characters, and you don't need\n> to use large size fonts.\n>\n> What happens if screen size is too small to contain both commit message\n> and notes?  Does it do the sensible thing of putting notes _below_\n> commit message in such situation?  I do not know CSS+HTML enogh to\n> answer this question myself.\n\nThe CSS forces the width of the notes div at 150px, which is the\namount left to the left of the commit message. This means that notes\nwill line-wrap, but they will not shift the text.\n\n> BTW. signoff?\n\n\nAs usual, I forgot.\n\n> P.S. We would probably want some support for notes also in feeds (Atom\n> and RSS feed), but this can be left for the future commit.\n\nI honestly have absolutely no idea how to do that.\n\n>> @@ -1631,6 +1631,7 @@ sub format_subject_html {\n>>  # display notes next to a commit\n>>  sub format_notes_html {\n>>       my %notes = %{$_[0]};\n>> +     my $tag = $_[1] || 'span' ;\n>\n> This could be\n>\n>        my $notes = shift;\n>        my $tag = shift || 'span' ;\n>\n> and then use %$notes.\n\nWould be much bettere, yes.\n\n>>       my $ret = \"\";\n>>       while (my ($ref, $text) = each %notes) {\n>>               # remove 'refs/notes/' and an optional final s\n>> @@ -1639,15 +1640,15 @@ sub format_notes_html {\n>>\n>>               # double markup is needed to allow pure CSS cross-browser 'popup'\n>>               # of the note\n>> -             $ret .= \"\";\n>> -             $ret .= \"\";\n>> +             $ret .= \"<$tag title='$ref' class='note-container $ref'>\";\n>> +             $ret .= \"<$tag title='$ref' class='note $ref'>\";\n>>               foreach my $line (split /\\n/, $text) {\n>>                       $ret .= esc_html($line) . \"
\";\n>>               }\n>> -             $ret .= \"\";\n>> +             $ret .= \"\";\n>>       }\n>>       if ($ret) {\n>> -             return \"$ret\";\n>> +             return \"<$tag class='notes'>$ret\";\n>>       } else {\n>>               return $ret;\n>>       }\n>\n> Nice trick, but is this distinction really necessary?\n\nI think so. The distinction is useful both from the structural point\nof view (block elements with block elements, inline elements with\ninline elements) and for CSS selection (the block case has totally\ndifferent styling than the inline case).\n\n>> +             print \"$notes\\n\";\n>>               print \"
\\n\";\n>>               git_print_log($co{'comment'}, -final_empty_line=> 1);\n>>               print \"
\\n\";\n>\n> With respect to the question about what happens if the screen is not\n> wide enough, shouldn't notes be put in HTML source below body (commit\n> message)?\n\nAs I mentioned, notes width is fixed at the amount of the whitespace\nto the left of the log, so this should not be an issue. Additionally,\nputting notes below makes it much harder to let them float to the left\nof the log body.\n\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133797","messageId":"201002061416.29481.jnareb@gmail.com","threadId":"22521","inReplyTo":"1265300338-25021-5-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCH 4/4] gitweb: show notes in commit(diff) view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-06T13:16:28Z","receivedAt":"2010-02-06T13:16:28Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 4 Jan 2010, Giuseppe Bilotta wrote:\n\n> The notes are shown side-by-side along the bottom of the commit\n> message. \n\nThe same question apply as for previous commit.\n\nWhat happens if screen size is too small to contain both commit message\nand notes? Does it do the sensible thing of putting notes _below_\ncommit message in such situation? I do not know CSS+HTML enogh to\nanswer this question myself.\n\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 0d0877e..0d03026 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -2837,12 +2837,31 @@ sub parse_commit {\n> \t%co = parse_commit_text(<$fd>, 1);\n> \tclose $fd;\n> \n> +\tmy %notes = ();\n> +\tforeach my $note_ref (get_note_refs()) {\n> +\t\tmy $obj = \"$note_ref:$co{'id'}\";\n> +\t\tif (open my $fd, '-|', git_cmd(), 'rev-parse',\n> +\t\t\t'--verify', '-q', $obj) {\n> +\t\t\tmy $exists = <$fd>;\n> +\t\t\tclose $fd;\n> +\t\t\tif (defined $exists) {\n> +\t\t\t\tif (open $fd, '-|', git_cmd(), 'show', $obj) {\n> +\t\t\t\t\t$notes{$note_ref} = scalar <$fd>;\n> +\t\t\t\t\tclose $fd;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\t$co{'notes'} = \\%notes;\n> +\n> \treturn %co;\n> }\n\nDuplicated code. Please put this code in a separate subroutine, to be\ncalled in those two places.\n \n> # return all refs matching refs/notes/ where the globspecs\n> # are taken from the notes feature content.\n> sub get_note_refs {\n> +\tlocal $/ = \"\";\n> +\n\nWhy it is needed here? Why you want to use empty lines as terminator\n(which means reading whole paragraphs), while treating two or more\nconsecutive empty lines as a single empty line (according to\nperlvar(1))?\n\nIf you want to slurp whole file, this should be\n\n \tlocal $/;\n\nor more explicit\n\n \tlocal $/ = undef;\n\n> \tmy @globs = gitweb_get_feature('notes');\n> \tmy @note_refs = ();\n> \tforeach my $glob (@globs) {\n> @@ -5875,6 +5894,7 @@ sub git_commit {\n> \n> \tprint \"
\\n\";\n> \tgit_print_log($co{'comment'});\n> +\tprint format_notes_html($co{'notes'}, 'div');\n> \tprint \"
\\n\";\n> \n> \tgit_difftree_body(\\@difftree, $hash, @$parents);\n> @@ -6230,6 +6250,7 @@ sub git_commitdiff {\n> \t\t\tgit_print_log($co{'comment'}, -final_empty_line=> 1, -remove_title => 1);\n> \t\t\tprint \"\\n\"; # class=\"log\"\n> \t\t}\n> +\t\tprint format_notes_html($co{'notes'}, 'div');\n> \n> \t} elsif ($format eq 'plain') {\n> \t\tmy $refs = git_get_references(\"tags\");\n\nThis of course assumes that we want notes treated exactly (or almost\nexactly) the same way for 'log', 'commit' and 'commitdiff' views.\nPerhaps it is a good assumption (at least for first step)...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"133803","messageId":"cb7bb73a1002060615o261d974cnf347ba5af5ae324b@mail.gmail.com","threadId":"22521","inReplyTo":"201002061416.29481.jnareb@gmail.com","subject":"Re: [PATCH 4/4] gitweb: show notes in commit(diff) view","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-06T14:15:39Z","receivedAt":"2010-02-06T14:15:39Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"2010/2/6 Jakub Narebski :\n> On Thu, 4 Jan 2010, Giuseppe Bilotta wrote:\n>\n>> The notes are shown side-by-side along the bottom of the commit\n>> message.\n>\n> The same question apply as for previous commit.\n>\n> What happens if screen size is too small to contain both commit message\n> and notes?  Does it do the sensible thing of putting notes _below_\n> commit message in such situation?  I do not know CSS+HTML enogh to\n> answer this question myself.\n\nIn this view the notes are printed side-by-side to each other, but at\nthe end of the commit message, so there's no interference at all.\n\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index 0d0877e..0d03026 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -2837,12 +2837,31 @@ sub parse_commit {\n>>       %co = parse_commit_text(<$fd>, 1);\n>>       close $fd;\n>>\n>> +     my %notes = ();\n>> +     foreach my $note_ref (get_note_refs()) {\n>> +             my $obj = \"$note_ref:$co{'id'}\";\n>> +             if (open my $fd, '-|', git_cmd(), 'rev-parse',\n>> +                     '--verify', '-q', $obj) {\n>> +                     my $exists = <$fd>;\n>> +                     close $fd;\n>> +                     if (defined $exists) {\n>> +                             if (open $fd, '-|', git_cmd(), 'show', $obj) {\n>> +                                     $notes{$note_ref} = scalar <$fd>;\n>> +                                     close $fd;\n>> +                             }\n>> +                     }\n>> +             }\n>> +     }\n>> +     $co{'notes'} = \\%notes;\n>> +\n>>       return %co;\n>>  }\n>\n> Duplicated code.  Please put this code in a separate subroutine, to be\n> called in those two places.\n\nYup, definitely a good idea.\n\n>>  # return all refs matching refs/notes/ where the globspecs\n>>  # are taken from the notes feature content.\n>>  sub get_note_refs {\n>> +     local $/ = \"\";\n>> +\n>\n> Why it is needed here?  Why you want to use empty lines as terminator\n> (which means reading whole paragraphs), while treating two or more\n> consecutive empty lines as a single empty line (according to\n> perlvar(1))?\n>\n> If you want to slurp whole file, this should be\n>\n>        local $/;\n>\n> or more explicit\n>\n>        local $/ = undef;\n\nAh, sorry, for some reason I thought \"\" was the default.\n\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133804","messageId":"201002061534.02972.jnareb@gmail.com","threadId":"22521","inReplyTo":"cb7bb73a1002060615o261d974cnf347ba5af5ae324b@mail.gmail.com","subject":"Re: [PATCH 4/4] gitweb: show notes in commit(diff) view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-06T14:34:02Z","receivedAt":"2010-02-06T14:34:02Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Giuseppe Bilotta wrote:\n> 2010/2/6 Jakub Narebski :\n>> On Thu, 4 Jan 2010, Giuseppe Bilotta wrote:\n\n[...]\n>>>  # return all refs matching refs/notes/ where the globspecs\n>>>  # are taken from the notes feature content.\n>>>  sub get_note_refs {\n>>> +     local $/ = \"\";\n>>> +\n>>\n>> Why it is needed here?  Why you want to use empty lines as terminator\n>> (which means reading whole paragraphs), while treating two or more\n>> consecutive empty lines as a single empty line (according to\n>> perlvar(1))?\n>>\n>> If you want to slurp whole file, this should be\n>>\n>>        local $/;\n>>\n>> or more explicit\n>>\n>>        local $/ = undef;\n> \n> Ah, sorry, for some reason I thought \"\" was the default.\n\nIf you wanted to use default value, why set it at all?\n\n-- \nJakub Narebski\nPoland\n"},{"id":"133810","messageId":"cb7bb73a1002060813l451ad669nf9ba2dc4643bbd58@mail.gmail.com","threadId":"22521","inReplyTo":"201002061534.02972.jnareb@gmail.com","subject":"Re: [PATCH 4/4] gitweb: show notes in commit(diff) view","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-06T16:13:02Z","receivedAt":"2010-02-06T16:13:02Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Sat, Feb 6, 2010 at 3:34 PM, Jakub Narebski wrote:\n> Giuseppe Bilotta wrote:\n>> 2010/2/6 Jakub Narebski :\n>>> On Thu, 4 Jan 2010, Giuseppe Bilotta wrote:\n>\n> [...]\n>>>>  # return all refs matching refs/notes/ where the globspecs\n>>>>  # are taken from the notes feature content.\n>>>>  sub get_note_refs {\n>>>> +     local $/ = \"\";\n>>>> +\n>>>\n>>> Why it is needed here?  Why you want to use empty lines as terminator\n>>> (which means reading whole paragraphs), while treating two or more\n>>> consecutive empty lines as a single empty line (according to\n>>> perlvar(1))?\n>>>\n>>> If you want to slurp whole file, this should be\n>>>\n>>>        local $/;\n>>>\n>>> or more explicit\n>>>\n>>>        local $/ = undef;\n>>\n>> Ah, sorry, for some reason I thought \"\" was the default.\n>\n> If you wanted to use default value, why set it at all?\n\n\nAch, sorry, forgot to reply to the first part of the question. It's\nused in a context where $/ is locally set to \\0, so it needs to be\nreset.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133829","messageId":"201002062247.30691.jnareb@gmail.com","threadId":"22521","inReplyTo":"cb7bb73a1002060514p33a4bcbdqb13d636cd6448998@mail.gmail.com","subject":"Re: [PATCH 3/4] gitweb: show notes in log","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-06T21:47:27Z","receivedAt":"2010-02-06T21:47:27Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:\n> 2010/2/6 Jakub Narebski :\n> > On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:\n> >\n> > > The notes are shown in full to the left of the log message.\n> >\n> > Thats all good if you have wide (high resolution) screen, and your\n> > project follows common commit message conventions of keeping lines in\n> > commit message no longer than at most 80 characters, and you don't need\n> > to use large size fonts.\n> >\n> > What happens if screen size is too small to contain both commit message\n> > and notes?  Does it do the sensible thing of putting notes _below_\n> > commit message in such situation?  I do not know CSS+HTML enogh to\n> > answer this question myself.\n> \n> The CSS forces the width of the notes div at 150px, which is the\n> amount left to the left of the commit message. This means that notes\n> will line-wrap, but they will not shift the text.\n[...]\n> > > +             print \"$notes\\n\";\n> > >               print \"
\\n\";\n> > >               git_print_log($co{'comment'}, -final_empty_line=> 1);\n> > >               print \"
\\n\";\n> >\n> > With respect to the question about what happens if the screen is not\n> > wide enough, shouldn't notes be put in HTML source below body (commit\n> > message)?\n> \n> As I mentioned, notes width is fixed at the amount of the whitespace\n> to the left of the log, so this should not be an issue. Additionally,\n> putting notes below makes it much harder to let them float to the left\n> of the log body.\n\nPerhaps the log body should be floated to the right, instead of notes\nbeing floated to the left, so that when screen width is to narrow for\nboth commit message and notes, notes would be put below commit message.\n\nA question how to create styles using HTML elements and CSS styling\nto get side-by-side with one below other as fallback can be asked\non http://stackoverflow.com, or perhaps even better on http://doctype.com/\n\n-- \nJakub Narebski\nPoland\n"},{"id":"133828","messageId":"201002062250.43613.jnareb@gmail.com","threadId":"22521","inReplyTo":"cb7bb73a1002060813l451ad669nf9ba2dc4643bbd58@mail.gmail.com","subject":"Re: [PATCH 4/4] gitweb: show notes in commit(diff) view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-06T21:50:43Z","receivedAt":"2010-02-06T21:50:43Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:\n> On Sat, Feb 6, 2010 at 3:34 PM, Jakub Narebski wrote:\n>> Giuseppe Bilotta wrote:\n>>> 2010/2/6 Jakub Narebski :\n>>>> On Thu, 4 Jan 2010, Giuseppe Bilotta wrote:\n>>\n>> [...]\n>>>>>  # return all refs matching refs/notes/ where the globspecs\n>>>>>  # are taken from the notes feature content.\n>>>>>  sub get_note_refs {\n>>>>> +     local $/ = \"\";\n>>>>> +\n>>>>\n>>>> Why it is needed here?  Why you want to use empty lines as terminator\n>>>> (which means reading whole paragraphs), while treating two or more\n>>>> consecutive empty lines as a single empty line (according to\n>>>> perlvar(1))?\n>>>>\n>>>> If you want to slurp whole file, this should be\n>>>>\n>>>>        local $/;\n>>>>\n>>>> or more explicit\n>>>>\n>>>>        local $/ = undef;\n>>>\n>>> Ah, sorry, for some reason I thought \"\" was the default.\n>>\n>> If you wanted to use default value, why set it at all?\n> \n> Ach, sorry, forgot to reply to the first part of the question. It's\n> used in a context where $/ is locally set to \\0, so it needs to be\n> reset.\n\nOh, so it should be something like the following, then?\n\n sub get_note_refs {\n +\t# reset to default value (can be called with $/ set to \"\\0\")\n +\tlocal $/ = \"\\n\"; # line by line\n\n> \n> -- \n> Giuseppe \"Oblomov\" Bilotta\n> \n\n-- \nJakub Narebski\nPoland\n"},{"id":"133830","messageId":"201002062314.00631.jnareb@gmail.com","threadId":"22521","inReplyTo":"cb7bb73a1002060102n5544f825x3d01860b1228cee@mail.gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-06T22:14:00Z","receivedAt":"2010-02-06T22:14:00Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:\n> 2010/2/6 Jakub Narebski :\n>> On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:\n\n>>> +     # Notes support. When this feature is enabled, the presence of notes\n>>> +     # for any commit is signaled, and the note content is made available\n>>> +     # in a way appropriate for the current view.\n>>> +     # Set this to '*' to enable all notes namespace, or to a shell-glob\n>>> +     # specification to enable specific namespaces only.\n>>\n>> It is not obvious from this description that you can provide _list_ of\n>> notes namespaces (or list of shell-globs).\n> \n> I'm starting to think it might make sense to not have a list here, but\n> rather a single value only. First of all, multiple refs can be\n> indicated à la shell with {ref1,ref2,ref3}. Or, we can also use the\n> intended command-line syntax ref1:ref2:ref3, which would help\n> consistency. It also makes things easier for project overrides, as per\n> your subsequent comment:\n\nSo it is to be single shell-glob / fnmatch (I think) compatible pattern,\nisn't it?\n \n[...]\n>>> +             my %notes = () ;\n>>> +             foreach my $note_ref (@note_refs) {\n>>> +                     my $obj = \"$note_ref:$co{'id'}\";\n>>> +                     if (open my $fd, '-|', git_cmd(), 'rev-parse',\n>>> +                             '--verify', '-q', $obj) {\n>>> +                             my $exists = <$fd>;\n>>> +                             close $fd;\n>>> +                             if (defined $exists) {\n>>> +                                     if (open $fd, '-|', git_cmd(), 'show', $obj) {\n>>> +                                             $notes{$note_ref} = scalar <$fd>;\n>>> +                                             close $fd;\n>>> +                                     }\n>>> +                             }\n>>> +                     }\n>>> +             }\n>>\n>> First, there are '--batch' and '--batch-check' options to git-cat-file.\n>> With these I think you can get all notes with just single git command,\n>> although using it is a bit complicated (requires open2 from IPC::Open2\n>> for bidi communication).\n> \n> Hm. The IPC::Open2 doc makes it sound horribly scary, but still doable.\n\nIt would look something like the following (fragment of my WIP code):\n\n\tuse IPC::Open2 qw(open2);\n\tuse IO::Handle;\n\n\t# ...\n\n\tunless ($object_stdout) {\n\t\t# Open bidi pipe the first time get_object is called.\n\t\t# open2 raises an exception on error, no need to 'or die'.\n\t\t$object_pid =\n\t\t\topen2($object_stdout, $object_stdin,\n\t\t\t git_cmd(), 'cat-file', '--batch');\n\t}\n\t$object_stdin->printflush(\"$object_id\\n\") # NB: \\n required to avoid deadlocking\n\t\tor die \"get_object: cannot write to pipe: $!\";\n\tmy ($sha1, $type, $size) =\n\t\tsplit ' ', $object_stdout->getline()\n\t\tor die \"get_object: cannot read from pipe: $!\";\n\tdie \"'$object_id' not found in repository\"\n\t\tif $type eq 'missing';\n\t$object_stdout->read(my $content, $size);\n\t$object_stdout->getline(); # eat trailing newline\n \nThe above fragment of code is tested that it works. You would probably\nneed to replace dies with something less fatal...\n\n>> Second, if not using 'git cat-file --batch', perhaps it would be easier\n>> to read each $note_ref tree using 'git ls-tree'/'git ls-tree -r', and\n>> parse its output to check for which commits/objects there are notes\n>> available, and only then call 'git show' (or reuse connection to\n>> 'git cat-file --batch').\n>>\n>> The second solution, with a bit more work, could work even in presence\n>> of fan-out schemes for notes, I think.\n> \n> An interesting approach. Without fan-out, git ls-tree -r\n> refs/notes/whatever [hash ...] gives us the blobs we're interested in.\n> In case of fan-out schemes, the efficiency of this approach probably\n> depends on the kind of fan-out we have, and would require some\n> heavy-duty grepping. A git ls-notes plumbing with a similar syntax and\n> output would be a nice thing to have.\n\nNo grepping, just pass '-r' option to 'git-ls-tree', and use\nparse_ls_tree_line() to parse lines. Then if we have fanout scheme\nwe would get, I guess, something like the following:\n\n 100644 blob 23da787d...\tde/adbeef...\n 100644 blob bc10f25f...\tc5/31d986...\n 100644 blob c9656ece...\t24/d93129...\n\nNow you only need to s!/!!g on filename to get SHA1 of annotated object\n(for which is the note).\n\nThe identifier of note itself would be either id from tree (e.g. 23da787d...\nfor note from first line), or note namespace composed with note \"pathname\",\n(e.g. refs/notes/commits:de/adbeef... for note from first line).\n\nEven if you use one git-show per note, it would need only git commands\nto discover object to note mapping.\n\n\nP.S. We still would want parse_commit_text to parse notes from default\nnamespace. parse_commit / parse_commits output contains notes from\ndefault namespace, e.g.:\n\n d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3 94ac0c7b30a7dc43d926b0ffbe90892c1e19e5f6\n tree b9ee8876df81b80b13c6b017be993fff8427cfaf\n parent 94ac0c7b30a7dc43d926b0ffbe90892c1e19e5f6\n author Jakub Narebski 1265309578 +0100\n committer Jakub Narebski 1265309578 +0100\n\n This is a commit message\n \n Signed-off-by: Jakub Narebski \n \n Notes:\n This is just a note for commit d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3\n\nto get commit message lines in $co{'comment'} (as array reference),\nand notes in $co{'note'} (or $co{'notes'}).\n\n-- \nJakub Narebski\nPoland\n"},{"id":"133831","messageId":"cb7bb73a1002061417n41213e65g6348bf4fd752f672@mail.gmail.com","threadId":"22521","inReplyTo":"201002062250.43613.jnareb@gmail.com","subject":"Re: [PATCH 4/4] gitweb: show notes in commit(diff) view","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-06T22:17:17Z","receivedAt":"2010-02-06T22:17:17Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Sat, Feb 6, 2010 at 10:50 PM, Jakub Narebski wrote:\n>\n> Oh, so it should be something like the following, then?\n>\n>   sub get_note_refs {\n>  +     # reset to default value (can be called with $/ set to \"\\0\")\n>  +     local $/ = \"\\n\"; # line by line\n\nYes, it's probably worth to mention it in a comment.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133833","messageId":"cb7bb73a1002061458s5b2c1e7ere83111429473d11c@mail.gmail.com","threadId":"22521","inReplyTo":"201002062314.00631.jnareb@gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-06T22:58:54Z","receivedAt":"2010-02-06T22:58:54Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Sat, Feb 6, 2010 at 11:14 PM, Jakub Narebski wrote:\n> On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:\n>> 2010/2/6 Jakub Narebski :\n>>> On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:\n>\n>>>> +     # Notes support. When this feature is enabled, the presence of notes\n>>>> +     # for any commit is signaled, and the note content is made available\n>>>> +     # in a way appropriate for the current view.\n>>>> +     # Set this to '*' to enable all notes namespace, or to a shell-glob\n>>>> +     # specification to enable specific namespaces only.\n>>>\n>>> It is not obvious from this description that you can provide _list_ of\n>>> notes namespaces (or list of shell-globs).\n>>\n>> I'm starting to think it might make sense to not have a list here, but\n>> rather a single value only. First of all, multiple refs can be\n>> indicated à la shell with {ref1,ref2,ref3}. Or, we can also use the\n>> intended command-line syntax ref1:ref2:ref3, which would help\n>> consistency. It also makes things easier for project overrides, as per\n>> your subsequent comment:\n>\n> So it is to be single shell-glob / fnmatch (I think) compatible pattern,\n> isn't it?\n\nSort of. fnmatch doesn't do brace expansion, which is a pity IMO, but\nthat's just my personal preference. Colon-separated, fnmatched\ncomponents is probably the easiest thing to implement to have multiple\nrefs. I'll go with whatever is chosen for core.\n\n>>> First, there are '--batch' and '--batch-check' options to git-cat-file.\n>>> With these I think you can get all notes with just single git command,\n>>> although using it is a bit complicated (requires open2 from IPC::Open2\n>>> for bidi communication).\n>>\n>> Hm. The IPC::Open2 doc makes it sound horribly scary, but still doable.\n>\n> It would look something like the following (fragment of my WIP code):\n>\n>        use IPC::Open2 qw(open2);\n>        use IO::Handle;\n>\n>        # ...\n>\n>        unless ($object_stdout) {\n>                # Open bidi pipe the first time get_object is called.\n>                # open2 raises an exception on error, no need to 'or die'.\n>                $object_pid =\n>                        open2($object_stdout, $object_stdin,\n>                              git_cmd(), 'cat-file', '--batch');\n>        }\n>        $object_stdin->printflush(\"$object_id\\n\") # NB: \\n required to avoid deadlocking\n>                or die \"get_object: cannot write to pipe: $!\";\n>        my ($sha1, $type, $size) =\n>                split ' ', $object_stdout->getline()\n>                or die \"get_object: cannot read from pipe: $!\";\n>        die \"'$object_id' not found in repository\"\n>                if $type eq 'missing';\n>        $object_stdout->read(my $content, $size);\n>        $object_stdout->getline();  # eat trailing newline\n>\n> The above fragment of code is tested that it works.  You would probably\n> need to replace dies with something less fatal...\n\nOn the other hand, as mentioned by Junio, this approach is not\nfuture-proof enough for any kind of fan-out schemes.\n\n>>> Second, if not using 'git cat-file --batch', perhaps it would be easier\n>>> to read each $note_ref tree using 'git ls-tree'/'git ls-tree -r', and\n>>> parse its output to check for which commits/objects there are notes\n>>> available, and only then call 'git show' (or reuse connection to\n>>> 'git cat-file --batch').\n>>>\n>>> The second solution, with a bit more work, could work even in presence\n>>> of fan-out schemes for notes, I think.\n>>\n>> An interesting approach. Without fan-out, git ls-tree -r\n>> refs/notes/whatever [hash ...] gives us the blobs we're interested in.\n>> In case of fan-out schemes, the efficiency of this approach probably\n>> depends on the kind of fan-out we have, and would require some\n>> heavy-duty grepping. A git ls-notes plumbing with a similar syntax and\n>> output would be a nice thing to have.\n>\n> No grepping, just pass '-r' option to 'git-ls-tree', and use\n> parse_ls_tree_line() to parse lines.  Then if we have fanout scheme\n> we would get, I guess, something like the following:\n>\n>  100644 blob 23da787d...       de/adbeef...\n>  100644 blob bc10f25f...       c5/31d986...\n>  100644 blob c9656ece...       24/d93129...\n>\n> Now you only need to s!/!!g on filename to get SHA1 of annotated object\n> (for which is the note).\n\nWhat worries me is that you're going to get fan-outs when there are\nLOTS of notes, and that's precisely the kind of situation where you\n_don't_ want to go through all the notes to pick the ones you're only\ninterested in.\n\nIf we have a guarantee that the fan-outs follow a 2/[2/...] scheme,\nthe open2 approach might still be the best way to go, by just trying\nnot only namespace:xxxxx...xxx but also namespace:xx/xxxxx etc.\nHorrible, but could still be coalesced in a single call. It mgiht also\nbe optimized to stop at the first successfull hit in a namespace.\n\n> P.S. We still would want parse_commit_text to parse notes from default\n> namespace.  parse_commit / parse_commits output contains notes from\n> default namespace, e.g.:\n>\n>  d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3 94ac0c7b30a7dc43d926b0ffbe90892c1e19e5f6\n>  tree b9ee8876df81b80b13c6b017be993fff8427cfaf\n>  parent 94ac0c7b30a7dc43d926b0ffbe90892c1e19e5f6\n>  author Jakub Narebski 1265309578 +0100\n>  committer Jakub Narebski 1265309578 +0100\n>\n>      This is a commit message\n>\n>      Signed-off-by: Jakub Narebski \n>\n>  Notes:\n>      This is just a note for commit d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3\n>\n> to get commit message lines in $co{'comment'} (as array reference),\n> and notes in $co{'note'} (or $co{'notes'}).\n\nI'm not getting these in the repo I'm testing this. And I think this\nis indeed the behavior of current git next\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133840","messageId":"201002070220.36897.jnareb@gmail.com","threadId":"22521","inReplyTo":"cb7bb73a1002061458s5b2c1e7ere83111429473d11c@mail.gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-07T01:20:35Z","receivedAt":"2010-02-07T01:20:35Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:\n> On Sat, Feb 6, 2010 at 11:14 PM, Jakub Narebski wrote:\n>> On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:\n>>> 2010/2/6 Jakub Narebski :\n>>>> On Thu, 4 Feb 2010, Giuseppe Bilotta wrote:\n>>\n>>>>> +     # Notes support. When this feature is enabled, the presence of notes\n>>>>> +     # for any commit is signaled, and the note content is made available\n>>>>> +     # in a way appropriate for the current view.\n>>>>> +     # Set this to '*' to enable all notes namespace, or to a shell-glob\n>>>>> +     # specification to enable specific namespaces only.\n>>>>\n>>>> It is not obvious from this description that you can provide _list_ of\n>>>> notes namespaces (or list of shell-globs).\n>>>\n>>> I'm starting to think it might make sense to not have a list here, but\n>>> rather a single value only. First of all, multiple refs can be\n>>> indicated à la shell with {ref1,ref2,ref3}. Or, we can also use the\n>>> intended command-line syntax ref1:ref2:ref3, which would help\n>>> consistency. It also makes things easier for project overrides, as per\n>>> your subsequent comment:\n>>\n>> So it is to be single shell-glob / fnmatch (I think) compatible pattern,\n>> isn't it?\n> \n> Sort of. fnmatch doesn't do brace expansion, which is a pity IMO, but\n> that's just my personal preference.\n\nWell, fnmatch is what I think git uses for e.g. for \ngit-for-each-ref.\n\n> Colon-separated, fnmatched components is probably the easiest thing to\n> implement to have multiple refs. I'll go with whatever is chosen for\n> core. \n\nI think that having actual list of patterns in $feature{'notes'}{'default'}\nmight be more clear; you would still need colon separated (or space\nseparated) list of patterns in per-repo override in gitweb.notes config\nvariable.\n\nSo it would be\n\n $feature{'notes'}{'default'} = ['commits', '*svn*'];\n $feature{'notes'}{'override'} = 1;\n\nbut\n\n [gitweb]\n \tnotes = commits:*svn*\n \nNote that refs names cannot contain either colon ':' or space ' '\n(see git-check-ref-format).\n\n>>>> First, there are '--batch' and '--batch-check' options to git-cat-file.\n>>>> With these I think you can get all notes with just single git command,\n>>>> although using it is a bit complicated (requires open2 from IPC::Open2\n>>>> for bidi communication).\n>>>\n>>> Hm. The IPC::Open2 doc makes it sound horribly scary, but still doable.\n>>\n>> It would look something like the following (fragment of my WIP code):\n>>\n>>        use IPC::Open2 qw(open2);\n>>        use IO::Handle;\n>>\n>>        # ...\n>>\n>>        unless ($object_stdout) {\n>>                # Open bidi pipe the first time get_object is called.\n>>                # open2 raises an exception on error, no need to 'or die'.\n>>                $object_pid =\n>>                        open2($object_stdout, $object_stdin,\n>>                              git_cmd(), 'cat-file', '--batch');\n>>        }\n>>        $object_stdin->printflush(\"$object_id\\n\") # NB: \\n required to avoid deadlocking\n>>                or die \"get_object: cannot write to pipe: $!\";\n>>        my ($sha1, $type, $size) =\n>>                split ' ', $object_stdout->getline()\n>>                or die \"get_object: cannot read from pipe: $!\";\n>>        die \"'$object_id' not found in repository\"\n>>                if $type eq 'missing';\n>>        $object_stdout->read(my $content, $size);\n>>        $object_stdout->getline();  # eat trailing newline\n>>\n>> The above fragment of code is tested that it works.  You would probably\n>> need to replace dies with something less fatal...\n> \n> On the other hand, as mentioned by Junio, this approach is not\n> future-proof enough for any kind of fan-out schemes.\n\nOn the third hand ;-P you propose below a trick to deal with fan-out\nschemes, assuming that they use 2-character component breaking.\n\nAlso, perhaps \"git notes show\" should acquire --batch / --batch-check\noptions, similar to git-cat-file's options of the same name?\n\n> \n>>>> Second, if not using 'git cat-file --batch', perhaps it would be easier\n>>>> to read each $note_ref tree using 'git ls-tree'/'git ls-tree -r', and\n>>>> parse its output to check for which commits/objects there are notes\n>>>> available, and only then call 'git show' (or reuse connection to\n>>>> 'git cat-file --batch').\n>>>>\n>>>> The second solution, with a bit more work, could work even in presence\n>>>> of fan-out schemes for notes, I think.\n>>>\n>>> An interesting approach. Without fan-out, git ls-tree -r\n>>> refs/notes/whatever [hash ...] gives us the blobs we're interested in.\n>>> In case of fan-out schemes, the efficiency of this approach probably\n>>> depends on the kind of fan-out we have, and would require some\n>>> heavy-duty grepping. A git ls-notes plumbing with a similar syntax and\n>>> output would be a nice thing to have.\n>>\n>> No grepping, just pass '-r' option to 'git-ls-tree', and use\n>> parse_ls_tree_line() to parse lines.  Then if we have fanout scheme\n>> we would get, I guess, something like the following:\n>>\n>>  100644 blob 23da787d...       de/adbeef...\n>>  100644 blob bc10f25f...       c5/31d986...\n>>  100644 blob c9656ece...       24/d93129...\n>>\n>> Now you only need to s!/!!g on filename to get SHA1 of annotated object\n>> (for which is the note).\n> \n> What worries me is that you're going to get fan-outs when there are\n> LOTS of notes, and that's precisely the kind of situation where you\n> _don't_ want to go through all the notes to pick the ones you're only\n> interested in.\n\nRight. This method would be contrary to the goals of fan-out schemes...\nwell, we could use 'git ls-tree' without '-r' option, or simply \n'git cat-file --batch' to read trees (note that we would get raw, \nunformatted tree, which is parseable with Perl, but it is not that easy),\nand go down level-by-level.\n\n> \n> If we have a guarantee that the fan-outs follow a 2/[2/...] scheme,\n> the open2 approach might still be the best way to go, by just trying\n> not only namespace:xxxxx...xxx but also namespace:xx/xxxxx etc.\n> Horrible, but could still be coalesced in a single call. It mgiht also\n> be optimized to stop at the first successfull hit in a namespace.\n\nNice trick! It seems like quite a good idea... but it would absolutely\nrequire using 'git cat-file --batch' rather than one git-show per try.\n\n>> P.S. We still would want parse_commit_text to parse notes from default\n>> namespace.  parse_commit / parse_commits output contains notes from\n>> default namespace, e.g.:\n>>\n>>  d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3 94ac0c7b30a7dc43d926b0ffbe90892c1e19e5f6\n>>  tree b9ee8876df81b80b13c6b017be993fff8427cfaf\n>>  parent 94ac0c7b30a7dc43d926b0ffbe90892c1e19e5f6\n>>  author Jakub Narebski 1265309578 +0100\n>>  committer Jakub Narebski 1265309578 +0100\n>>\n>>      This is a commit message\n>>\n>>      Signed-off-by: Jakub Narebski \n>>\n>>  Notes:\n>>      This is just a note for commit d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3\n>>\n>> to get commit message lines in $co{'comment'} (as array reference),\n>> and notes in $co{'note'} (or $co{'notes'}).\n> \n> I'm not getting these in the repo I'm testing this. And I think this\n> is indeed the behavior of current git next\n\nErrr, I not made myself clear. \n\nI have added a note to a commit, using \"git notes edit d6bbe7f\". Now if\nyou take a look at gitweb output for this commit (e.g. 'commit' view for\nthis commit) using gitweb without your changes, you would see that it\nflattened notes at the bottom of the commit message (which I think is\nintended result by notes implementation).\n\nIf you run the command that parse_commit runs, namely\n\n $ git rev-list --parents --header -z --max-count=1 \\\n d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3\n\nyou would get (up to invisible NUL characters) the output shown above.\nFrom this output I would like to separate commit message from notes\nin parse_commit_text subroutine.\n\nI have set neither GIT_NOTES_REF nor core.notesRef.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"133842","messageId":"201002070238.53527.jnareb@gmail.com","threadId":"22521","inReplyTo":"201002070220.36897.jnareb@gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-07T01:38:52Z","receivedAt":"2010-02-07T01:38:52Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, 7 Feb 2010, Jakub Narebski wrote:\n\n> Right.  This method would be contrary to the goals of fan-out schemes...\n> well, we could use 'git ls-tree' without '-r' option, or simply \n> 'git cat-file --batch' to read trees (note that we would get raw, \n> unformatted tree, which is parseable with Perl, but it is not that easy),\n> and go down level-by-level.\n\nFYI, here is how you can parse raw tree output from 'git cat-file --batch',\nassuming that you have plain-ASCII filenames ('use bytes;' would probably\nbe needed):\n\n-- 8< --\nsub decode_tree {\n\tmy $contents = shift;\n\n\t# ...\n\n\twhile (my @entry = decode_tree_entry($contents)) {\n\n\t\t# ...\n\n\t\tmy $len = tree_entry_len(@entry);\n\t\tcontents = substr($contents, $len);\n\t\tlast unless $contents;\n\t}\n\n\t# ...\n}\n\nsub tree_entry_len {\n\tmy ($mode_str, $filename) = @_;\n\n\t# length of mode string + separator + 20 bytes of SHA-1\n\t# + length of filename (in bytes) + terminating NUL ('\\0')\n\tlength($mode_str)+1 + length($filename)+1 + 20;\n}\n\nsub decode_tree_entry {\n\tmy $buf = shift;\n\n\t$buf =~ s/^([0-7]+) //;\n\tmy ($mode_str) = $1;\n\tmy ($filename, $sha1_str) = unpack('Z*H[40]', $buf);\n\n\treturn ($mode_str, $filename, $sha1_str);\n}\n-- >8 --\n\n-- \nJakub Narebski\nPoland\n"},{"id":"133843","messageId":"201002070248.03855.johan@herland.net","threadId":"22521","inReplyTo":"201002070220.36897.jnareb@gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2010-02-07T01:48:03Z","receivedAt":"2010-02-07T01:48:03Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Sunday 07 February 2010, Jakub Narebski wrote:\n> On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:\n> > On the other hand, as mentioned by Junio, this approach is not\n> > future-proof enough for any kind of fan-out schemes.\n> \n> On the third hand ;-P you propose below a trick to deal with fan-out\n> schemes, assuming that they use 2-character component breaking.\n\nThe current notes code (as it stands in 'pu') use only 2-character component \nbreaking, and I don't see any other fanout mechanism being added anytime \nsoon.\n\n> Also, perhaps \"git notes show\" should acquire --batch / --batch-check\n> options, similar to git-cat-file's options of the same name?\n\nI'd much rather have support for ^{notes} (or similar) in the rev-parse \nmachinery, so that you could look up deadbeef's notes by passing \n\"deadbeef^{notes}\" to 'git cat-file --batch'.\n\n> > What worries me is that you're going to get fan-outs when there are\n> > LOTS of notes, and that's precisely the kind of situation where you\n> > _don't_ want to go through all the notes to pick the ones you're only\n> > interested in.\n> \n> Right. This method would be contrary to the goals of fan-out schemes...\n> well, we could use 'git ls-tree' without '-r' option, or simply\n> 'git cat-file --batch' to read trees (note that we would get raw,\n> unformatted tree, which is parseable with Perl, but it is not that easy),\n> and go down level-by-level.\n\nIMHO, it's much better/nicer to re-use the notes code for parsing note \ntrees. See above suggestion on deadbeef^{notes}.\n\n> > If we have a guarantee that the fan-outs follow a 2/[2/...] scheme,\n> > the open2 approach might still be the best way to go, by just trying\n> > not only namespace:xxxxx...xxx but also namespace:xx/xxxxx etc.\n> > Horrible, but could still be coalesced in a single call. It mgiht also\n> > be optimized to stop at the first successfull hit in a namespace.\n> \n> Nice trick! It seems like quite a good idea... but it would absolutely\n> require using 'git cat-file --batch' rather than one git-show per try.\n\nStill, I'd still much rather use the notes.c code itself for doing this \nsince it should always be the fastest (not to mention future-proof) way of \nmaking lookups in the notes tree.\n\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"133865","messageId":"cb7bb73a1002070257g8f1e59dvf124c8966cdf3270@mail.gmail.com","threadId":"22521","inReplyTo":"201002070220.36897.jnareb@gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-07T10:57:58Z","receivedAt":"2010-02-07T10:57:58Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Sun, Feb 7, 2010 at 2:20 AM, Jakub Narebski wrote:\n> On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:\n>> On Sat, Feb 6, 2010 at 11:14 PM, Jakub Narebski wrote:\n>>> So it is to be single shell-glob / fnmatch (I think) compatible pattern,\n>>> isn't it?\n>>\n>> Sort of. fnmatch doesn't do brace expansion, which is a pity IMO, but\n>> that's just my personal preference.\n>\n> Well, fnmatch is what I think git uses for e.g. for\n> git-for-each-ref.\n\nfnmatch is the function to use to match a single component. The\nquestion is how to group components together in a single spec (aside\nfrom shell globs and []); my personal choice would be brace expansion,\ncolon-separated was suggested by (IIRC) Junio. Of course, for gitweb\nI'll go with whatever is chosen for core.\n\n>> Colon-separated, fnmatched components is probably the easiest thing to\n>> implement to have multiple refs. I'll go with whatever is chosen for\n>> core.\n>\n> I think that having actual list of patterns in $feature{'notes'}{'default'}\n> might be more clear; you would still need colon separated (or space\n> separated) list of patterns in per-repo override in gitweb.notes config\n> variable.\n>\n> So it would be\n>\n>  $feature{'notes'}{'default'} = ['commits', '*svn*'];\n>  $feature{'notes'}{'override'} = 1;\n>\n> but\n>\n>  [gitweb]\n>        notes = commits:*svn*\n>\n> Note that refs names cannot contain either colon ':' or space ' '\n> (see git-check-ref-format).\n\nThat makes sense. Colon-separated values will probably be allowed in\n$feature{'notes'} too, which will keep the code more consistent.\n\n[...]\n\n>>> The above fragment of code is tested that it works.  You would probably\n>>> need to replace dies with something less fatal...\n>>\n>> On the other hand, as mentioned by Junio, this approach is not\n>> future-proof enough for any kind of fan-out schemes.\n>\n> On the third hand ;-P you propose below a trick to deal with fan-out\n> schemes, assuming that they use 2-character component breaking.\n>\n> Also, perhaps \"git notes show\" should acquire --batch / --batch-check\n> options, similar to git-cat-file's options of the same name?\n\nThere is little doubt that batch note processing should be available\nin core, be it with git notes show --batch(-check), git ls-notes, or\nboth. I also agree with Johan that gitweb should use these mechanisms\nas soon as they are available. The approaches we're discussing here\nare basically a temporary workaround for the missing core support.\n\n[...]\n\n>> If we have a guarantee that the fan-outs follow a 2/[2/...] scheme,\n>> the open2 approach might still be the best way to go, by just trying\n>> not only namespace:xxxxx...xxx but also namespace:xx/xxxxx etc.\n>> Horrible, but could still be coalesced in a single call. It mgiht also\n>> be optimized to stop at the first successfull hit in a namespace.\n>\n> Nice trick!  It seems like quite a good idea... but it would absolutely\n> require using 'git cat-file --batch' rather than one git-show per try.\n\nOh, that's a given. git-show was just the only approach I could think\nof not knowing about the batch processing git commands.\n\n>> I'm not getting these in the repo I'm testing this. And I think this\n>> is indeed the behavior of current git next\n>\n> Errr, I not made myself clear.\n>\n> I have added a note to a commit, using \"git notes edit d6bbe7f\".  Now if\n> you take a look at gitweb output for this commit (e.g. 'commit' view for\n> this commit) using gitweb without your changes, you would see that it\n> flattened notes at the bottom of the commit message (which I think is\n> intended result by notes implementation).\n>\n> If you run the command that parse_commit runs, namely\n>\n>  $ git rev-list --parents --header -z --max-count=1 \\\n>    d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3\n>\n> you would get (up to invisible NUL characters) the output shown above.\n\nAnd that's what I don't get. git version 1.7.0.rc1.193.ge8618. If I\nremember correctly, the behaviour of automatically displaying notes in\ngit log & friends was changed recently. So git log -1\ne8618c52b5f815624251f048609744c9558d92a1 gives me the notes I put\nthere for testing, git rev-list --parents --header -z --max-count=1\ndoes not.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133866","messageId":"201002071208.44705.jnareb@gmail.com","threadId":"22521","inReplyTo":"201002070248.03855.johan@herland.net","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-07T11:08:44Z","receivedAt":"2010-02-07T11:08:44Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, 7 February 2010, Johan Herland wrote:\n> On Sunday 07 February 2010, Jakub Narebski wrote:\n> > On Sat, 6 Feb 2010, Giuseppe Bilotta wrote:\n\n> > > On the other hand, as mentioned by Junio, this approach is not\n> > > future-proof enough for any kind of fan-out schemes.\n> > \n> > On the third hand ;-P you propose below a trick to deal with fan-out\n> > schemes, assuming that they use 2-character component breaking.\n> \n> The current notes code (as it stands in 'pu') use only 2-character component \n> breaking, and I don't see any other fanout mechanism being added anytime \n> soon.\n\nOn one hand checking :403f3b3e..., then :40/3f3b3e...,\nthen :40/3f/3b3e... etc. feels a bit kludgy...\n\nOn the other hand it would allow to support notes in gitweb even if git\nbinary does not have notes support (and yet notes did get somehow into\nrepository).\n \n> > Also, perhaps \"git notes show\" should acquire --batch / --batch-check\n> > options, similar to git-cat-file's options of the same name?\n> \n> I'd much rather have support for ^{notes} (or similar) in the rev-parse \n> machinery, so that you could look up deadbeef's notes by passing \n> \"deadbeef^{notes}\" to 'git cat-file --batch'.\n\n+1. Good idea!\n\nThe only caveat is that if/when we support either of:\n* allowing to specify multiple notes namespaces (e.g. multiple --notes-ref\n or --notes option, or PATH-like GIT_NOTES_REF with colon-separated list\n of multiple notes namespaces)\n* allowing to have multiple notes per object ('tree' notes)\nthen ^{notes} would mean multiple objects. But it is not\nmuch different from supported ^! and ^@ syntax.\n\n[...]\n> > > If we have a guarantee that the fan-outs follow a 2/[2/...] scheme,\n> > > the open2 approach might still be the best way to go, by just trying\n> > > not only namespace:xxxxx...xxx but also namespace:xx/xxxxx etc.\n> > > Horrible, but could still be coalesced in a single call. It mgiht also\n> > > be optimized to stop at the first successfull hit in a namespace.\n> > \n> > Nice trick! It seems like quite a good idea... but it would absolutely\n> > require using 'git cat-file --batch' rather than one git-show per try.\n> \n> Still, I'd still much rather use the notes.c code itself for doing this \n> since it should always be the fastest (not to mention future-proof) way of \n> making lookups in the notes tree.\n\nOne of Giuseppe goals seems to be to support notes in gitweb even if used\ngit binary doesn't have support for notes (git-notes command, ^{notes}\nextended SHA1 syntax).\n\n-- \nJakub Narebski\nPoland\n"},{"id":"133868","messageId":"201002071211.36440.jnareb@gmail.com","threadId":"22521","inReplyTo":"cb7bb73a1002070257g8f1e59dvf124c8966cdf3270@mail.gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-07T11:11:35Z","receivedAt":"2010-02-07T11:11:35Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Giuseppe Bilotta wrote:\n> On Sun, Feb 7, 2010 at 2:20 AM, Jakub Narebski wrote:\n\n> > Errr, I not made myself clear.\n> >\n> > I have added a note to a commit, using \"git notes edit d6bbe7f\".  Now if\n> > you take a look at gitweb output for this commit (e.g. 'commit' view for\n> > this commit) using gitweb without your changes, you would see that it\n> > flattened notes at the bottom of the commit message (which I think is\n> > intended result by notes implementation).\n> >\n> > If you run the command that parse_commit runs, namely\n> >\n> >  $ git rev-list --parents --header -z --max-count=1 \\\n> >    d6bbe7fd52058cdf0e48bec00701ae0f4861dcd3\n> >\n> > you would get (up to invisible NUL characters) the output shown above.\n> \n> And that's what I don't get. git version 1.7.0.rc1.193.ge8618. If I\n> remember correctly, the behaviour of automatically displaying notes in\n> git log & friends was changed recently. So git log -1\n> e8618c52b5f815624251f048609744c9558d92a1 gives me the notes I put\n> there for testing, git rev-list --parents --header -z --max-count=1\n> does not.\n\nAh, sorry. Git version 1.6.6.1 here.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"133867","messageId":"cb7bb73a1002070314t4f382d31k91423eac00a68715@mail.gmail.com","threadId":"22521","inReplyTo":"201002070248.03855.johan@herland.net","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-07T11:14:19Z","receivedAt":"2010-02-07T11:14:19Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Sun, Feb 7, 2010 at 2:48 AM, Johan Herland wrote:\n> On Sunday 07 February 2010, Jakub Narebski wrote:\n>\n>> Also, perhaps \"git notes show\" should acquire --batch / --batch-check\n>> options, similar to git-cat-file's options of the same name?\n>\n> I'd much rather have support for ^{notes} (or similar) in the rev-parse\n> machinery, so that you could look up deadbeef's notes by passing\n> \"deadbeef^{notes}\" to 'git cat-file --batch'.\n\nMaybe something like deadbeef@{notes[:namespace]}? The ability to\nembed the notes namespace to use in the call is very useful to be able\nto access all the notes with a single git call.\n\n>> > If we have a guarantee that the fan-outs follow a 2/[2/...] scheme,\n>> > the open2 approach might still be the best way to go, by just trying\n>> > not only namespace:xxxxx...xxx but also namespace:xx/xxxxx etc.\n>> > Horrible, but could still be coalesced in a single call. It mgiht also\n>> > be optimized to stop at the first successfull hit in a namespace.\n>>\n>> Nice trick!  It seems like quite a good idea... but it would absolutely\n>> require using 'git cat-file --batch' rather than one git-show per try.\n>\n> Still, I'd still much rather use the notes.c code itself for doing this\n> since it should always be the fastest (not to mention future-proof) way of\n> making lookups in the notes tree.\n\nI agree with you on this, btw. As I mentioned in the other message,\nthese would just be workarounds for the current lack of support of\nthese features in core. I'd probably try and have a go at the thing\nmyself, too (i.e. first implement the core functionality, and then use\nit in gitweb), but I honestly don't feel confident enough to hack at\ngit core.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133871","messageId":"201002071341.36440.jnareb@gmail.com","threadId":"22521","inReplyTo":"cb7bb73a1002070314t4f382d31k91423eac00a68715@mail.gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-07T12:41:35Z","receivedAt":"2010-02-07T12:41:35Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, 7 Feb 2010, Giuseppe Bilotta wrote:\n> On Sun, Feb 7, 2010 at 2:48 AM, Johan Herland wrote:\n> > On Sunday 07 February 2010, Jakub Narebski wrote:\n> >\n> > > Also, perhaps \"git notes show\" should acquire --batch / --batch-check\n> > > options, similar to git-cat-file's options of the same name?\n> >\n> > I'd much rather have support for ^{notes} (or similar) in the rev-parse\n> > machinery, so that you could look up deadbeef's notes by passing\n> > \"deadbeef^{notes}\" to 'git cat-file --batch'.\n> \n> Maybe something like deadbeef@{notes[:namespace]}? The ability to\n> embed the notes namespace to use in the call is very useful to be able\n> to access all the notes with a single git call.\n\nThat is just bikeshedding, but I'd rather not use '@', which currently\nis used only for _reflog_ based revision specifiers: []@{},\n[]@{}, @{-}, for notes which are not reflog based.\n\nWe can use\n\n echo ^{notes} | git --notes-ref= cat-file --batch\n\nor perhaps\n\n echo ^{notes:} | git cat-file --batch\n\n-- \nJakub Narebski\nPoland\n"},{"id":"133879","messageId":"7vock0evs8.fsf@alter.siamese.dyndns.org","threadId":"22521","inReplyTo":"201002071341.36440.jnareb@gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-07T18:38:15Z","receivedAt":"2010-02-07T18:38:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n> That is just bikeshedding, but I'd rather not use '@', which currently\n> is used only for _reflog_ based revision specifiers: []@{},\n> []@{}, @{-}, for notes which are not reflog based.\n\nProbably a nicer way to say the same thing is to avoid \"reflog based\"\nwhich sounds like you are talking about an implementation detail.\n\nA fundamental reason to favor your \"bikeshedding\" (I don't think it is a\nbikeshedding---it is a sound argument against using \"@{...}\") is that the\nat-brace notation applies to a ref, not to an arbitrary commit. Applying\n@{yesterday} to an arbitrary commit does not make any sense.\n\nNotes are fundamenally metainformation about an _object_ [*1*] and are not\nmetainformation about refs. Since whatever magic notation to denote notes\nwe choose wants to be applied to an arbitrary commit, it shouldn't be the\nat-brace syntax.\n\n[Footnote]\n\n*1* Yes, I am aware of movements to misuse notes to annotate anything\nafter mapping it to a random SHA-1 value, but I think that is outside the\nscope of notes. Our design decision should be based on supporting the\nprimary use of annotating an object, and that might still keep such a use\nworking, in which case that would be an added bonus. But our design\nshouldn't be constrained by such a secondary use.\n"},{"id":"133885","messageId":"cb7bb73a1002071211n23a410f9icc3d4d29179c44ca@mail.gmail.com","threadId":"22521","inReplyTo":"7vock0evs8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2010-02-07T20:11:32Z","receivedAt":"2010-02-07T20:11:32Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Sun, Feb 7, 2010 at 7:38 PM, Junio C Hamano wrote:\n>\n> Notes are fundamenally metainformation about an _object_ [*1*] and are not\n> metainformation about refs.  Since whatever magic notation to denote notes\n> we choose wants to be applied to an arbitrary commit, it shouldn't be the\n> at-brace syntax.\n\nMakes sense. ^{note[:namespace]} is ok for me too btw, although maybe\nit looks a little off-base when compared with the tag indicator ^{}\nwhich works, in a sense, in the opposite direction.\n\n> [Footnote]\n>\n> *1* Yes, I am aware of movements to misuse notes to annotate anything\n> after mapping it to a random SHA-1 value, but I think that is outside the\n> scope of notes.  Our design decision should be based on supporting the\n> primary use of annotating an object, and that might still keep such a use\n> working, in which case that would be an added bonus.  But our design\n> shouldn't be constrained by such a secondary use.\n\n\nBTW, I still think that notes should be attachable to named refs (not\nSHA-1, thus) too.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"133890","messageId":"201002072208.06627.jnareb@gmail.com","threadId":"22521","inReplyTo":"cb7bb73a1002071211n23a410f9icc3d4d29179c44ca@mail.gmail.com","subject":"Re: [PATCH 1/4] gitweb: notes feature","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-02-07T21:08:05Z","receivedAt":"2010-02-07T21:08:05Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, 7 Feb 2010, Giuseppe Bilotta wrote:\n> On Sun, Feb 7, 2010 at 7:38 PM, Junio C Hamano wrote:\n> >\n> > Notes are fundamenally metainformation about an _object_ [*1*] and are not\n> > metainformation about refs.  Since whatever magic notation to denote notes\n> > we choose wants to be applied to an arbitrary commit, it shouldn't be the\n> > at-brace syntax.\n> \n> Makes sense. ^{note[:namespace]} is ok for me too btw, although maybe\n> it looks a little off-base when compared with the tag indicator ^{}\n> which works, in a sense, in the opposite direction.\n\nWell, notes refer to objects (commits), but the whole idea of notes\nwas to have easy mapping in the reverse direction, from object to\nits annotations.\n\nWe could invent yet another syntax, e.g. ^@{} or ^@{}\n(following ^@ notation for parents, which can also return multiple SHA1s).\n\n> > [Footnote]\n> >\n> > *1* Yes, I am aware of movements to misuse notes to annotate anything\n> > after mapping it to a random SHA-1 value, but I think that is outside the\n> > scope of notes.  Our design decision should be based on supporting the\n> > primary use of annotating an object, and that might still keep such a use\n> > working, in which case that would be an added bonus.  But our design\n> > shouldn't be constrained by such a secondary use.\n> \n> BTW, I still think that notes should be attachable to named refs (not\n> SHA-1, thus) too.\n\nI have just realized that it is totally no-go. Why? Because names of\nrefs are local to repository: what is one 'master' might be other 'origin';\nwhat is one 'for-linus' might be other 'from-alan', what's one \n'refs/heads/next' might be other 'refs/remotes/origin/next'.\n\nAlso I think that there would be problem with renaming and deleting refs.\n\n-- \nJakub Narebski\nPoland\n"}]}