{"thread":{"id":"16519","subject":"[PATCH 0/2] gitweb: patch view","startedAt":"2008-11-29T13:41:09Z","lastAt":"2008-12-03T09:25:22Z","messageCount":13,"participants":["Giuseppe Bilotta","Sverre Rabbelier","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"96720","messageId":"1227966071-11104-1-git-send-email-giuseppe.bilotta@gmail.com","threadId":"16519","inReplyTo":null,"subject":"[PATCH 0/2] gitweb: patch view","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-11-29T13:41:09Z","receivedAt":"2008-11-29T13:41:09Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"I recently discovered that the commitdiff_plain view is not exactly\nsomething that can be used by git am directly (for example, the subject\nline gets duplicated in the commit message body after using git am).\n\nSince I'm not sure if it was the case to fix the plain view because I\ndon't know what its intended usage was, I prepared a new view,\nuncreatively called 'patch', that exposes git format-patch output\ndirectly.\n\nThe second patch exposes it from commitdiff view (obviosly), but also\nfrom shortlog view, when less than 16 patches are begin shown.\n\nGiuseppe Bilotta (2):\n  gitweb: add patch view\n  gitweb: links to patch action in commitdiff and shortlog view\n\n gitweb/gitweb.perl |   35 +++++++++++++++++++++++++++++++++--\n 1 files changed, 33 insertions(+), 2 deletions(-)\n"},{"id":"96722","messageId":"1227966071-11104-2-git-send-email-giuseppe.bilotta@gmail.com","threadId":"16519","inReplyTo":"1227966071-11104-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCH 1/2] gitweb: add patch view","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-11-29T13:41:10Z","receivedAt":"2008-11-29T13:41:10Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Trying to use 'commitdiff_plain' output as input to git am results in\nsome annoying results such as doubled subject lines. We thus offer a new\n'patch' view that exposes format-patch output directly. This makes it\neasier to offer patches by linking to gitweb repositories.\n---\n gitweb/gitweb.perl |   25 ++++++++++++++++++++++++-\n 1 files changed, 24 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 933e137..befe6b6 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -485,6 +485,7 @@ our %actions = (\n \t\"heads\" => \\&git_heads,\n \t\"history\" => \\&git_history,\n \t\"log\" => \\&git_log,\n+\t\"patch\" => \\&git_patch,\n \t\"rss\" => \\&git_rss,\n \t\"atom\" => \\&git_atom,\n \t\"search\" => \\&git_search,\n@@ -5465,7 +5466,11 @@ sub git_commitdiff {\n \t\topen $fd, \"-|\", git_cmd(), \"diff-tree\", '-r', @diff_opts,\n \t\t\t'-p', $hash_parent_param, $hash, \"--\"\n \t\t\tor die_error(500, \"Open git-diff-tree failed\");\n-\n+\t} elsif ($format eq 'patch') {\n+\t\topen $fd, \"-|\", git_cmd(), \"format-patch\", '--stdout',\n+\t\t\t$hash_parent ? \"$hash_parent..$hash\" :\n+\t\t\t('--root', '-1', $hash)\n+\t\t\tor die_error(500, \"Open git-format-patch failed\");\n \t} else {\n \t\tdie_error(400, \"Unknown commitdiff format\");\n \t}\n@@ -5514,6 +5519,14 @@ sub git_commitdiff {\n \t\t\tprint to_utf8($line) . \"\\n\";\n \t\t}\n \t\tprint \"---\\n\\n\";\n+\t} elsif ($format eq 'patch') {\n+\t\tmy $filename = basename($project) . \"-$hash.patch\";\n+\n+\t\tprint $cgi->header(\n+\t\t\t-type => 'text/plain',\n+\t\t\t-charset => 'utf-8',\n+\t\t\t-expires => $expires,\n+\t\t\t-content_disposition => 'inline; filename=\"' . \"$filename\" . '\"');\n \t}\n \n \t# write patch\n@@ -5535,6 +5548,11 @@ sub git_commitdiff {\n \t\tprint <$fd>;\n \t\tclose $fd\n \t\t\tor print \"Reading git-diff-tree failed\\n\";\n+\t} elsif ($format eq 'patch') {\n+\t\tlocal $/ = undef;\n+\t\tprint <$fd>;\n+\t\tclose $fd\n+\t\t\tor print \"Reading git-format-patch failed\\n\";\n \t}\n }\n \n@@ -5542,6 +5560,11 @@ sub git_commitdiff_plain {\n \tgit_commitdiff('plain');\n }\n \n+# format-patch-style patches\n+sub git_patch {\n+\tgit_commitdiff('patch');\n+}\n+\n sub git_history {\n \tif (!defined $hash_base) {\n \t\t$hash_base = git_get_head_hash($project);\n-- \n1.5.6.5\n"},{"id":"96721","messageId":"1227966071-11104-3-git-send-email-giuseppe.bilotta@gmail.com","threadId":"16519","inReplyTo":"1227966071-11104-2-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCH 2/2] gitweb: links to patch action in commitdiff and shortlog view","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-11-29T13:41:11Z","receivedAt":"2008-11-29T13:41:11Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"The link from commitdiff view is an obviously needed one, but we also\noffer the option to link to the patchset in shortlog view, when there\nare less than 15 commits being shown.\n---\n gitweb/gitweb.perl |   10 +++++++++-\n 1 files changed, 9 insertions(+), 1 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex befe6b6..5b18fdf 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -5382,7 +5382,9 @@ sub git_commitdiff {\n \tif ($format eq 'html') {\n \t\t$formats_nav =\n \t\t\t$cgi->a({-href => href(action=>\"commitdiff_plain\", -replay=>1)},\n-\t\t\t        \"raw\");\n+\t\t\t        \"raw\") . \" | \" .\n+\t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n+\t\t\t        \"patch\");\n \n \t\tif (defined $hash_parent &&\n \t\t    $hash_parent ne '-c' && $hash_parent ne '--cc') {\n@@ -5915,6 +5917,12 @@ sub git_shortlog {\n \t\t\t$cgi->a({-href => href(-replay=>1, page=>$page+1),\n \t\t\t         -accesskey => \"n\", -title => \"Alt-n\"}, \"next\");\n \t}\n+\t# TODO this should be configurable\n+\tif ($#commitlist <= 15) {\n+\t\t$paging_nav .= \" &sdot; \" .\n+\t\t\t$cgi->a({-href => href(action=>\"patch\", -replay=>1)},\n+\t\t\t        $#commitlist > 1 ? \"patchset\" : \"patch\");\n+\t}\n \n \tgit_header_html();\n \tgit_print_page_nav('shortlog','', $hash,$hash,$hash, $paging_nav);\n-- \n1.5.6.5\n"},{"id":"96725","messageId":"bd6139dc0811290743s6cf8e534nddd8a09698ea22b9@mail.gmail.com","threadId":"16519","inReplyTo":"1227966071-11104-2-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCH 1/2] gitweb: add patch view","fromName":"Sverre Rabbelier","fromEmail":"alturin@gmail.com","sentAt":"2008-11-29T15:43:51Z","receivedAt":"2008-11-29T15:43:51Z","isPatch":true,"sender":{"key":"alturin@gmail.com","avatar":null},"body":"On Sat, Nov 29, 2008 at 14:41, Giuseppe Bilotta\n<giuseppe.bilotta@gmail.com> wrote:\n> Trying to use 'commitdiff_plain' output as input to git am results in\n> some annoying results such as doubled subject lines. We thus offer a new\n> 'patch' view that exposes format-patch output directly. This makes it\n> easier to offer patches by linking to gitweb repositories.\n\nIf this does what I think it does I would be very happy with this\nfeature :). Only yesterday I wanted to link someone to a patch I put\nup on repo.or.cz, but instead ended up telling them to download the\nsnapshot.\n\nAs an additional feature request; would it be possible to have a view\nthat exposes a patch that is directly applyable by the patch command?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"96726","messageId":"200811291710.27891.jnareb@gmail.com","threadId":"16519","inReplyTo":"bd6139dc0811290743s6cf8e534nddd8a09698ea22b9@mail.gmail.com","subject":"Re: [PATCH 1/2] gitweb: add patch view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-11-29T16:10:26Z","receivedAt":"2008-11-29T16:10:26Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 29 Nov 2008, Sverre Rabbelier wrote:\n> On Sat, Nov 29, 2008 at 14:41, Giuseppe Bilotta\n> <giuseppe.bilotta@gmail.com> wrote:\n\n> > Trying to use 'commitdiff_plain' output as input to git am results in\n> > some annoying results such as doubled subject lines. We thus offer a new\n> > 'patch' view that exposes format-patch output directly. This makes it\n> > easier to offer patches by linking to gitweb repositories.\n> \n> If this does what I think it does I would be very happy with this\n> feature :). Only yesterday I wanted to link someone to a patch I put\n> up on repo.or.cz, but instead ended up telling them to download the\n> snapshot.\n\nTrue. 'commitdiff_plain' wasn't good enough; what's more it suffers\nfrom the same ambiguity as 'commitdiff', i.e. it is both means to\nshow diff _for_ a commit (perhaps selecting one of parents), and\nshowing diff _between_ two commits; the new 'patch' always shows\ndiff for a commit, or for a series of commits.\n\n>\n> As an additional feature request; would it be possible to have a view\n> that exposes a patch that is directly applyable by the patch command?\n\nBoth new 'patch' ('patchset') and old 'blobdiff_plain' should be\ndirectly applicable by patch and by git-apply.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"96727","messageId":"cb7bb73a0811290848j1b77fe89m66ead7cc4f5ca2bb@mail.gmail.com","threadId":"16519","inReplyTo":"200811291710.27891.jnareb@gmail.com","subject":"Re: [PATCH 1/2] gitweb: add patch view","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-11-29T16:48:13Z","receivedAt":"2008-11-29T16:48:13Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Wow, I honestly didn't expect this idea to be so successful. I thought\nI was the only one using gitweb to send patches around, honestly 8-D\n\nOn Sat, Nov 29, 2008 at 5:10 PM, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Sat, 29 Nov 2008, Sverre Rabbelier wrote:\n>>\n>> If this does what I think it does I would be very happy with this\n>> feature :). Only yesterday I wanted to link someone to a patch I put\n>> up on repo.or.cz, but instead ended up telling them to download the\n>> snapshot.\n>\n> True. 'commitdiff_plain' wasn't good enough; what's more it suffers\n> from the same ambiguity as 'commitdiff', i.e. it is both means to\n> show diff _for_ a commit (perhaps selecting one of parents), and\n> showing diff _between_ two commits; the new 'patch' always shows\n> diff for a commit, or for a series of commits.\n\nMaybe commitdiff should me renamed to just be 'diff'.\n\nAlso, I was in doubt about the name for the new view, and I did\nconsider 'patchset' (which you mention in your email). I chose to\nstick with the shorter form in the end, since many people complain\nthat gitweb already produces paths that are too long.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"96729","messageId":"bd6139dc0811290850r404da348yda5dd5f1eb5dc95c@mail.gmail.com","threadId":"16519","inReplyTo":"cb7bb73a0811290848j1b77fe89m66ead7cc4f5ca2bb@mail.gmail.com","subject":"Re: [PATCH 1/2] gitweb: add patch view","fromName":"Sverre Rabbelier","fromEmail":"alturin@gmail.com","sentAt":"2008-11-29T16:50:46Z","receivedAt":"2008-11-29T16:50:46Z","isPatch":true,"sender":{"key":"alturin@gmail.com","avatar":null},"body":"On Sat, Nov 29, 2008 at 17:48, Giuseppe Bilotta\n<giuseppe.bilotta@gmail.com> wrote:\n> Also, I was in doubt about the name for the new view, and I did\n> consider 'patchset' (which you mention in your email). I chose to\n> stick with the shorter form in the end, since many people complain\n> that gitweb already produces paths that are too long.\n\nHeh, what do I care about the length of the url, there's so many url\nshorteners out there, that's not really a problem to me :).\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"96752","messageId":"200811300206.23240.jnareb@gmail.com","threadId":"16519","inReplyTo":"1227966071-11104-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCH 0/2] gitweb: patch view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-11-30T01:06:21Z","receivedAt":"2008-11-30T01:06:21Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 29 Nov 2008, Giuseppe Bilotta wrote:\n\n> I recently discovered that the commitdiff_plain view is not exactly\n> something that can be used by git am directly (for example, the subject\n> line gets duplicated in the commit message body after using git am).\n\nThat's because gitweb generates email-like format \"by hand\", instead\nof using '--format=email' or git-format-patch like in your series. On\nthe other hand that allows us to add extra headers, namely X-Git-Tag:\n(which hasn't best implementation, anyway) and X-Git-Url: with URL\nfor given output.\n \n> Since I'm not sure if it was the case to fix the plain view because I\n> don't know what its intended usage was, I prepared a new view,\n> uncreatively called 'patch', that exposes git format-patch output\n> directly.\n\nPerhaps 'format_patch' would be better... hmmm... ?\n\nActually IMHO both 'commitdiff' and 'commitdiff_plain' try to do two\nthings at once. First to show diff _for_ a commit, i.e. equivalent of\n\"git show\" or \"git show --pretty=email\", perhaps choosing one of\nparents for a merge commit. Then showing commit message for $hash has\nsense. The fact that 'commit' view doesn't show patchset, while\n'commitdiff' does might be result of historical situation.\n\nSecond, to show diff _between_ commits, i.e. equivalent of \n\"git diff branch master\". Then there doesn't make much sense to show\nfull commit message _only_ for one side of diff. IMHO that should be\nmain purpose of 'commitdiff' and 'commitdiff_plain' views, or simply\n'diff' / 'diff_plain' future views.\n\n\nWhat 'patch' view does, what might be not obvious from this description\nand from first patch in series, is to show diffs for _series_ of\ncommits. It means equivalent of \"git log -p\" or \"git whatchanged\".\nIt might make more sense to have plain git-format-patch output, but it\ncould be useful to have some kind of 'git log -p' HTML output.\n\nSo even if 'commitdiff' / 'commitdiff_plain' is fixed, 'patch' whould\nstill have its place.\n\n\nBy the way, we still might want to add somehow X-Git-Url and X-Git-Tag\nheaders later to 'patch' ('patchset') output format.\n\n> \n> The second patch exposes it from commitdiff view (obviosly), but also\n> from shortlog view, when less than 16 patches are begin shown.\n\nWhy this nonconfigurable limit?\n\n> \n> Giuseppe Bilotta (2):\n>   gitweb: add patch view\n>   gitweb: links to patch action in commitdiff and shortlog view\n> \n>  gitweb/gitweb.perl |   35 +++++++++++++++++++++++++++++++++--\n>  1 files changed, 33 insertions(+), 2 deletions(-)\n\nThank you for your work on gitweb\n-- \nJakub Narebski\nPoland\n"},{"id":"96756","messageId":"cb7bb73a0811291744t2bb9c8c1t1dac497705e2c3c2@mail.gmail.com","threadId":"16519","inReplyTo":"200811300206.23240.jnareb@gmail.com","subject":"Re: [PATCH 0/2] gitweb: patch view","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-11-30T01:44:38Z","receivedAt":"2008-11-30T01:44:38Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Sun, Nov 30, 2008 at 2:06 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Sat, 29 Nov 2008, Giuseppe Bilotta wrote:\n>\n>> I recently discovered that the commitdiff_plain view is not exactly\n>> something that can be used by git am directly (for example, the subject\n>> line gets duplicated in the commit message body after using git am).\n>\n> That's because gitweb generates email-like format \"by hand\", instead\n> of using '--format=email' or git-format-patch like in your series. On\n> the other hand that allows us to add extra headers, namely X-Git-Tag:\n> (which hasn't best implementation, anyway) and X-Git-Url: with URL\n> for given output.\n\n> By the way, we still might want to add somehow X-Git-Url and X-Git-Tag\n> headers later to 'patch' ('patchset') output format.\n\nYeah, I've been thinking about it, but I couldn't find an easy and\nrobust way to do it. Plus, should we add them for each patch, or just\nonce for the whole patchset?\n\n>> Since I'm not sure if it was the case to fix the plain view because I\n>> don't know what its intended usage was, I prepared a new view,\n>> uncreatively called 'patch', that exposes git format-patch output\n>> directly.\n>\n> Perhaps 'format_patch' would be better... hmmm... ?\n\nConsidering I think commitdiff is ugly and long, you can guess my\nopinion on format_patch 8-P. 'patchset' might be a good candidate,\nconsidering it's what it does when both hash_parent and hash are\ngiven.\n\n> Actually IMHO both 'commitdiff' and 'commitdiff_plain' try to do two\n> things at once. First to show diff _for_ a commit, i.e. equivalent of\n> \"git show\" or \"git show --pretty=email\", perhaps choosing one of\n> parents for a merge commit. Then showing commit message for $hash has\n> sense. The fact that 'commit' view doesn't show patchset, while\n> 'commitdiff' does might be result of historical situation.\n>\n> Second, to show diff _between_ commits, i.e. equivalent of\n> \"git diff branch master\". Then there doesn't make much sense to show\n> full commit message _only_ for one side of diff. IMHO that should be\n> main purpose of 'commitdiff' and 'commitdiff_plain' views, or simply\n> 'diff' / 'diff_plain' future views.\n\nWe can probably consider deprecating commitdiff(_plain) and have the\nfollowing three views:\n\n* commit(_plain): do what commitdiff(_plain) currently does for a single commit\n* diff(_plain): do what commitdiff(_plain) currently does for\nparent..hash views, modulo something to be discussed for commit\nmessages (a shortlog rather maybe?)\n* patch[set?][_plain?]: format-patch style output (maybe with option\nfor HTML stuff too)\n\n> What 'patch' view does, what might be not obvious from this description\n> and from first patch in series, is to show diffs for _series_ of\n> commits. It means equivalent of \"git log -p\" or \"git whatchanged\".\n> It might make more sense to have plain git-format-patch output, but it\n> could be useful to have some kind of 'git log -p' HTML output.\n>\n> So even if 'commitdiff' / 'commitdiff_plain' is fixed, 'patch' whould\n> still have its place.\n\nNice to know. Do consider the current version more of a\nproof-of-concept that some definitive code.\n\n>> The second patch exposes it from commitdiff view (obviosly), but also\n>> from shortlog view, when less than 16 patches are begin shown.\n>\n> Why this nonconfigurable limit?\n\nBecause the patch was actually a quick hack for the proof of concept\n8-) I wasn't even sure the patch idea would have been worth it (as\nopposed to email-izing commitdiff_plain).\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"96805","messageId":"200812010145.36612.jnareb@gmail.com","threadId":"16519","inReplyTo":"cb7bb73a0811291744t2bb9c8c1t1dac497705e2c3c2@mail.gmail.com","subject":"Re: [PATCH 0/2] gitweb: patch view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-12-01T00:45:34Z","receivedAt":"2008-12-01T00:45:34Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, 30 Nov 2008, Giuseppe Bilotta wrote:\n> On Sun, Nov 30, 2008 at 2:06 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n>> On Sat, 29 Nov 2008, Giuseppe Bilotta wrote:\n>>\n>>> I recently discovered that the commitdiff_plain view is not exactly\n>>> something that can be used by git am directly (for example, the subject\n>>> line gets duplicated in the commit message body after using git am).\n>>\n>> That's because gitweb generates email-like format \"by hand\", instead\n>> of using '--format=email' or git-format-patch like in your series. On\n>> the other hand that allows us to add extra headers, namely X-Git-Tag:\n>> (which hasn't best implementation, anyway) and X-Git-Url: with URL\n>> for given output.\n> \n>> By the way, we still might want to add somehow X-Git-Url and X-Git-Tag\n>> headers later to 'patch' ('patchset') output format.\n> \n> Yeah, I've been thinking about it, but I couldn't find an easy and\n> robust way to do it. Plus, should we add them for each patch, or just\n> once for the whole patchset?\n\nTrue, that is a complication. Perhaps they should be added only for\nsingle patch?\n\n>>> Since I'm not sure if it was the case to fix the plain view because I\n>>> don't know what its intended usage was, I prepared a new view,\n>>> uncreatively called 'patch', that exposes git format-patch output\n>>> directly.\n>>\n>> Perhaps 'format_patch' would be better... hmmm... ?\n> \n> Considering I think commitdiff is ugly and long, you can guess my\n> opinion on format_patch 8-P. 'patchset' might be a good candidate,\n> considering it's what it does when both hash_parent and hash are\n> given.\n\nTrue, 'patchset' might be even better, especially that it hints\nwhat it does for a range a..b (not diff of endpoints, but series\nof patches).\n\n>> Actually IMHO both 'commitdiff' and 'commitdiff_plain' try to do two\n>> things at once. First to show diff _for_ a commit, i.e. equivalent of\n>> \"git show\" or \"git show --pretty=email\", perhaps choosing one of\n>> parents for a merge commit. Then showing commit message for $hash has\n>> sense. The fact that 'commit' view doesn't show patchset, while\n>> 'commitdiff' does might be result of historical situation.\n>>\n>> Second, to show diff _between_ commits, i.e. equivalent of\n>> \"git diff branch master\". Then there doesn't make much sense to show\n>> full commit message _only_ for one side of diff. IMHO that should be\n>> main purpose of 'commitdiff' and 'commitdiff_plain' views, or simply\n>> 'diff' / 'diff_plain' future views.\n> \n> We can probably consider deprecating commitdiff(_plain) and have the\n> following three views:\n> \n> * commit(_plain): do what commitdiff(_plain) currently does for a single commit\n\nEquivalent of \"git show\" (and not merely \"git cat-file -t commit\").\n\n> * diff(_plain): do what commitdiff(_plain) currently does for\n> parent..hash views, modulo something to be discussed for commit\n> messages (a shortlog rather maybe?)\n\nEquivalent of \"git diff\" (or \"git diff-tree\").\n\nDiffstat, or dirstat might be a good idea. Shortlog... I am not sure.\nDiff is about endpoints, and they can be in reverse, too.\n\nThere is a problem how to denote endpoints.\n\n> * patch[set?][_plain?]: format-patch style output (maybe with option\n> for HTML stuff too)\n\nEquivalent of \"git format-patch\".\n\nActually the HTML format would be more like \"git log -p\", so perhaps\nthat could be handled simply as a version of 'log' view (perhaps via\n@extra_options aka 'opt' parameter).\n\n>> What 'patch' view does, what might be not obvious from this description\n>> and from first patch in series, is to show diffs for _series_ of\n>> commits. It means equivalent of \"git log -p\" or \"git whatchanged\".\n>> It might make more sense to have plain git-format-patch output, but it\n>> could be useful to have some kind of 'git log -p' HTML output.\n>>\n>> So even if 'commitdiff' / 'commitdiff_plain' is fixed, 'patch' whould\n>> still have its place.\n> \n> Nice to know. Do consider the current version more of a\n> proof-of-concept that some definitive code.\n\nAh. O.K. It would be nice if this patch was marked as RFC (well, lack\nof signoff hints at this), or as WIP, or as PoC,...\n\n>>> The second patch exposes it from commitdiff view (obviosly), but also\n>>> from shortlog view, when less than 16 patches are begin shown.\n>>\n>> Why this nonconfigurable limit?\n> \n> Because the patch was actually a quick hack for the proof of concept\n> 8-) I wasn't even sure the patch idea would have been worth it (as\n> opposed to email-izing commitdiff_plain).\n\nAh.\n\nWell, we might want to impose some limit to avoid generating and sending\npatchset for a whole history. Perhaps to page size (100), or some similar\nnumber?\n-- \nJakub Narebski\nPoland\n"},{"id":"96806","messageId":"cb7bb73a0811301710i383105b0j80b8dbf4563f93ca@mail.gmail.com","threadId":"16519","inReplyTo":"200812010145.36612.jnareb@gmail.com","subject":"Re: [PATCH 0/2] gitweb: patch view","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-12-01T01:10:58Z","receivedAt":"2008-12-01T01:10:58Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Mon, Dec 1, 2008 at 1:45 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Sun, 30 Nov 2008, Giuseppe Bilotta wrote:\n>> On Sun, Nov 30, 2008 at 2:06 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n>>> On Sat, 29 Nov 2008, Giuseppe Bilotta wrote:\n>>\n>>> By the way, we still might want to add somehow X-Git-Url and X-Git-Tag\n>>> headers later to 'patch' ('patchset') output format.\n>>\n>> Yeah, I've been thinking about it, but I couldn't find an easy and\n>> robust way to do it. Plus, should we add them for each patch, or just\n>> once for the whole patchset?\n>\n> True, that is a complication. Perhaps they should be added only for\n> single patch?\n\nAlthough that's rather easy to implement technically, it also creates\nsome kind of inconsistency.\n\n>> Considering I think commitdiff is ugly and long, you can guess my\n>> opinion on format_patch 8-P. 'patchset' might be a good candidate,\n>> considering it's what it does when both hash_parent and hash are\n>> given.\n>\n> True, 'patchset' might be even better, especially that it hints\n> what it does for a range a..b (not diff of endpoints, but series\n> of patches).\n\nGood, I'll rename it.\n\n>> We can probably consider deprecating commitdiff(_plain) and have the\n>> following three views:\n>>\n>> * commit(_plain): do what commitdiff(_plain) currently does for a single commit\n>\n> Equivalent of \"git show\" (and not merely \"git cat-file -t commit\").\n>\n>> * diff(_plain): do what commitdiff(_plain) currently does for\n>> parent..hash views, modulo something to be discussed for commit\n>> messages (a shortlog rather maybe?)\n>\n> Equivalent of \"git diff\" (or \"git diff-tree\").\n>\n> Diffstat, or dirstat might be a good idea. Shortlog... I am not sure.\n> Diff is about endpoints, and they can be in reverse, too.\n>\n> There is a problem how to denote endpoints.\n\nHm? Doesn't parent..hash work? Or are you talking about something else?\n\n>> * patch[set?][_plain?]: format-patch style output (maybe with option\n>> for HTML stuff too)\n>\n> Equivalent of \"git format-patch\".\n>\n> Actually the HTML format would be more like \"git log -p\", so perhaps\n> that could be handled simply as a version of 'log' view (perhaps via\n> @extra_options aka 'opt' parameter).\n\nThis is starting to get complicated ... I'm not sure how far in this I\ncan go with this patchset, so for the time being I'll probably just\nstick to refining the (plain) patchset feature.\n\n>>> What 'patch' view does, what might be not obvious from this description\n>>> and from first patch in series, is to show diffs for _series_ of\n>>> commits. It means equivalent of \"git log -p\" or \"git whatchanged\".\n>>> It might make more sense to have plain git-format-patch output, but it\n>>> could be useful to have some kind of 'git log -p' HTML output.\n>>>\n>>> So even if 'commitdiff' / 'commitdiff_plain' is fixed, 'patch' whould\n>>> still have its place.\n>>\n>> Nice to know. Do consider the current version more of a\n>> proof-of-concept that some definitive code.\n>\n> Ah. O.K. It would be nice if this patch was marked as RFC (well, lack\n> of signoff hints at this), or as WIP, or as PoC,...\n\nDamn,  I always forget about that.\n\n>>>> The second patch exposes it from commitdiff view (obviosly), but also\n>>>> from shortlog view, when less than 16 patches are begin shown.\n>>>\n>>> Why this nonconfigurable limit?\n>>\n>> Because the patch was actually a quick hack for the proof of concept\n>> 8-) I wasn't even sure the patch idea would have been worth it (as\n>> opposed to email-izing commitdiff_plain).\n>\n> Ah.\n>\n> Well, we might want to impose some limit to avoid generating and sending\n> patchset for a whole history. Perhaps to page size (100), or some similar\n> number?\n\nThe reason why I chose 16 is that (1) it's a rather commonly used\n'small' number across gitweb and (2) it's a rather acceptable\n'universal' upper limit for patchsets. There _are_ a few patchbombs\nthat considerably overtake that limit, but observe that this limit is\nnot an arbitrary limit on patchsets generated by the 'patchset' view,\nbut only a condition for which a link is generated from shortlog view.\n\nWe may want to have TWO limits here: one is the absolute maximum limit\nto the number of patches dumped in a patchset (to prevent DoS attacks\nby repeated requests of the whole history), and the other one is the\nlimit for autogenerated patchset links.\n\nBTW, autogenerated patchset links probably make sense taking some\nprevious branch name as point of reference. e.g., if I have branch1\nwithin the history of branch2, we probably want some (semi)automatic\nway of getting a patchset for branch1..branch2 --of course, we also\nwant to do a shortlog between them, so that's a more general feature\nwe should think about.\n\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"96826","messageId":"200812011202.41300.jnareb@gmail.com","threadId":"16519","inReplyTo":"cb7bb73a0811301710i383105b0j80b8dbf4563f93ca@mail.gmail.com","subject":"Re: [PATCH 0/2] gitweb: patch view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-12-01T11:02:39Z","receivedAt":"2008-12-01T11:02:39Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 1 December 2008, Giuseppe Bilotta wrote:\n> On Mon, Dec 1, 2008 at 1:45 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n>> On Sun, 30 Nov 2008, Giuseppe Bilotta wrote:\n>>> On Sun, Nov 30, 2008 at 2:06 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n>>>> On Sat, 29 Nov 2008, Giuseppe Bilotta wrote:\n>>>\n>>>> By the way, we still might want to add somehow X-Git-Url and X-Git-Tag\n>>>> headers later to 'patch' ('patchset') output format.\n>>>\n>>> Yeah, I've been thinking about it, but I couldn't find an easy and\n>>> robust way to do it. Plus, should we add them for each patch, or just\n>>> once for the whole patchset?\n>>\n>> True, that is a complication. Perhaps they should be added only for\n>> single patch?\n> \n> Although that's rather easy to implement technically, it also creates\n> some kind of inconsistency.\n\nWell, it is problem also from technical point of view. Currently we can\njust stream (dump) git-format-patch output to browser (not forgetting\nadding '--encoding=utf8' if it is not used already), and do not need\nto have markers between commits. It is very simple code, which is its\nown advantage.\n\n>From theoretical point of view corrected X-Git-Tag functioning as\na kind of ref marker but for the raw (text/plain) output could be added\nfor each end every patch, so there would be no inconsistency for _this_\nextra header.\n\nI don't know what can be done about X-Git-URL.\n\n>>> Considering I think commitdiff is ugly and long, you can guess my\n>>> opinion on format_patch 8-P. 'patchset' might be a good candidate,\n>>> considering it's what it does when both hash_parent and hash are\n>>> given.\n>>\n>> True, 'patchset' might be even better, especially that it hints\n>> what it does for a range a..b (not diff of endpoints, but series\n>> of patches).\n> \n> Good, I'll rename it.\n\nI just don't know if it would be best name. Perhaps 'patches' would\nbe better?\n\n[...]\n>>> * diff(_plain): do what commitdiff(_plain) currently does for\n>>> parent..hash views, modulo something to be discussed for commit\n>>> messages (a shortlog rather maybe?)\n>>\n>> Equivalent of \"git diff\" (or \"git diff-tree\").\n>>\n>> Diffstat, or dirstat might be a good idea. Shortlog... I am not sure.\n>> Diff is about endpoints, and they can be in reverse, too.\n>>\n>> There is a problem how to denote endpoints.\n> \n> Hm? Doesn't parent..hash work? Or are you talking about something else?\n\nErrr... I meant here for the user, not for gitweb. To somehow denote\nbefore patch itself the endpoints. Just like for diff _for_ a commit\nwe have commit message denoting :-).\n\n>>> * patch[set?][_plain?]: format-patch style output (maybe with option\n>>> for HTML stuff too)\n>>\n>> Equivalent of \"git format-patch\".\n>>\n>> Actually the HTML format would be more like \"git log -p\", so perhaps\n>> that could be handled simply as a version of 'log' view (perhaps via\n>> @extra_options aka 'opt' parameter).\n> \n> This is starting to get complicated ... I'm not sure how far in this I\n> can go with this patchset, so for the time being I'll probably just\n> stick to refining the (plain) patchset feature.\n\nWhat I meant here is that it would be IMHO enough to have 'patch' view\n(or whatever it ends up named) be raw format / plain/text format only,\nand leave HTML equivalent for extra options/extra format to 'log' view.\n\n[...]\n>>>>> The second patch exposes it from commitdiff view (obviosly), but also\n>>>>> from shortlog view, when less than 16 patches are begin shown.\n>>>>\n>>>> Why this nonconfigurable limit?\n>>>\n>>> Because the patch was actually a quick hack for the proof of concept\n>>> 8-) I wasn't even sure the patch idea would have been worth it (as\n>>> opposed to email-izing commitdiff_plain).\n>>\n>> Ah.\n>>\n>> Well, we might want to impose some limit to avoid generating and sending\n>> patchset for a whole history. Perhaps to page size (100), or some similar\n>> number?\n> \n> The reason why I chose 16 is that (1) it's a rather commonly used\n> 'small' number across gitweb and (2) it's a rather acceptable\n> 'universal' upper limit for patchsets. There _are_ a few patchbombs\n> that considerably overtake that limit, but observe that this limit is\n> not an arbitrary limit on patchsets generated by the 'patchset' view,\n> but only a condition for which a link is generated from shortlog view.\n\nI see.\n\n> We may want to have TWO limits here: one is the absolute maximum limit\n> to the number of patches dumped in a patchset (to prevent DoS attacks\n> by repeated requests of the whole history), and the other one is the\n> limit for autogenerated patchset links.\n\nA pageful (100 commits) as hard limit against DoS attacks?\n\n[...]\n-- \nJakub Narebski\nPoland\n"},{"id":"97039","messageId":"cb7bb73a0812030125h3456d4d6occe6b6509b8d21c9@mail.gmail.com","threadId":"16519","inReplyTo":"200812011202.41300.jnareb@gmail.com","subject":"Re: [PATCH 0/2] gitweb: patch view","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2008-12-03T09:25:22Z","receivedAt":"2008-12-03T09:25:22Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Mon, Dec 1, 2008 at 12:02 PM, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Mon, 1 December 2008, Giuseppe Bilotta wrote:\n>> On Mon, Dec 1, 2008 at 1:45 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n>>> On Sun, 30 Nov 2008, Giuseppe Bilotta wrote:\n>>>> On Sun, Nov 30, 2008 at 2:06 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n>>>>> On Sat, 29 Nov 2008, Giuseppe Bilotta wrote:\n>>>>\n>>>>> By the way, we still might want to add somehow X-Git-Url and X-Git-Tag\n>>>>> headers later to 'patch' ('patchset') output format.\n>>>>\n>>>> Yeah, I've been thinking about it, but I couldn't find an easy and\n>>>> robust way to do it. Plus, should we add them for each patch, or just\n>>>> once for the whole patchset?\n>>>\n>>> True, that is a complication. Perhaps they should be added only for\n>>> single patch?\n>>\n>> Although that's rather easy to implement technically, it also creates\n>> some kind of inconsistency.\n>\n> Well, it is problem also from technical point of view. Currently we can\n> just stream (dump) git-format-patch output to browser (not forgetting\n> adding '--encoding=utf8' if it is not used already), and do not need\n> to have markers between commits. It is very simple code, which is its\n> own advantage.\n>\n> From theoretical point of view corrected X-Git-Tag functioning as\n> a kind of ref marker but for the raw (text/plain) output could be added\n> for each end every patch, so there would be no inconsistency for _this_\n> extra header.\n>\n> I don't know what can be done about X-Git-URL.\n\nI'm thinking that the best way to achieve these results would be to\nhave some way to specify extra headers to git format-patch from the\ncommand line and not just from a config file. Plus, we want to make\nthem 'dynamic' in the sense that we want to be able to put hash or ref\nnames etc in them. For the moment I'll mark this 'TODO' in the file.\n\n>>>> Considering I think commitdiff is ugly and long, you can guess my\n>>>> opinion on format_patch 8-P. 'patchset' might be a good candidate,\n>>>> considering it's what it does when both hash_parent and hash are\n>>>> given.\n>>>\n>>> True, 'patchset' might be even better, especially that it hints\n>>> what it does for a range a..b (not diff of endpoints, but series\n>>> of patches).\n>>\n>> Good, I'll rename it.\n>\n> I just don't know if it would be best name. Perhaps 'patches' would\n> be better?\n\nThe only thing I don't like about 'patches' is that if you ask for a\nsingle commit you get a single patch. I'd rather stick to 'patch'\nthen, maybe make 'patches' a synonym?\n\n>>>> * diff(_plain): do what commitdiff(_plain) currently does for\n>>>> parent..hash views, modulo something to be discussed for commit\n>>>> messages (a shortlog rather maybe?)\n>>>\n>>> Equivalent of \"git diff\" (or \"git diff-tree\").\n>>>\n>>> Diffstat, or dirstat might be a good idea. Shortlog... I am not sure.\n>>> Diff is about endpoints, and they can be in reverse, too.\n>>>\n>>> There is a problem how to denote endpoints.\n>>\n>> Hm? Doesn't parent..hash work? Or are you talking about something else?\n>\n> Errr... I meant here for the user, not for gitweb. To somehow denote\n> before patch itself the endpoints. Just like for diff _for_ a commit\n> we have commit message denoting :-).\n\nAh, in the sense that you have to specify parent..hash manually in the\nURL presently? I've seen some patches to non-main gitweb doing this\nkind of thing. If it got merged with upstream we could use that as\nwell.\n\n>>>> * patch[set?][_plain?]: format-patch style output (maybe with option\n>>>> for HTML stuff too)\n>>>\n>>> Equivalent of \"git format-patch\".\n>>>\n>>> Actually the HTML format would be more like \"git log -p\", so perhaps\n>>> that could be handled simply as a version of 'log' view (perhaps via\n>>> @extra_options aka 'opt' parameter).\n>>\n>> This is starting to get complicated ... I'm not sure how far in this I\n>> can go with this patchset, so for the time being I'll probably just\n>> stick to refining the (plain) patchset feature.\n>\n> What I meant here is that it would be IMHO enough to have 'patch' view\n> (or whatever it ends up named) be raw format / plain/text format only,\n> and leave HTML equivalent for extra options/extra format to 'log' view.\n\nAh, ok. I'll resubmit a cleaned up version of these two patches for\nthe time being then.\n\n>>>>>> The second patch exposes it from commitdiff view (obviosly), but also\n>>>>>> from shortlog view, when less than 16 patches are begin shown.\n>>>>>\n>>>>> Why this nonconfigurable limit?\n>>>>\n>>>> Because the patch was actually a quick hack for the proof of concept\n>>>> 8-) I wasn't even sure the patch idea would have been worth it (as\n>>>> opposed to email-izing commitdiff_plain).\n>>>\n>>> Ah.\n>>>\n>>> Well, we might want to impose some limit to avoid generating and sending\n>>> patchset for a whole history. Perhaps to page size (100), or some similar\n>>> number?\n>>\n>> The reason why I chose 16 is that (1) it's a rather commonly used\n>> 'small' number across gitweb and (2) it's a rather acceptable\n>> 'universal' upper limit for patchsets. There _are_ a few patchbombs\n>> that considerably overtake that limit, but observe that this limit is\n>> not an arbitrary limit on patchsets generated by the 'patchset' view,\n>> but only a condition for which a link is generated from shortlog view.\n>\n> I see.\n>\n>> We may want to have TWO limits here: one is the absolute maximum limit\n>> to the number of patches dumped in a patchset (to prevent DoS attacks\n>> by repeated requests of the whole history), and the other one is the\n>> limit for autogenerated patchset links.\n>\n> A pageful (100 commits) as hard limit against DoS attacks?\n\nI suspect 100 is too hight already, but I guess we can tune it later.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"}]}