{"thread":{"id":"43355","subject":"Re: [PATCH/RFC] gitweb: New improved patchset view","startedAt":"2006-10-29T17:50:46Z","lastAt":"2006-10-29T20:29:15Z","messageCount":3,"participants":["Luben Tuikov","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"297377","messageId":"200610291850.46321.jnareb@gmail.com","threadId":"43355","inReplyTo":"200610291122.30852.jnareb@gmail.com","subject":"Re: [PATCH/RFC] gitweb: New improved patchset view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-29T17:50:46Z","receivedAt":"2006-10-29T17:50:46Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"A couple of questions regarding new patchset/diff look for gitweb.\nCurrently patch starts with \"git diff\" header\n\n diff --git a/file1 b/file2\n\nthen we have a couple of extended header lines\n\n old|new|deleted file|new file mode \n copy|rename from|to \n (dis)?similarity index \n index .. []\n\nthen we have two-line from-file/to-file header\n\n --- a/file1\n +++ b/file2\n\nthen patch itself.\n\n\n1. Which parts to convert to hyperlinks, and which to do not? Which\n links have visible and which hidden (underline on mouseover, the same\n color as neighbouring text)?\n\n I think that a/file1 in \"git diff\" header should be turned into\n visible hyperlink unless file is created, and b/file2 should be link\n unless file is deleted.\n\n Also both -es in \"index\" extended header lines should be turned\n into links, as it is the only way to have hyperlink to all previous\n versions of the file in the case of \"combined diff\" format (to be\n added later). The question is if those hyperlinks should be visible;\n I don't have compelling reason against. Should we use title attribute\n to give filename perhaps, or is it unnecessary?\n\n Currently file1 and file2 in \"--- a/file1\" and \"+++ b/file2\" are\n turned into hidden links. Should we leave it, or should we remove\n this link as we have similar link just above? If we decide to have\n this link, should we also hyperlink in \"copy|rename\" extended\n header line?\n\n2. Use quoted or unquoted filename, remove or leave surrounding quotes\n in quoted filename? Should we unquote the not hyperlinked filename\n in the case of creation/deletion? What should be span of link:\n \"a/_file1_\", \"_a/file1_\", _\"a/file1\"_\n \"_file_\", _\"file\"_ \n (where '_' marks beginning and end of link) for quoted filenames?\n What should be span of link for unquoted filenames:\n a/_file_, _a/file_\n Currently gitweb uses a/_file_ in ---/+++ line.\n\n3. How (and if) to explain numerical mode: the currently used\n / () e.g. 100755/-rwxr-xr-x (file).\n Or () should be enough? Should we mark the addition\n compared to git-diff output? Or should we explain only on\n mouseover, using for example:\n 100644\n or just\n 100755?\n-- \nJakub Narebski\n"},{"id":"294469","messageId":"20061029195554.77410.qmail@web31813.mail.mud.yahoo.com","threadId":"43355","inReplyTo":"200610291850.46321.jnareb@gmail.com","subject":"Re: [PATCH/RFC] gitweb: New improved patchset view","fromName":"Luben Tuikov","fromEmail":"ltuikov@yahoo.com","sentAt":"2006-10-29T19:55:54Z","receivedAt":"2006-10-29T19:55:54Z","isPatch":true,"sender":{"key":"ltuikov@yahoo.com","avatar":null},"body":"--- Jakub Narebski wrote:\n> A couple of questions regarding new patchset/diff look for gitweb.\n> Currently patch starts with \"git diff\" header\n> \n> diff --git a/file1 b/file2\n> \n> then we have a couple of extended header lines\n> \n> old|new|deleted file|new file mode \n> copy|rename from|to \n> (dis)?similarity index \n> index .. []\n> \n> then we have two-line from-file/to-file header\n\nFirst revert back to monospace in the commit message.\nAs I mentioned in that other email, where the person\ndidn't CC you and thus so I didn't, see commit \n4b7ce6e2d6ba088da50de1df38b040ea2c0b8f18.\n\nCan you please hyperlink what you had intended to\nquote? I.e. filenames which can lead the user browsing to a\nstate (pre-rename, post-rename, etc). Thanks.\n\n> \n> --- a/file1\n> +++ b/file2\n> \n> then patch itself.\n> \n> \n> 1. Which parts to convert to hyperlinks, and which to do not? Which\n> links have visible and which hidden (underline on mouseover, the same\n> color as neighbouring text)?\n\nI think the current state of your patch is good, sans the comments\nreceived so far: monospace, that bug in the index links, etc.\nLets get that in, and then you can RFC another improvement on top of this.\n\n> I think that a/file1 in \"git diff\" header should be turned into\n> visible hyperlink unless file is created, and b/file2 should be link\n> unless file is deleted.\n\nThis is intuitive and makes sense.\n\n> Also both -es in \"index\" extended header lines should be turned\n> into links, as it is the only way to have hyperlink to all previou\n\nFix the bug though.\n\n> versions of the file in the case of \"combined diff\" format (to be\n> added later). The question is if those hyperlinks should be visible;\n\nI personally love little hidden gems, but some people like everything\nto be overly obvious to them. Hidden gems are part of the learning,\nbut I'm sure I'm not going to convince everyone.\n\nHidden.\n\n> I don't have compelling reason against. Should we use title attribute\n> to give filename perhaps, or is it unnecessary?\n> \n> Currently file1 and file2 in \"--- a/file1\" and \"+++ b/file2\" are\n> turned into hidden links. Should we leave it, or should we remove\n> this link as we have similar link just above? If we decide to have\n\nLeave it -- it's cool.\n\n> this link, should we also hyperlink in \"copy|rename\" extended\n> header line?\n\nIf it makes sense.\n\n> 2. Use quoted or unquoted filename, remove or leave surrounding quotes\n> in quoted filename? Should we unquote the not hyperlinked filename\n\nAre you familiar with the term \"legalism\"?\n\n> in the case of creation/deletion? What should be span of link:\n> \"a/_file1_\", \"_a/file1_\", _\"a/file1\"_\n> \"_file_\", _\"file\"_ \n> (where '_' marks beginning and end of link) for quoted filenames?\n> What should be span of link for unquoted filenames:\n> a/_file_, _a/file_\n> Currently gitweb uses a/_file_ in ---/+++ line.\n\nUnquoted! Now that we've solved this \"problem\", lets move on to more\ninteresting things. ;-)\n\n> 3. How (and if) to explain numerical mode: the currently used\n\nLet's not explain it for now. Let's have your patch go in sans the\ncomments already posted. We can always debate on that later.\n\nThanks,\n Luben\n\n\n> / () e.g. 100755/-rwxr-xr-x (file).\n> Or () should be enough? Should we mark the addition\n> compared to git-diff output? Or should we explain only on\n> mouseover, using for example:\n> 100644\n> or just\n> 100755?\n> -- \n> Jakub Narebski\n> Poland\n> -\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \n"},{"id":"296840","messageId":"200610292129.15372.jnareb@gmail.com","threadId":"43355","inReplyTo":"20061029195554.77410.qmail@web31813.mail.mud.yahoo.com","subject":"Re: [PATCH/RFC] gitweb: New improved patchset view","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-29T20:29:15Z","receivedAt":"2006-10-29T20:29:15Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Luben Tuikov wrote:\n> --- Jakub Narebski wrote:\n> > A couple of questions regarding new patchset/diff look for gitweb.\n> > Currently patch starts with \"git diff\" header\n> > \n> > diff --git a/file1 b/file2\n> > \n> > then we have a couple of extended header lines\n> > \n> > old|new|deleted file|new file mode \n> > copy|rename from|to \n> > (dis)?similarity index \n> > index .. []\n> > \n> > then we have two-line from-file/to-file header\n> \n> First revert back to monospace in the commit message.\n> As I mentioned in that other email, where the person\n> didn't CC you and thus so I didn't, see commit \n> 4b7ce6e2d6ba088da50de1df38b040ea2c0b8f18.\n\nThis regression was caused by the not-accepted (I think) patch\n\"gitweb: Slight visual improvements to commitdiff view\",\nand is not caused by _this_ patch.\n\nI'll redo abovementioned patch correctly later.\n\n> Can you please hyperlink what you had intended to\n> quote? I.e. filenames which can lead the user browsing to a\n> state (pre-rename, post-rename, etc). Thanks.\n\nI don't understand this comment. The above is pre-changes git-diff\npatch output. The changes are/were mentioned below.\n\n> > \n> > --- a/file1\n> > +++ b/file2\n> > \n> > then patch itself.\n> > \n> > \n> > 1. Which parts to convert to hyperlinks, and which to do not? Which\n> > links have visible and which hidden (underline on mouseover, the same\n> > color as neighbouring text)?\n> \n> I think the current state of your patch is good, sans the comments\n> received so far: monospace, that bug in the index links, etc.\n> Lets get that in, and then you can RFC another improvement on top of this.\n\nO.K. I send the corrected version (as \"take 2\") in a while.\n\n-- \nJakub Narebski\n"}]}