{"thread":{"id":"36683","subject":"[PATCH] Documentation/technical/api-hashmap: Remove source highlighting","startedAt":"2014-05-17T11:08:55Z","lastAt":"2014-05-20T17:31:29Z","messageCount":6,"participants":["Anders Kaseorg","Jeremiah Mahler","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"242044","messageId":"alpine.DEB.2.02.1405170707260.44324@all-night-tool.MIT.EDU","threadId":"36683","inReplyTo":null,"subject":"[PATCH] Documentation/technical/api-hashmap: Remove source highlighting","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2014-05-17T11:08:55Z","receivedAt":"2014-05-17T11:08:55Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"The highlighting was pretty, but unfortunately, the failure mode when\nsource-highlight is not installed was that the entire code block\ndisappears.  See https://bugs.debian.org/745591,\nhttps://bugs.launchpad.net/bugs/1316810.\n\nSigned-off-by: Anders Kaseorg <andersk@mit.edu>\n---\n Documentation/technical/api-hashmap.txt | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/Documentation/technical/api-hashmap.txt b/Documentation/technical/api-hashmap.txt\nindex 42ca234..b977ae8 100644\n--- a/Documentation/technical/api-hashmap.txt\n+++ b/Documentation/technical/api-hashmap.txt\n@@ -166,7 +166,6 @@ Usage example\n -------------\n \n Here's a simple usage example that maps long keys to double values.\n-[source,c]\n ------------\n struct hashmap map;\n \n-- \n2.0.0.rc3\n"},{"id":"242053","messageId":"20140517152219.GA31912@hudson.localdomain","threadId":"36683","inReplyTo":"alpine.DEB.2.02.1405170707260.44324@all-night-tool.MIT.EDU","subject":"Re: [PATCH] Documentation/technical/api-hashmap: Remove source highlighting","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-05-17T15:22:19Z","receivedAt":"2014-05-17T15:22:19Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"On Sat, May 17, 2014 at 07:08:55AM -0400, Anders Kaseorg wrote:\n> The highlighting was pretty, but unfortunately, the failure mode when\n> source-highlight is not installed was that the entire code block\n> disappears.  See https://bugs.debian.org/745591,\n> https://bugs.launchpad.net/bugs/1316810.\n> \n\nI agree that a broken document is an unacceptable failure mode.\n\nBut I do not understand why 'source-highlight' is not an install\nrequirement for 'git-doc'.  If I install 'source-highlight' on\nmy Debian machine the code looks great.\n\n  apt-get install source-highlight\n\nI also noticed that this seems to be the single place where source\ncode highlighting is used in Documentation/technical.\nSo it might be worthwhile to eliminate this dependency all together\nas Anders patch does.\n\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"},{"id":"242076","messageId":"alpine.DEB.2.02.1405172035160.44324@all-night-tool.MIT.EDU","threadId":"36683","inReplyTo":"20140517152219.GA31912@hudson.localdomain","subject":"Re: [PATCH] Documentation/technical/api-hashmap: Remove source highlighting","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2014-05-18T00:50:35Z","receivedAt":"2014-05-18T00:50:35Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"On Sat, 17 May 2014, Jeremiah Mahler wrote:\n> I agree that a broken document is an unacceptable failure mode.\n> \n> But I do not understand why 'source-highlight' is not an install\n> requirement for 'git-doc'.  If I install 'source-highlight' on\n> my Debian machine the code looks great.\n> \n>   apt-get install source-highlight\n\nYes; when I noticed this failure, I asked Jonathan to add source-highlight \nas a build dependency in Debian (https://bugs.debian.org/745591).  But \nthen Ubuntu forked the packaging to revert this change \n(https://bugs.launchpad.net/bugs/1316810), because source-highlight in the \ncommunity-supported universe repository is not allowed to be a build \ndependency of git in the Canonical-supported main repository.  So now the \nUbuntu package still has broken documentation _and_ it’s lost the ability \nto automatically synchronize updates from Debian.\n\nIf we’re going to make Git depend on source-highlight, then we would want \nto make sure it’s documented in INSTALL and that its absence is properly \ndiagnosed.  Maybe then we could make the argument to Canonical that \nsource-highlight should become officially supported in main \n(https://wiki.ubuntu.com/UbuntuMainInclusionRequirements).\n\nBut I don’t that would be worth it just to make one page of the API \ndocumentation a little more colorful (and it sounds like you agree).\n\nAnders\n"},{"id":"242182","messageId":"xmqq4n0l3dc7.fsf@gitster.dls.corp.google.com","threadId":"36683","inReplyTo":"alpine.DEB.2.02.1405172035160.44324@all-night-tool.MIT.EDU","subject":"Re: [PATCH] Documentation/technical/api-hashmap: Remove source highlighting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-19T17:09:12Z","receivedAt":"2014-05-19T17:09:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anders Kaseorg <andersk@MIT.EDU> writes:\n\n> Yes; when I noticed this failure, I asked Jonathan to add source-highlight \n> as a build dependency in Debian (https://bugs.debian.org/745591).  But \n> then Ubuntu forked the packaging to revert this change \n> (https://bugs.launchpad.net/bugs/1316810), because source-highlight in the \n> community-supported universe repository is not allowed to be a build \n> dependency ...\n\nThe reasoning and solution Ubuntu has sounds sensible *but* it also\nsoudns like it is incomplete.  If Ubuntu does not want to use\nhighlight, it can apply a change like the patch in question as part\nof their fork to make the end result consistent and they are failing\nto do so.  If the tooling do not use highlight, the source should\nnot require highlight, either.  It is ultimately their bug.\n\nIt however *is* our business, as their upstream, to make it easier\nfor distros that want to use and distros that do not want to depend\non highlight, and aiming for a solution that relieves Ubuntu or any\nother distros from needing to carry one more patch is a good thing.\n\nHow bad does the documentation look with the patch applied (I know\nhow bad it looks without source-highlight installed)?  If it is not\ntoo bad, then it sounds like a sensible solution to drop the\nhighlight markup unconditionally like the patch that started this\nthread does, taking the \"common denominator\" approach.  You seem to\nagree, and I do not object, either.\n\n> But I don’t that would be worth it just to make one page of the API \n> documentation a little more colorful (and it sounds like you agree).\n"},{"id":"242226","messageId":"alpine.DEB.2.02.1405191839230.44324@all-night-tool.MIT.EDU","threadId":"36683","inReplyTo":"xmqq4n0l3dc7.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Documentation/technical/api-hashmap: Remove source highlighting","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2014-05-19T23:40:35Z","receivedAt":"2014-05-19T23:40:35Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"On Mon, 19 May 2014, Junio C Hamano wrote:\n> If Ubuntu does not want to use highlight, it can apply a change like the \n> patch in question as part of their fork to make the end result \n> consistent and they are failing to do so.\n\nSure, Ubuntu can apply that patch, but the larger problem remains: if the \nUbuntu packaging is even slightly different from Debian’s, it is no longer \neligible for automatic synchronization from Debian.\n\nWhen that has happened in the past, the result was that the Ubuntu version \nlanguished far behind the Debian version, because Canonical doesn’t have \nthe manpower to manually merge every Debian update.  Ubuntu 9.04 shipped \nwith Git 1.6.0.4 instead of 1.6.2.4 because they had failed to update \ntheir packaging after patching out a dependency on cvsps.\n\nIf this Ubuntu nonsense were obstructing something important upstream, \nthen of course I would be yelling at Ubuntu to get their act together; in \nthat case, I filed a main inclusion report to get Canonical to officially \nsupport cvsps (https://bugs.launchpad.net/bugs/369111) so Ubuntu could \ndrop their patch.  But here we’re talking about syntax highlighting in one \npage of internal documentation that basically nobody is going to read, and \nthat argument wouldn’t carry any weight.\n\nWe could put the patch into Debian instead of Ubuntu to eliminate the \nUbuntu delta; Jonathan has been friendly enough to Ubuntu that I think he \nwould grudgingly agree.  But I’m submitting it upstream because other \ndistros will eventually run into the same problem: there’s no obvious cue \nthat source-highlight needs to be added as a new dependency besides a \nwarning message buried in the middle of the build log.\n\n> How bad does the documentation look with the patch applied (I know how \n> bad it looks without source-highlight installed)?  If it is not too bad, \n> then it sounds like a sensible solution to drop the highlight markup \n> unconditionally like the patch that started this thread does, taking the \n> \"common denominator\" approach.  You seem to agree, and I do not object, \n> either.\n\nOriginal version with syntax-highlight installed (pretty):\nhttp://web.mit.edu/andersk/Public/api-hashmap/old-highlight.html\n\nOriginal version with syntax-highlight missing (corrupted):\nhttp://web.mit.edu/andersk/Public/api-hashmap/old-no-highlight.html\n\nPatched version (boring but readable):\nhttp://web.mit.edu/andersk/Public/api-hashmap/patched.html\n\nAnders\n"},{"id":"242289","messageId":"xmqqiop01hn2.fsf@gitster.dls.corp.google.com","threadId":"36683","inReplyTo":"alpine.DEB.2.02.1405191839230.44324@all-night-tool.MIT.EDU","subject":"Re: [PATCH] Documentation/technical/api-hashmap: Remove source highlighting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-20T17:31:29Z","receivedAt":"2014-05-20T17:31:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anders Kaseorg <andersk@MIT.EDU> writes:\n\n>> How bad does the documentation look with the patch applied (I know how \n>> bad it looks without source-highlight installed)?  If it is not too bad, \n>> then it sounds like a sensible solution to drop the highlight markup \n>> unconditionally like the patch that started this thread does, taking the \n>> \"common denominator\" approach.  You seem to agree, and I do not object, \n>> either.\n>\n> Original version with syntax-highlight installed (pretty):\n> http://web.mit.edu/andersk/Public/api-hashmap/old-highlight.html\n>\n> Original version with syntax-highlight missing (corrupted):\n> http://web.mit.edu/andersk/Public/api-hashmap/old-no-highlight.html\n>\n> Patched version (boring but readable):\n> http://web.mit.edu/andersk/Public/api-hashmap/patched.html\n\nThanks.  I've queued the patch for v2.0 and the comparison between\nthe first and the third clearly shows that it is the right thing to\ndo ;-).\n"}]}