git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] gitweb: Added syntax highlight support for golang
@ 2014-02-07 21:10 Pavan Kumar Sunkara
  2014-02-07 21:54 ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Pavan Kumar Sunkara @ 2014-02-07 21:10 UTC (permalink / raw)
  To: git; +Cc: Pavan Kumar Sunkara

Golang is quickly becoming one of the major programming languages.

This change switches on golang syntax highlight support by default
in gitweb rather than asking the users to do it using config files.

Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>
---
 gitweb/gitweb.perl |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index bf7fd67..aa6fcfd 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -273,7 +273,7 @@ our %highlight_basename = (
 our %highlight_ext = (
 	# main extensions, defining name of syntax;
 	# see files in /usr/share/highlight/langDefs/ directory
-	(map { $_ => $_ } qw(py rb java css js tex bib xml awk bat ini spec tcl sql)),
+	(map { $_ => $_ } qw(py rb java go css js tex bib xml awk bat ini spec tcl sql)),
 	# alternate extensions, see /etc/highlight/filetypes.conf
 	(map { $_ => 'c'   } qw(c h)),
 	(map { $_ => 'sh'  } qw(sh bash zsh ksh)),
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] gitweb: Added syntax highlight support for golang
  2014-02-07 21:10 [PATCH] gitweb: Added syntax highlight support for golang Pavan Kumar Sunkara
@ 2014-02-07 21:54 ` Junio C Hamano
  2014-02-07 21:56   ` Pavan Kumar Sunkara
  0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2014-02-07 21:54 UTC (permalink / raw)
  To: Pavan Kumar Sunkara; +Cc: git, Johannes Schindelin, Jakub Narębski

Pavan Kumar Sunkara <pavan.sss1991@gmail.com> writes:

> Golang is quickly becoming one of the major programming languages.
>
> This change switches on golang syntax highlight support by default
> in gitweb rather than asking the users to do it using config files.

Looks trivially harmless ;-)

I haven't touched this part of our system, but the patch makes me
wonder if there is a way for us to _ask_ the installed 'highlight'
binary what languages it knows about.  This hash is used only in
guess_file_syntax sub, and it may not be unreasonable to populate it
lazily there, or at least generate this part by parsing output from
'highlight -p' at build-install time.

> Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>
> ---
>  gitweb/gitweb.perl |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index bf7fd67..aa6fcfd 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -273,7 +273,7 @@ our %highlight_basename = (
>  our %highlight_ext = (
>  	# main extensions, defining name of syntax;
>  	# see files in /usr/share/highlight/langDefs/ directory
> -	(map { $_ => $_ } qw(py rb java css js tex bib xml awk bat ini spec tcl sql)),
> +	(map { $_ => $_ } qw(py rb java go css js tex bib xml awk bat ini spec tcl sql)),
>  	# alternate extensions, see /etc/highlight/filetypes.conf
>  	(map { $_ => 'c'   } qw(c h)),
>  	(map { $_ => 'sh'  } qw(sh bash zsh ksh)),

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gitweb: Added syntax highlight support for golang
  2014-02-07 21:54 ` Junio C Hamano
@ 2014-02-07 21:56   ` Pavan Kumar Sunkara
  2014-02-07 21:58     ` Pavan Kumar Sunkara
  0 siblings, 1 reply; 6+ messages in thread
From: Pavan Kumar Sunkara @ 2014-02-07 21:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List, Johannes Schindelin, Jakub Narębski

The highlight project which is being used by gitweb supports this. I
checked it before submitting the patch.

Thanks

On Sat, Feb 8, 2014 at 3:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Pavan Kumar Sunkara <pavan.sss1991@gmail.com> writes:
>
>> Golang is quickly becoming one of the major programming languages.
>>
>> This change switches on golang syntax highlight support by default
>> in gitweb rather than asking the users to do it using config files.
>
> Looks trivially harmless ;-)
>
> I haven't touched this part of our system, but the patch makes me
> wonder if there is a way for us to _ask_ the installed 'highlight'
> binary what languages it knows about.  This hash is used only in
> guess_file_syntax sub, and it may not be unreasonable to populate it
> lazily there, or at least generate this part by parsing output from
> 'highlight -p' at build-install time.
>
>> Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>
>> ---
>>  gitweb/gitweb.perl |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
>> index bf7fd67..aa6fcfd 100755
>> --- a/gitweb/gitweb.perl
>> +++ b/gitweb/gitweb.perl
>> @@ -273,7 +273,7 @@ our %highlight_basename = (
>>  our %highlight_ext = (
>>       # main extensions, defining name of syntax;
>>       # see files in /usr/share/highlight/langDefs/ directory
>> -     (map { $_ => $_ } qw(py rb java css js tex bib xml awk bat ini spec tcl sql)),
>> +     (map { $_ => $_ } qw(py rb java go css js tex bib xml awk bat ini spec tcl sql)),
>>       # alternate extensions, see /etc/highlight/filetypes.conf
>>       (map { $_ => 'c'   } qw(c h)),
>>       (map { $_ => 'sh'  } qw(sh bash zsh ksh)),



-- 
- Pavan Kumar Sunkara

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gitweb: Added syntax highlight support for golang
  2014-02-07 21:56   ` Pavan Kumar Sunkara
@ 2014-02-07 21:58     ` Pavan Kumar Sunkara
  2014-02-07 23:01       ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Pavan Kumar Sunkara @ 2014-02-07 21:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List, Johannes Schindelin, Jakub Narębski

Sorry. I misunderstood your message. Yes, I guess lazy loading the
supported file extensions would be better. But not all highlighters
support `-p` option. So, I think its better to leave it to the user.

Thanks

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gitweb: Added syntax highlight support for golang
  2014-02-07 21:58     ` Pavan Kumar Sunkara
@ 2014-02-07 23:01       ` Junio C Hamano
  2014-02-07 23:10         ` Pavan Kumar Sunkara
  0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2014-02-07 23:01 UTC (permalink / raw)
  To: Pavan Kumar Sunkara; +Cc: Git List, Johannes Schindelin, Jakub Narębski

Pavan Kumar Sunkara <pavan.sss1991@gmail.com> writes:

> Sorry. I misunderstood your message. Yes, I guess lazy loading the
> supported file extensions would be better. But not all highlighters
> support `-p` option. So, I think its better to leave it to the user.

Yes, those highlighters that do not support `-p` may have to rely on
the hard-coded list %highlight_ext.

But with the same line of reasoning, not all versions of highligher
supports 'go' language, so it's better to leave that to the user,
no?  The version of 'highlight' you may have may know about 'go',
and somebody else's 'highlight' may not yet.  A hard-coded list that
appears in %highlight_ext will be correct for only one of you while
the other between you two needs to customize it to his system.

Note that I was not talking about removing the configurability.
Even with lazy loading and/or auto-genearting at build-install time
when 'highlight -p' is available, the users still want to be able to
customize, and supporting that is fine.

But for those whose 'highlight' does support '-p', it will help to
lazily discover the list of supported languages and/or enumarate
them at build-install time.  They do not have to keep adding new
language (or removing it from the list we give as the upstream) to
adjust it to their system.

In any case, the comment was not about this patch from you, but
about the future direction for the code it touches in general.  In
other words, it did not mean "because it does not update the
mechanism to lazily discover the list of languages, and instead
added yet another language to the existing one, it is not an
acceptable solution to start supporting 'go'".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] gitweb: Added syntax highlight support for golang
  2014-02-07 23:01       ` Junio C Hamano
@ 2014-02-07 23:10         ` Pavan Kumar Sunkara
  0 siblings, 0 replies; 6+ messages in thread
From: Pavan Kumar Sunkara @ 2014-02-07 23:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List, Johannes Schindelin, Jakub Narębski

Yeah. I agree with you.

I am currently looking into allowing users to customize the parameters
given to their highlighter. I will try to look into this.

Thanks

On Sat, Feb 8, 2014 at 4:31 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Pavan Kumar Sunkara <pavan.sss1991@gmail.com> writes:
>
>> Sorry. I misunderstood your message. Yes, I guess lazy loading the
>> supported file extensions would be better. But not all highlighters
>> support `-p` option. So, I think its better to leave it to the user.
>
> Yes, those highlighters that do not support `-p` may have to rely on
> the hard-coded list %highlight_ext.
>
> But with the same line of reasoning, not all versions of highligher
> supports 'go' language, so it's better to leave that to the user,
> no?  The version of 'highlight' you may have may know about 'go',
> and somebody else's 'highlight' may not yet.  A hard-coded list that
> appears in %highlight_ext will be correct for only one of you while
> the other between you two needs to customize it to his system.
>
> Note that I was not talking about removing the configurability.
> Even with lazy loading and/or auto-genearting at build-install time
> when 'highlight -p' is available, the users still want to be able to
> customize, and supporting that is fine.
>
> But for those whose 'highlight' does support '-p', it will help to
> lazily discover the list of supported languages and/or enumarate
> them at build-install time.  They do not have to keep adding new
> language (or removing it from the list we give as the upstream) to
> adjust it to their system.
>
> In any case, the comment was not about this patch from you, but
> about the future direction for the code it touches in general.  In
> other words, it did not mean "because it does not update the
> mechanism to lazily discover the list of languages, and instead
> added yet another language to the existing one, it is not an
> acceptable solution to start supporting 'go'".



-- 
- Pavan Kumar Sunkara

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-02-07 23:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-07 21:10 [PATCH] gitweb: Added syntax highlight support for golang Pavan Kumar Sunkara
2014-02-07 21:54 ` Junio C Hamano
2014-02-07 21:56   ` Pavan Kumar Sunkara
2014-02-07 21:58     ` Pavan Kumar Sunkara
2014-02-07 23:01       ` Junio C Hamano
2014-02-07 23:10         ` Pavan Kumar Sunkara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).