{"thread":{"id":"35825","subject":"[PATCH] gitweb: Added syntax highlight support for golang","startedAt":"2014-02-07T21:10:41Z","lastAt":"2014-02-07T23:10:59Z","messageCount":6,"participants":["Pavan Kumar Sunkara","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"234470","messageId":"1391807441-23049-1-git-send-email-pavan.sss1991@gmail.com","threadId":"35825","inReplyTo":null,"subject":"[PATCH] gitweb: Added syntax highlight support for golang","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2014-02-07T21:10:41Z","receivedAt":"2014-02-07T21:10:41Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"Golang is quickly becoming one of the major programming languages.\n\nThis change switches on golang syntax highlight support by default\nin gitweb rather than asking the users to do it using config files.\n\nSigned-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>\n---\n gitweb/gitweb.perl |    2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex bf7fd67..aa6fcfd 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -273,7 +273,7 @@ our %highlight_basename = (\n our %highlight_ext = (\n \t# main extensions, defining name of syntax;\n \t# see files in /usr/share/highlight/langDefs/ directory\n-\t(map { $_ => $_ } qw(py rb java css js tex bib xml awk bat ini spec tcl sql)),\n+\t(map { $_ => $_ } qw(py rb java go css js tex bib xml awk bat ini spec tcl sql)),\n \t# alternate extensions, see /etc/highlight/filetypes.conf\n \t(map { $_ => 'c'   } qw(c h)),\n \t(map { $_ => 'sh'  } qw(sh bash zsh ksh)),\n-- \n1.7.10.4\n"},{"id":"234476","messageId":"xmqqiosqtwqk.fsf@gitster.dls.corp.google.com","threadId":"35825","inReplyTo":"1391807441-23049-1-git-send-email-pavan.sss1991@gmail.com","subject":"Re: [PATCH] gitweb: Added syntax highlight support for golang","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-07T21:54:11Z","receivedAt":"2014-02-07T21:54:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pavan Kumar Sunkara <pavan.sss1991@gmail.com> writes:\n\n> Golang is quickly becoming one of the major programming languages.\n>\n> This change switches on golang syntax highlight support by default\n> in gitweb rather than asking the users to do it using config files.\n\nLooks trivially harmless ;-)\n\nI haven't touched this part of our system, but the patch makes me\nwonder if there is a way for us to _ask_ the installed 'highlight'\nbinary what languages it knows about.  This hash is used only in\nguess_file_syntax sub, and it may not be unreasonable to populate it\nlazily there, or at least generate this part by parsing output from\n'highlight -p' at build-install time.\n\n> Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>\n> ---\n>  gitweb/gitweb.perl |    2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index bf7fd67..aa6fcfd 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -273,7 +273,7 @@ our %highlight_basename = (\n>  our %highlight_ext = (\n>  \t# main extensions, defining name of syntax;\n>  \t# see files in /usr/share/highlight/langDefs/ directory\n> -\t(map { $_ => $_ } qw(py rb java css js tex bib xml awk bat ini spec tcl sql)),\n> +\t(map { $_ => $_ } qw(py rb java go css js tex bib xml awk bat ini spec tcl sql)),\n>  \t# alternate extensions, see /etc/highlight/filetypes.conf\n>  \t(map { $_ => 'c'   } qw(c h)),\n>  \t(map { $_ => 'sh'  } qw(sh bash zsh ksh)),\n"},{"id":"234477","messageId":"CAK9CXBXXge+ZGN_ocWMH5jkPJcTg74rhtWsDiOuqAeeGXDW_tg@mail.gmail.com","threadId":"35825","inReplyTo":"xmqqiosqtwqk.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] gitweb: Added syntax highlight support for golang","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2014-02-07T21:56:05Z","receivedAt":"2014-02-07T21:56:05Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"The highlight project which is being used by gitweb supports this. I\nchecked it before submitting the patch.\n\nThanks\n\nOn Sat, Feb 8, 2014 at 3:24 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Pavan Kumar Sunkara <pavan.sss1991@gmail.com> writes:\n>\n>> Golang is quickly becoming one of the major programming languages.\n>>\n>> This change switches on golang syntax highlight support by default\n>> in gitweb rather than asking the users to do it using config files.\n>\n> Looks trivially harmless ;-)\n>\n> I haven't touched this part of our system, but the patch makes me\n> wonder if there is a way for us to _ask_ the installed 'highlight'\n> binary what languages it knows about.  This hash is used only in\n> guess_file_syntax sub, and it may not be unreasonable to populate it\n> lazily there, or at least generate this part by parsing output from\n> 'highlight -p' at build-install time.\n>\n>> Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>\n>> ---\n>>  gitweb/gitweb.perl |    2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n>> index bf7fd67..aa6fcfd 100755\n>> --- a/gitweb/gitweb.perl\n>> +++ b/gitweb/gitweb.perl\n>> @@ -273,7 +273,7 @@ our %highlight_basename = (\n>>  our %highlight_ext = (\n>>       # main extensions, defining name of syntax;\n>>       # see files in /usr/share/highlight/langDefs/ directory\n>> -     (map { $_ => $_ } qw(py rb java css js tex bib xml awk bat ini spec tcl sql)),\n>> +     (map { $_ => $_ } qw(py rb java go css js tex bib xml awk bat ini spec tcl sql)),\n>>       # alternate extensions, see /etc/highlight/filetypes.conf\n>>       (map { $_ => 'c'   } qw(c h)),\n>>       (map { $_ => 'sh'  } qw(sh bash zsh ksh)),\n\n\n\n-- \n- Pavan Kumar Sunkara\n"},{"id":"234478","messageId":"CAK9CXBUewMhr19KarmymOXmoq1ijPuU2mq4hqc6a-W0TCq5SRg@mail.gmail.com","threadId":"35825","inReplyTo":"CAK9CXBXXge+ZGN_ocWMH5jkPJcTg74rhtWsDiOuqAeeGXDW_tg@mail.gmail.com","subject":"Re: [PATCH] gitweb: Added syntax highlight support for golang","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2014-02-07T21:58:44Z","receivedAt":"2014-02-07T21:58:44Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"Sorry. I misunderstood your message. Yes, I guess lazy loading the\nsupported file extensions would be better. But not all highlighters\nsupport `-p` option. So, I think its better to leave it to the user.\n\nThanks\n"},{"id":"234481","messageId":"xmqqa9e2ttln.fsf@gitster.dls.corp.google.com","threadId":"35825","inReplyTo":"CAK9CXBUewMhr19KarmymOXmoq1ijPuU2mq4hqc6a-W0TCq5SRg@mail.gmail.com","subject":"Re: [PATCH] gitweb: Added syntax highlight support for golang","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-02-07T23:01:56Z","receivedAt":"2014-02-07T23:01:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pavan Kumar Sunkara <pavan.sss1991@gmail.com> writes:\n\n> Sorry. I misunderstood your message. Yes, I guess lazy loading the\n> supported file extensions would be better. But not all highlighters\n> support `-p` option. So, I think its better to leave it to the user.\n\nYes, those highlighters that do not support `-p` may have to rely on\nthe hard-coded list %highlight_ext.\n\nBut with the same line of reasoning, not all versions of highligher\nsupports 'go' language, so it's better to leave that to the user,\nno?  The version of 'highlight' you may have may know about 'go',\nand somebody else's 'highlight' may not yet.  A hard-coded list that\nappears in %highlight_ext will be correct for only one of you while\nthe other between you two needs to customize it to his system.\n\nNote that I was not talking about removing the configurability.\nEven with lazy loading and/or auto-genearting at build-install time\nwhen 'highlight -p' is available, the users still want to be able to\ncustomize, and supporting that is fine.\n\nBut for those whose 'highlight' does support '-p', it will help to\nlazily discover the list of supported languages and/or enumarate\nthem at build-install time.  They do not have to keep adding new\nlanguage (or removing it from the list we give as the upstream) to\nadjust it to their system.\n\nIn any case, the comment was not about this patch from you, but\nabout the future direction for the code it touches in general.  In\nother words, it did not mean \"because it does not update the\nmechanism to lazily discover the list of languages, and instead\nadded yet another language to the existing one, it is not an\nacceptable solution to start supporting 'go'\".\n"},{"id":"234482","messageId":"CAK9CXBWiaORDJaTam=X02UiBCsF2m_kW33_DqbQSLfoY1JphiQ@mail.gmail.com","threadId":"35825","inReplyTo":"xmqqa9e2ttln.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] gitweb: Added syntax highlight support for golang","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2014-02-07T23:10:59Z","receivedAt":"2014-02-07T23:10:59Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"Yeah. I agree with you.\n\nI am currently looking into allowing users to customize the parameters\ngiven to their highlighter. I will try to look into this.\n\nThanks\n\nOn Sat, Feb 8, 2014 at 4:31 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Pavan Kumar Sunkara <pavan.sss1991@gmail.com> writes:\n>\n>> Sorry. I misunderstood your message. Yes, I guess lazy loading the\n>> supported file extensions would be better. But not all highlighters\n>> support `-p` option. So, I think its better to leave it to the user.\n>\n> Yes, those highlighters that do not support `-p` may have to rely on\n> the hard-coded list %highlight_ext.\n>\n> But with the same line of reasoning, not all versions of highligher\n> supports 'go' language, so it's better to leave that to the user,\n> no?  The version of 'highlight' you may have may know about 'go',\n> and somebody else's 'highlight' may not yet.  A hard-coded list that\n> appears in %highlight_ext will be correct for only one of you while\n> the other between you two needs to customize it to his system.\n>\n> Note that I was not talking about removing the configurability.\n> Even with lazy loading and/or auto-genearting at build-install time\n> when 'highlight -p' is available, the users still want to be able to\n> customize, and supporting that is fine.\n>\n> But for those whose 'highlight' does support '-p', it will help to\n> lazily discover the list of supported languages and/or enumarate\n> them at build-install time.  They do not have to keep adding new\n> language (or removing it from the list we give as the upstream) to\n> adjust it to their system.\n>\n> In any case, the comment was not about this patch from you, but\n> about the future direction for the code it touches in general.  In\n> other words, it did not mean \"because it does not update the\n> mechanism to lazily discover the list of languages, and instead\n> added yet another language to the existing one, it is not an\n> acceptable solution to start supporting 'go'\".\n\n\n\n-- \n- Pavan Kumar Sunkara\n"}]}