{"thread":{"id":"43172","subject":"Re: [EGIT PATCH] Convert author and comment on demand.","startedAt":"2006-12-03T00:45:09Z","lastAt":"2006-12-03T18:08:02Z","messageCount":5,"participants":["Linus Torvalds","Robin Rosenberg","Shawn Pearce","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"297177","messageId":"200612030145.09576.robin.rosenberg@dewire.com","threadId":"43172","inReplyTo":null,"subject":"[EGIT PATCH] Convert author and comment on demand.","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2006-12-03T00:45:09Z","receivedAt":"2006-12-03T00:45:09Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"This sppeds up reading commits a lot by only store the byte\narray data when reading commits. For the eclipse plugin I only\nneed the tree to filter out which commits to display and I can\ntake the cost of converting the comments to string for the\nvery few commits to display. Only the displayed commits are actually\nconverted so this results in convertig author and comment information\nfor about five commits rather than 20,000 (in my repo).\n\nSigned-off-by: Robin Rosenberg \n---\n .../src/org/spearce/jgit/lib/Commit.java | 73 +++++++++++++------\n 1 files changed, 50 insertions(+), 23 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Commit.java \nb/org.spearce.jgit/src/org/spearce/jgit/lib/Commit.java\nindex 4e03a5a..14fa602 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Commit.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Commit.java\n@@ -16,10 +16,16 @@\n */\n package org.spearce.jgit.lib;\n \n+import java.io.BufferedReader;\n+import java.io.ByteArrayInputStream;\n import java.io.IOException;\n+import java.io.InputStreamReader;\n+import java.nio.charset.Charset;\n import java.util.ArrayList;\n+import java.util.Arrays;\n import java.util.List;\n \n+import org.spearce.jgit.errors.CorruptObjectException;\n import org.spearce.jgit.errors.MissingObjectException;\n \n public class Commit implements Treeish {\n@@ -39,6 +45,8 @@ public class Commit implements Treeish {\n \n private Tree treeObj;\n \n+ private byte[] raw;\n+\n public Commit(final Repository db) {\n \tobjdb = db;\n \tparentIds = new ArrayList(2);\n@@ -58,29 +66,7 @@ public class Commit implements Treeish {\n \t rawPtr += 48;\n \t}\n \n-\t//\n-\t// if (n == null || !n.startsWith(\"author \")) {\n-\t// throw new CorruptObjectException(commitId, \"no author\");\n-\t// }\n-\t// author = new PersonIdent(n.substring(\"author \".length()));\n-\t//\n-\t// n = br.readLine();\n-\t// if (n == null || !n.startsWith(\"committer \")) {\n-\t// throw new CorruptObjectException(commitId, \"no committer\");\n-\t// }\n-\t// committer = new PersonIdent(n.substring(\"committer \".length()));\n-\t//\n-\t// n = br.readLine();\n-\t// if (n == null || !n.equals(\"\")) {\n-\t// throw new CorruptObjectException(commitId, \"malformed header\");\n-\t// }\n-\t//\n-\t// tempMessage = new StringBuffer();\n-\t// readBuf = new char[128];\n-\t// while ((readLen = br.read(readBuf)) > 0) {\n-\t// tempMessage.append(readBuf, 0, readLen);\n-\t// }\n-\t// message = tempMessage.toString();\n+\tthis.raw = raw;\n }\n \n public ObjectId getCommitId() {\n@@ -119,6 +105,7 @@ public class Commit implements Treeish {\n }\n \n public PersonIdent getAuthor() {\n+\tdecode();\n \treturn author;\n }\n \n@@ -127,6 +114,7 @@ public class Commit implements Treeish {\n }\n \n public PersonIdent getCommitter() {\n+\tdecode();\n \treturn committer;\n }\n \n@@ -139,9 +127,48 @@ public class Commit implements Treeish {\n }\n \n public String getMessage() {\n+\tdecode();\n \treturn message;\n }\n \n+ private void decode() {\n+\tif (raw!=null) {\n+\t try {\n+ \tBufferedReader br=new BufferedReader(new InputStreamReader(new \nByteArrayInputStream(raw)));\n+ \tString n=br.readLine();\n+ if (n == null || !n.startsWith(\"tree \")) {\n+ throw new CorruptObjectException(commitId, \"no tree\");\n+ }\n+ while ((n = br.readLine())!=null && n.startsWith(\"parent \"))\n+ \t;\n+ if (n == null || !n.startsWith(\"author \")) {\n+ throw new CorruptObjectException(commitId, \"no author\");\n+ }\n+ author = new PersonIdent(n.substring(\"author \".length()));\n+ n = br.readLine();\n+ if (n == null || !n.startsWith(\"committer \")) {\n+ throw new CorruptObjectException(commitId, \"no \ncommitter\");\n+ }\n+ committer = new \nPersonIdent(n.substring(\"committer \".length()));\n+ n = br.readLine();\n+ if (n == null || !n.equals(\"\")) {\n+ throw new CorruptObjectException(commitId, \"malformed \nheader\");\n+ }\n+ StringBuffer tempMessage = new StringBuffer();\n+ char[] readBuf = new char[2048];\n+ int readLen;\n+\t\twhile ((readLen = br.read(readBuf)) > 0) {\n+ tempMessage.append(readBuf, 0, readLen);\n+ }\n+ message = tempMessage.toString();\n+\t } catch (IOException e) {\n+\t\te.printStackTrace();\n+\t } finally {\n+\t\traw = null;\n+\t }\n+\t}\n+ }\n+\n public void setMessage(final String m) {\n \tmessage = m;\n }\n-- \n1.4.4.gf05d\n\n"},{"id":"295755","messageId":"20061203021655.GA26668@spearce.org","threadId":"43172","inReplyTo":"200612030145.09576.robin.rosenberg@dewire.com","subject":"Re: [EGIT PATCH] Convert author and comment on demand.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-12-03T02:16:55Z","receivedAt":"2006-12-03T02:16:55Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg wrote:\n> This sppeds up reading commits a lot by only store the byte\n> array data when reading commits.\n\nThanks. I was working on something similar but did not have\na chance to finish it. I've applied your patch instead.\n\n> +\t try {\n> + \tBufferedReader br=new BufferedReader(new InputStreamReader(new \n> ByteArrayInputStream(raw)));\n> + \tString n=br.readLine();\n\nSomething's wrong with your mail client... the patch was mangled.\n\n-- \n"},{"id":"294984","messageId":"200612031318.33332.robin.rosenberg@dewire.com","threadId":"43172","inReplyTo":"20061203021655.GA26668@spearce.org","subject":"Re: [EGIT PATCH] Convert author and comment on demand.","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2006-12-03T12:18:32Z","receivedAt":"2006-12-03T12:18:32Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"söndag 03 december 2006 03:16 skrev Shawn Pearce:\n> Robin Rosenberg wrote:\n> > This sppeds up reading commits a lot by only store the byte\n> > array data when reading commits.\n>\n> Thanks. I was working on something similar but did not have\n> a chance to finish it. I've applied your patch instead.\n>\n> > +\t try {\n> > + \tBufferedReader br=new BufferedReader(new InputStreamReader(new\n> > ByteArrayInputStream(raw)));\n> > + \tString n=br.readLine();\n>\n> Something's wrong with your mail client... the patch was mangled.\n\nNot really, only the user. KMail by default wraps lines and I didn't turn that \noff before sending. Usually I send patches just using stg mail, but this time \nI used git-format-patch and kmail. Generatig attachements may be better (next \ntime).\n\n"},{"id":"298315","messageId":"ekug7o$35c$1@sea.gmane.org","threadId":"43172","inReplyTo":"200612031318.33332.robin.rosenberg@dewire.com","subject":"Re: [EGIT PATCH] Convert author and comment on demand.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-12-03T12:34:00Z","receivedAt":"2006-12-03T12:34:00Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Robin Rosenberg wrote:\n\n> söndag 03 december 2006 03:16 skrev Shawn Pearce:\n>>\n>> Something's wrong with your mail client... the patch was mangled.\n> \n> Not really, only the user. KMail by default wraps lines and I didn't turn that \n> off before sending. Usually I send patches just using stg mail, but this time \n> I used git-format-patch and kmail. Generatig attachements may be better (next \n> time).\n\nIt is easier to comment on patch if it is embedded in the mail, and not\nin attachement. Just remember to turn off word wrapping before sending\nthe patch.\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n\n"},{"id":"293967","messageId":"Pine.LNX.4.64.0612030955320.3476@woody.osdl.org","threadId":"43172","inReplyTo":"ekug7o$35c$1@sea.gmane.org","subject":"Re: [EGIT PATCH] Convert author and comment on demand.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-12-03T18:08:02Z","receivedAt":"2006-12-03T18:08:02Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 3 Dec 2006, Jakub Narebski wrote:\n> \n> It is easier to comment on patch if it is embedded in the mail, and not\n> in attachement.\n\nThis is why I _hate_ patches in attachments.\n\nSure, I can make my mail reader show the attachments. But when I \"reply\", \nI want the patch to be indented with the regular \"> \" thing and visible in \nthe reply, so that I can say \"I like the patch in general, but part \nof it is seriously broken\".\n\nAnd yes, the personal mailreader I have has a \"include attachements in \nreply\" mode. But I don't generally want to include attachments in any \nreply, I just want it for _patches_.\n\n[ Side ntoe: besides, many people send broken attachments that aren't \n marked as text, but as 8-bit binary data or something, even if it's a \n text-patch - so \"attachement problems\" are almost as common as the \n non-attachement \"line wrap\" or whitespace problems are - people who \n think that attachments automatically means that the thing is correct are \n just deluding themselves.\n\n The fact is, you can get attachments wrong exactly the same way you get \n linewrapping wrong. It's just that the pure binary data is likely to \n always make it through correctly with an attachment, but if it gets \n marked as a binary MIME-type, that doesn't much help, because while the \n data is there, it's still going to act the wrong way for any \n _discussion_ about it. ]\n\nIn other words: there are lots of things that make sense as true \nattachments. It's just that \"patch\" is not one of them.\n\nPatches, unlike for example full files or tar-balls, by their very design \nare (a) text and (b) something where the whole _point_ is discussion and \nquoting about them. If we didn't want to discuss the patch contents, we \nwouldn't send them as patches in the first place, they'd be git-to-git \ntransfers.\n\nSo this all boils down to one thing:\n\n\tPATCHES ARE NOT SEPARATE FILES TO BE ATTACHED TO AN EMAIL.\n\nPatches are to be considered part of the discussion, not separate. And \nthus they should be in the main body of the email, exactly so that people \n(regardless of mailer settings and details like that) will automatically \nquote them, and they get passed around as integral to the discussion as \nthe discussion itself.\n\nI just jumped in because this is a pet peeve of mine. Some people seem to \nthink that patches are \"binary files\" just because they have whitespace \nrequirements and line-wrapping matters. But whitespace and line wrapping \nis important even in regular discussion, and patches really _are_ about \nthe _human_ communication, just as much - and perhaps more so - as they \nare about feeding to the \"patch\" program.\n\n"}]}