{"thread":{"id":"44027","subject":"[PATCH] gitweb: use highlight's shebang detection","startedAt":"2016-09-06T19:02:14Z","lastAt":"2016-09-28T07:37:41Z","messageCount":23,"participants":["Ian Kelling","Jakub Narębski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"301227","messageId":"20160906190037.12442-1-ian@iankelling.org","threadId":"44027","inReplyTo":null,"subject":"[PATCH] gitweb: use highlight's shebang detection","fromName":"Ian Kelling","fromEmail":"ian@iankelling.org","sentAt":"2016-09-06T19:00:37Z","receivedAt":"2016-09-06T19:02:14Z","isPatch":true,"sender":{"key":"ian@iankelling.org","avatar":"https://avatars.githubusercontent.com/u/54232?v=4"},"body":"The highlight binary can detect language by shebang when we can't tell\nthe syntax type by the name of the file. To use highlight's shebang\ndetection, add highlight to the pipeline whenever highlight is enabled.\n\nDocument the shebang detection and add a test which exercises it in\nt/t9500-gitweb-standalone-no-errors.sh.\n\nSigned-off-by: Ian Kelling \n---\n\nNotes:\n I wondered if adding highlight to the pipeline would make viewing a blob\n with no highlighting take longer but it did not on my computer. I found\n no noticeable impact on small files and strangely, on a 159k file, it\n took 7% less time averaged over several requests.\n\n Documentation/gitweb.conf.txt | 21 ++++++++++++++-------\n gitweb/gitweb.perl | 10 +++++-----\n t/t9500-gitweb-standalone-no-errors.sh | 18 +++++++++++++-----\n 3 files changed, 32 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\nindex a79e350..e632089 100644\n--- a/Documentation/gitweb.conf.txt\n+++ b/Documentation/gitweb.conf.txt\n@@ -246,13 +246,20 @@ $highlight_bin::\n \tNote that 'highlight' feature must be set for gitweb to actually\n \tuse syntax highlighting.\n +\n-*NOTE*: if you want to add support for new file type (supported by\n-\"highlight\" but not used by gitweb), you need to modify `%highlight_ext`\n-or `%highlight_basename`, depending on whether you detect type of file\n-based on extension (for example \"sh\") or on its basename (for example\n-\"Makefile\"). The keys of these hashes are extension and basename,\n-respectively, and value for given key is name of syntax to be passed via\n-`--syntax ` to highlighter.\n+*NOTE*: for a file to be highlighted, its syntax type must be detected\n+and that syntax must be supported by \"highlight\". The default syntax\n+detection is minimal, and there are many supported syntax types with no\n+detection by default. There are three options for adding syntax\n+detection. The first and second priority are `%highlight_basename` and\n+`%highlight_ext`, which detect based on basename (the full filename, for\n+example \"Makefile\") and extension (for example \"sh\"). The keys of these\n+hashes are the basename and extension, respectively, and the value for a\n+given key is the name of the syntax to be passed via `--syntax `\n+to \"highlight\". The last priority is the \"highlight\" configuration of\n+`Shebang` regular expressions to detect the language based on the first\n+line in the file, (for example, matching the line \"#!/bin/bash\"). See\n+the highlight documentation and the default config at\n+/etc/highlight/filetypes.conf for more details.\n +\n For example if repositories you are hosting use \"phtml\" extension for\n PHP files, and you want to have correct syntax-highlighting for those\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 33d701d..a672181 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3931,15 +3931,16 @@ sub guess_file_syntax {\n # or return original FD if no highlighting\n sub run_highlighter {\n \tmy ($fd, $highlight, $syntax) = @_;\n-\treturn $fd unless ($highlight && defined $syntax);\n+\treturn $fd unless ($highlight);\n \n \tclose $fd;\n+\tmy $syntax_arg = (defined $syntax) ? \"--syntax $syntax\" : \"--force\";\n \topen $fd, quote_command(git_cmd(), \"cat-file\", \"blob\", $hash).\" | \".\n \t quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',\n \t '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',\n \t '--', \"-fe=$fallback_encoding\").\" | \".\n \t quote_command($highlight_bin).\n-\t \" --replace-tabs=8 --fragment --syntax $syntax |\"\n+\t \" --replace-tabs=8 --fragment $syntax_arg |\"\n \t\tor die_error(500, \"Couldn't open file or run syntax highlighter\");\n \treturn $fd;\n }\n@@ -7063,8 +7064,7 @@ sub git_blob {\n \n \tmy $highlight = gitweb_check_feature('highlight');\n \tmy $syntax = guess_file_syntax($highlight, $mimetype, $file_name);\n-\t$fd = run_highlighter($fd, $highlight, $syntax)\n-\t\tif $syntax;\n+\t$fd = run_highlighter($fd, $highlight, $syntax);\n \n \tgit_header_html(undef, $expires);\n \tmy $formats_nav = '';\n@@ -7117,7 +7117,7 @@ sub git_blob {\n \t\t\t$line = untabify($line);\n \t\t\tprintf qq!
%4i %s
\\n!,\n \t\t\t $nr, esc_attr(href(-replay => 1)), $nr, $nr,\n-\t\t\t $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);\n+\t\t\t $highlight ? sanitize($line) : esc_html($line, -nbsp=>1);\n \t\t}\n \t}\n \tclose $fd\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex e94b2f1..9e5fcfe 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -702,12 +702,20 @@ test_expect_success HIGHLIGHT \\\n \t gitweb_run \"p=.git;a=blob;f=file\"'\n \n test_expect_success HIGHLIGHT \\\n-\t'syntax highlighting (highlighted, shell script)' \\\n+\t'syntax highlighting (highlighted, shell script shebang)' \\\n \t'git config gitweb.highlight yes &&\n-\t echo \"#!/usr/bin/sh\" > test.sh &&\n-\t git add test.sh &&\n-\t git commit -m \"Add test.sh\" &&\n-\t gitweb_run \"p=.git;a=blob;f=test.sh\"'\n+\t echo \"#!/usr/bin/sh\" > test &&\n+\t git add test &&\n+\t git commit -m \"Add test\" &&\n+\t gitweb_run \"p=.git;a=blob;f=test\"'\n+\n+test_expect_success HIGHLIGHT \\\n+\t'syntax highlighting (highlighted, header file)' \\\n+\t'git config gitweb.highlight yes &&\n+\t echo \"#define ANSWER 42\" > test.h &&\n+\t git add test.h &&\n+\t git commit -m \"Add test.h\" &&\n+\t gitweb_run \"p=.git;a=blob;f=test.h\"'\n \n # ----------------------------------------------------------------------\n # forks of projects\n-- \n2.9.3\n\n"},{"id":"302249","messageId":"108ce713-337a-801a-6c3b-089ef25a3883@gmail.com","threadId":"44027","inReplyTo":"20160906190037.12442-1-ian@iankelling.org","subject":"Re: [PATCH] gitweb: use highlight's shebang detection","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-09-20T20:22:48Z","receivedAt":"2016-09-20T20:23:11Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 06.09.2016 o 21:00, Ian Kelling pisze:\n\n> The highlight binary can detect language by shebang when we can't tell\n> the syntax type by the name of the file. \n\nWas it something always present among highlight[1] binary capabilities,\nor is it something present only in new enough highlight app? Or only\nin some specific fork / specific binary? I couldn't find language\ndetection in highlight[1] documentation...\n\n[1]: http://www.andre-simon.de/doku/highlight/en/highlight.php\n\nIf this feature is available only for some version, or for some\nhighlighters, gitweb would have to provide an option to configure\nit. It might be an additional configuration variable, it might\nbe a special value in the %highlight_basename or %highlight_ext.\n\n> To use highlight's shebang\n> detection, add highlight to the pipeline whenever highlight is enabled.\n\nThis describes what this patch does, but the sentence feels\na bit convoluted, as it is stated.\n\n> \n> Document the shebang detection and add a test which exercises it in\n> t/t9500-gitweb-standalone-no-errors.sh.\n\nNice!\n\n> \n> Signed-off-by: Ian Kelling \n> ---\n> \n> Notes:\n> I wondered if adding highlight to the pipeline would make viewing a blob\n> with no highlighting take longer but it did not on my computer. I found\n> no noticeable impact on small files and strangely, on a 159k file, it\n> took 7% less time averaged over several requests.\n\nStrange. I would guess that invoking separate binary and perl would always\nadd to the time (especially on operation systems where forking / running\ncommand is expensive... though those are not often used with web servers,\nisn't it).\n\n> \n> Documentation/gitweb.conf.txt | 21 ++++++++++++++-------\n> gitweb/gitweb.perl | 10 +++++-----\n> t/t9500-gitweb-standalone-no-errors.sh | 18 +++++++++++++-----\n> 3 files changed, 32 insertions(+), 17 deletions(-)\n> \n> diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\n> index a79e350..e632089 100644\n> --- a/Documentation/gitweb.conf.txt\n> +++ b/Documentation/gitweb.conf.txt\n> @@ -246,13 +246,20 @@ $highlight_bin::\n> \tNote that 'highlight' feature must be set for gitweb to actually\n> \tuse syntax highlighting.\n> +\n> -*NOTE*: if you want to add support for new file type (supported by\n> -\"highlight\" but not used by gitweb), you need to modify `%highlight_ext`\n> -or `%highlight_basename`, depending on whether you detect type of file\n> -based on extension (for example \"sh\") or on its basename (for example\n> -\"Makefile\"). The keys of these hashes are extension and basename,\n> -respectively, and value for given key is name of syntax to be passed via\n> -`--syntax ` to highlighter.\n> +*NOTE*: for a file to be highlighted, its syntax type must be detected\n> +and that syntax must be supported by \"highlight\". The default syntax\n> +detection is minimal, and there are many supported syntax types with no\n> +detection by default. There are three options for adding syntax\n> +detection. The first and second priority are `%highlight_basename` and\n> +`%highlight_ext`, which detect based on basename (the full filename, for\n> +example \"Makefile\") and extension (for example \"sh\"). The keys of these\n> +hashes are the basename and extension, respectively, and the value for a\n> +given key is the name of the syntax to be passed via `--syntax `\n> +to \"highlight\". The last priority is the \"highlight\" configuration of\n> +`Shebang` regular expressions to detect the language based on the first\n> +line in the file, (for example, matching the line \"#!/bin/bash\"). See\n> +the highlight documentation and the default config at\n> +/etc/highlight/filetypes.conf for more details.\n\nAll right; in addition to expanding the docs, it also improves them.\n\n> +\n> For example if repositories you are hosting use \"phtml\" extension for\n> PHP files, and you want to have correct syntax-highlighting for those\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 33d701d..a672181 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -3931,15 +3931,16 @@ sub guess_file_syntax {\n> # or return original FD if no highlighting\n> sub run_highlighter {\n> \tmy ($fd, $highlight, $syntax) = @_;\n> -\treturn $fd unless ($highlight && defined $syntax);\n> +\treturn $fd unless ($highlight);\n\nHere we would have check if we want / can invoke \"highlight\".\n\n> \n> \tclose $fd;\n> +\tmy $syntax_arg = (defined $syntax) ? \"--syntax $syntax\" : \"--force\";\n> \topen $fd, quote_command(git_cmd(), \"cat-file\", \"blob\", $hash).\" | \".\n> \t quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',\n> \t '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',\n> \t '--', \"-fe=$fallback_encoding\").\" | \".\n> \t quote_command($highlight_bin).\n> -\t \" --replace-tabs=8 --fragment --syntax $syntax |\"\n> +\t \" --replace-tabs=8 --fragment $syntax_arg |\"\n> \t\tor die_error(500, \"Couldn't open file or run syntax highlighter\");\n> \treturn $fd;\n> }\n\nAll right (well, except for the question asked at the beginning).\n\n> @@ -7063,8 +7064,7 @@ sub git_blob {\n> \n> \tmy $highlight = gitweb_check_feature('highlight');\n> \tmy $syntax = guess_file_syntax($highlight, $mimetype, $file_name);\n> -\t$fd = run_highlighter($fd, $highlight, $syntax)\n> -\t\tif $syntax;\n\nHmmm... it looks like the old code checked if there was $syntax defined\ntwice: once for truthy value in caller, once for definedness in run_highlighter().\n\n> +\t$fd = run_highlighter($fd, $highlight, $syntax);\n\nAll right.\n\n> \n> \tgit_header_html(undef, $expires);\n> \tmy $formats_nav = '';\n> @@ -7117,7 +7117,7 @@ sub git_blob {\n> \t\t\t$line = untabify($line);\n> \t\t\tprintf qq!
%4i %s
\\n!,\n> \t\t\t $nr, esc_attr(href(-replay => 1)), $nr, $nr,\n> -\t\t\t $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);\n> +\t\t\t $highlight ? sanitize($line) : esc_html($line, -nbsp=>1);\n\nOh, well. It looks like checking if highlighter could be run in\nrun_highlight() is wrong, as the caller (that is, git_blob()) needs\nto know if it is using \"highlight\" output (which is HTML) or raw blob\ncontents (which needs to be HTML-escaped).\n\n> \t\t}\n> \t}\n> \tclose $fd\n> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\n> index e94b2f1..9e5fcfe 100755\n> --- a/t/t9500-gitweb-standalone-no-errors.sh\n> +++ b/t/t9500-gitweb-standalone-no-errors.sh\n> @@ -702,12 +702,20 @@ test_expect_success HIGHLIGHT \\\n> \t gitweb_run \"p=.git;a=blob;f=file\"'\n> \n> test_expect_success HIGHLIGHT \\\n> -\t'syntax highlighting (highlighted, shell script)' \\\n> +\t'syntax highlighting (highlighted, shell script shebang)' \\\n\nIt would be nice to have in test name that it checks if highlighter\nautodetection works, or at least doesn't crash gitweb.\n\n> \t'git config gitweb.highlight yes &&\n> -\t echo \"#!/usr/bin/sh\" > test.sh &&\n> -\t git add test.sh &&\n> -\t git commit -m \"Add test.sh\" &&\n> -\t gitweb_run \"p=.git;a=blob;f=test.sh\"'\n> +\t echo \"#!/usr/bin/sh\" > test &&\n> +\t git add test &&\n> +\t git commit -m \"Add test\" &&\n> +\t gitweb_run \"p=.git;a=blob;f=test\"'\n> +\n> +test_expect_success HIGHLIGHT \\\n> +\t'syntax highlighting (highlighted, header file)' \\\n\nDo we check explicit syntax knowledge (based on the extension),\nor autodetect again?\n\n> +\t'git config gitweb.highlight yes &&\n> +\t echo \"#define ANSWER 42\" > test.h &&\n> +\t git add test.h &&\n> +\t git commit -m \"Add test.h\" &&\n> +\t gitweb_run \"p=.git;a=blob;f=test.h\"'\n> \n> # ----------------------------------------------------------------------\n> # forks of projects\n> \n\nThank you for your work on this patch,\n-- \nJakub Narębski\n\n"},{"id":"302289","messageId":"xmqq4m59b43v.fsf@gitster.mtv.corp.google.com","threadId":"44027","inReplyTo":"108ce713-337a-801a-6c3b-089ef25a3883@gmail.com","subject":"Re: [PATCH] gitweb: use highlight's shebang detection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-21T16:38:12Z","receivedAt":"2016-09-21T16:38:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narębski writes:\n\n> W dniu 06.09.2016 o 21:00, Ian Kelling pisze:\n>\n>> The highlight binary can detect language by shebang when we can't tell\n>> the syntax type by the name of the file. \n>\n> Was it something always present among highlight[1] binary capabilities,\n> or is it something present only in new enough highlight app? Or only\n> in some specific fork / specific binary? I couldn't find language\n> detection in highlight[1] documentation...\n> ...\n> Thank you for your work on this patch,\n\nThanks for reviewing. It seems that there will be further exchange\nneeded before I can pick it up?\n"},{"id":"302303","messageId":"bef8e911-0f4a-2b42-3f98-98686bf2de44@gmail.com","threadId":"44027","inReplyTo":"xmqq4m59b43v.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] gitweb: use highlight's shebang detection","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-09-21T17:51:14Z","receivedAt":"2016-09-21T17:51:39Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 21.09.2016 o 18:38, Junio C Hamano pisze:\n> Jakub Narębski writes:\n>> W dniu 06.09.2016 o 21:00, Ian Kelling pisze:\n>>\n>>> The highlight binary can detect language by shebang when we can't tell\n>>> the syntax type by the name of the file. \n>>\n>> Was it something always present among highlight[1] binary capabilities,\n>> or is it something present only in new enough highlight app? Or only\n>> in some specific fork / specific binary? I couldn't find language\n>> detection in highlight[1] documentation...\n>> ...\n>> Thank you for your work on this patch,\n> \n> Thanks for reviewing. It seems that there will be further exchange\n> needed before I can pick it up?\n\nYes, I think so.\n\n-- \nJakub Narębski\n\n"},{"id":"302338","messageId":"1474496142.400086.733142417.560B3AFF@webmail.messagingengine.com","threadId":"44027","inReplyTo":"108ce713-337a-801a-6c3b-089ef25a3883@gmail.com","subject":"Re: [PATCH] gitweb: use highlight's shebang detection","fromName":"Ian Kelling","fromEmail":"ian@iankelling.org","sentAt":"2016-09-21T22:15:42Z","receivedAt":"2016-09-21T22:15:48Z","isPatch":true,"sender":{"key":"ian@iankelling.org","avatar":"https://avatars.githubusercontent.com/u/54232?v=4"},"body":"On Tue, Sep 20, 2016, at 01:22 PM, Jakub Narębski wrote:\n> W dniu 06.09.2016 o 21:00, Ian Kelling pisze:\n>\n> > The highlight binary can detect language by shebang when we can't tell\n> > the syntax type by the name of the file.\n>\n> Was it something always present among highlight[1] binary capabilities,\n> or is it something present only in new enough highlight app? Or only\n> in some specific fork / specific binary? I couldn't find language\n> detection in highlight[1] documentation...\n>\n> [1]: http://www.andre-simon.de/doku/highlight/en/highlight.php\n\nSearch for the word shebang, it's mentioned twice.\n\n>\n> If this feature is available only for some version, or for some\n> highlighters, gitweb would have to provide an option to configure\n> it. It might be an additional configuration variable, it might\n> be a special value in the %highlight_basename or %highlight_ext.\n\nGood question. It was added upstream in 2007, and I tested that it's\nfunctioning in the earliest distros I have easy access to: ubuntu 14.04\nand debian wheezy.\n\n>\n> > To use highlight's shebang\n> > detection, add highlight to the pipeline whenever highlight is enabled.\n>\n> This describes what this patch does, but the sentence feels\n> a bit convoluted, as it is stated.\n>\n\nAgreed. I've changed it in v2 of the patch, and perhaps this will make\nthe rest of the patch clearer too. The new paragraph is:\n\n The highlight binary can detect language by shebang when we can't\n tell\n the syntax type by the name of the file. In that case, pass the blob\n to \"highlight --force\" and the resulting html will have markup for\n highlighting if the language was detected.\n\n\n\n> >\n> > Document the shebang detection and add a test which exercises it in\n> > t/t9500-gitweb-standalone-no-errors.sh.\n>\n> Nice!\n>\n> >\n> > Signed-off-by: Ian Kelling \n> > ---\n> >\n> > Notes:\n> > I wondered if adding highlight to the pipeline would make viewing a blob\n> > with no highlighting take longer but it did not on my computer. I found\n> > no noticeable impact on small files and strangely, on a 159k file, it\n> > took 7% less time averaged over several requests.\n>\n> Strange. I would guess that invoking separate binary and perl would\n> always\n> add to the time (especially on operation systems where forking / running\n> command is expensive... though those are not often used with web servers,\n> isn't it).\n\nI dug into this a little more, and I think it's because when we call\nhighlight, we later call sanitize() instead of esc_html(). sanitize() is\nfaster and makes up for the extra time highlight takes. I ran a test on\nmy machine calling sanitize and esc_html on each line of gitweb.perl 100\ntimes: 7.4s for sanitize, 12.4s for esc_html.\n\n>\n> >\n> > Documentation/gitweb.conf.txt | 21 ++++++++++++++-------\n> > gitweb/gitweb.perl | 10 +++++-----\n> > t/t9500-gitweb-standalone-no-errors.sh | 18 +++++++++++++-----\n> > 3 files changed, 32 insertions(+), 17 deletions(-)\n> >\n> > diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\n> > index a79e350..e632089 100644\n> > --- a/Documentation/gitweb.conf.txt\n> > +++ b/Documentation/gitweb.conf.txt\n> > @@ -246,13 +246,20 @@ $highlight_bin::\n> > \tNote that 'highlight' feature must be set for gitweb to actually\n> > \tuse syntax highlighting.\n> > +\n> > -*NOTE*: if you want to add support for new file type (supported by\n> > -\"highlight\" but not used by gitweb), you need to modify `%highlight_ext`\n> > -or `%highlight_basename`, depending on whether you detect type of file\n> > -based on extension (for example \"sh\") or on its basename (for example\n> > -\"Makefile\"). The keys of these hashes are extension and basename,\n> > -respectively, and value for given key is name of syntax to be passed via\n> > -`--syntax ` to highlighter.\n> > +*NOTE*: for a file to be highlighted, its syntax type must be detected\n> > +and that syntax must be supported by \"highlight\". The default syntax\n> > +detection is minimal, and there are many supported syntax types with no\n> > +detection by default. There are three options for adding syntax\n> > +detection. The first and second priority are `%highlight_basename` and\n> > +`%highlight_ext`, which detect based on basename (the full filename, for\n> > +example \"Makefile\") and extension (for example \"sh\"). The keys of these\n> > +hashes are the basename and extension, respectively, and the value for a\n> > +given key is the name of the syntax to be passed via `--syntax `\n> > +to \"highlight\". The last priority is the \"highlight\" configuration of\n> > +`Shebang` regular expressions to detect the language based on the first\n> > +line in the file, (for example, matching the line \"#!/bin/bash\"). See\n> > +the highlight documentation and the default config at\n> > +/etc/highlight/filetypes.conf for more details.\n>\n> All right; in addition to expanding the docs, it also improves them.\n\nNoted in v2 commit log.\n\n>\n> > +\n> > For example if repositories you are hosting use \"phtml\" extension for\n> > PHP files, and you want to have correct syntax-highlighting for those\n> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> > index 33d701d..a672181 100755\n> > --- a/gitweb/gitweb.perl\n> > +++ b/gitweb/gitweb.perl\n> > @@ -3931,15 +3931,16 @@ sub guess_file_syntax {\n> > # or return original FD if no highlighting\n> > sub run_highlighter {\n> > \tmy ($fd, $highlight, $syntax) = @_;\n> > -\treturn $fd unless ($highlight && defined $syntax);\n> > +\treturn $fd unless ($highlight);\n>\n> Here we would have check if we want / can invoke \"highlight\".\n\nI think it's right as is. $highlight says the user wants highlighting,\nand now we still want to invoke it when we do not know $syntax.\n\nWhile I was double checking, I noticed there was an unused parameter to\nguess_file_syntax(), $mimetype, which could easily make this not\nobvious. I removed it in v2.\n\n>\n> >\n> > \tclose $fd;\n> > +\tmy $syntax_arg = (defined $syntax) ? \"--syntax $syntax\" : \"--force\";\n> > \topen $fd, quote_command(git_cmd(), \"cat-file\", \"blob\", $hash).\" | \".\n> > \t quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',\n> > \t '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',\n> > \t '--', \"-fe=$fallback_encoding\").\" | \".\n> > \t quote_command($highlight_bin).\n> > -\t \" --replace-tabs=8 --fragment --syntax $syntax |\"\n> > +\t \" --replace-tabs=8 --fragment $syntax_arg |\"\n> > \t\tor die_error(500, \"Couldn't open file or run syntax highlighter\");\n> > \treturn $fd;\n> > }\n>\n> All right (well, except for the question asked at the beginning).\n>\n> > @@ -7063,8 +7064,7 @@ sub git_blob {\n> >\n> > \tmy $highlight = gitweb_check_feature('highlight');\n> > \tmy $syntax = guess_file_syntax($highlight, $mimetype, $file_name);\n> > -\t$fd = run_highlighter($fd, $highlight, $syntax)\n> > -\t\tif $syntax;\n>\n> Hmmm... it looks like the old code checked if there was $syntax defined\n> twice: once for truthy value in caller, once for definedness in\n> run_highlighter().\n>\n> > +\t$fd = run_highlighter($fd, $highlight, $syntax);\n>\n> All right.\n>\n> >\n> > \tgit_header_html(undef, $expires);\n> > \tmy $formats_nav = '';\n> > @@ -7117,7 +7117,7 @@ sub git_blob {\n> > \t\t\t$line = untabify($line);\n> > \t\t\tprintf qq!
%4i %s
\\n!,\n> > \t\t\t $nr, esc_attr(href(-replay => 1)), $nr, $nr,\n> > -\t\t\t $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);\n> > +\t\t\t $highlight ? sanitize($line) : esc_html($line, -nbsp=>1);\n>\n> Oh, well. It looks like checking if highlighter could be run in\n> run_highlight() is wrong, as the caller (that is, git_blob()) needs\n> to know if it is using \"highlight\" output (which is HTML) or raw blob\n> contents (which needs to be HTML-escaped).\n\nPer previous comment, run_highlight() is right, and we use the same\ncondition here to know if the highlight binary was used. If highlight\nwas run with --force and did not detect a language in the shebang, it\nstill outputs html but without adding the highlight markup.\n\n>\n> > \t\t}\n> > \t}\n> > \tclose $fd\n> > diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\n> > index e94b2f1..9e5fcfe 100755\n> > --- a/t/t9500-gitweb-standalone-no-errors.sh\n> > +++ b/t/t9500-gitweb-standalone-no-errors.sh\n> > @@ -702,12 +702,20 @@ test_expect_success HIGHLIGHT \\\n> > \t gitweb_run \"p=.git;a=blob;f=file\"'\n> >\n> > test_expect_success HIGHLIGHT \\\n> > -\t'syntax highlighting (highlighted, shell script)' \\\n> > +\t'syntax highlighting (highlighted, shell script shebang)' \\\n>\n> It would be nice to have in test name that it checks if highlighter\n> autodetection works, or at least doesn't crash gitweb.\n\nI've updated it to:\nsyntax highlighting (highlighter language autodetection)\nI'm happy to use any suggestion you have.\n\n>\n> > \t'git config gitweb.highlight yes &&\n> > -\t echo \"#!/usr/bin/sh\" > test.sh &&\n> > -\t git add test.sh &&\n> > -\t git commit -m \"Add test.sh\" &&\n> > -\t gitweb_run \"p=.git;a=blob;f=test.sh\"'\n> > +\t echo \"#!/usr/bin/sh\" > test &&\n> > +\t git add test &&\n> > +\t git commit -m \"Add test\" &&\n> > +\t gitweb_run \"p=.git;a=blob;f=test\"'\n> > +\n> > +test_expect_success HIGHLIGHT \\\n> > +\t'syntax highlighting (highlighted, header file)' \\\n>\n> Do we check explicit syntax knowledge (based on the extension),\n> or autodetect again?\n\nWe have explicit syntax knowledge here. My thinking was this would\nmodify the existing test so that it highlights a different language than\nthe autodetected one, but the patch is simpler if I just make the\nautodetcted one be a different language. I've done that in v2.\n\n>\n> > +\t'git config gitweb.highlight yes &&\n> > +\t echo \"#define ANSWER 42\" > test.h &&\n> > +\t git add test.h &&\n> > +\t git commit -m \"Add test.h\" &&\n> > +\t gitweb_run \"p=.git;a=blob;f=test.h\"'\n> >\n> > # ----------------------------------------------------------------------\n> > # forks of projects\n> >\n>\n> Thank you for your work on this patch,\n> --\n> Jakub Narębski\n\nThank you for reviewing it!\n"},{"id":"302339","messageId":"20160921221856.27830-1-ian@iankelling.org","threadId":"44027","inReplyTo":"108ce713-337a-801a-6c3b-089ef25a3883@gmail.com","subject":"[PATCH] gitweb: use highlight's shebang detection","fromName":"Ian Kelling","fromEmail":"ian@iankelling.org","sentAt":"2016-09-21T22:18:56Z","receivedAt":"2016-09-21T22:19:04Z","isPatch":true,"sender":{"key":"ian@iankelling.org","avatar":"https://avatars.githubusercontent.com/u/54232?v=4"},"body":"The highlight binary can detect language by shebang when we can't tell\nthe syntax type by the name of the file. In that case, pass the blob\nto \"highlight --force\" and the resulting html will have markup for\nhighlighting if the language was detected.\n\nDocument the feature and improve syntax highlight documentation, add\ntest to ensure gitweb doesn't crash when language detection is used,\nand remove an unused parameter from gitweb_check_feature().\n\nSigned-off-by: Ian Kelling \n---\n Documentation/gitweb.conf.txt | 21 ++++++++++++++-------\n gitweb/gitweb.perl | 14 +++++++-------\n t/t9500-gitweb-standalone-no-errors.sh | 8 ++++++++\n 3 files changed, 29 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\nindex a79e350..e632089 100644\n--- a/Documentation/gitweb.conf.txt\n+++ b/Documentation/gitweb.conf.txt\n@@ -246,13 +246,20 @@ $highlight_bin::\n \tNote that 'highlight' feature must be set for gitweb to actually\n \tuse syntax highlighting.\n +\n-*NOTE*: if you want to add support for new file type (supported by\n-\"highlight\" but not used by gitweb), you need to modify `%highlight_ext`\n-or `%highlight_basename`, depending on whether you detect type of file\n-based on extension (for example \"sh\") or on its basename (for example\n-\"Makefile\"). The keys of these hashes are extension and basename,\n-respectively, and value for given key is name of syntax to be passed via\n-`--syntax ` to highlighter.\n+*NOTE*: for a file to be highlighted, its syntax type must be detected\n+and that syntax must be supported by \"highlight\". The default syntax\n+detection is minimal, and there are many supported syntax types with no\n+detection by default. There are three options for adding syntax\n+detection. The first and second priority are `%highlight_basename` and\n+`%highlight_ext`, which detect based on basename (the full filename, for\n+example \"Makefile\") and extension (for example \"sh\"). The keys of these\n+hashes are the basename and extension, respectively, and the value for a\n+given key is the name of the syntax to be passed via `--syntax `\n+to \"highlight\". The last priority is the \"highlight\" configuration of\n+`Shebang` regular expressions to detect the language based on the first\n+line in the file, (for example, matching the line \"#!/bin/bash\"). See\n+the highlight documentation and the default config at\n+/etc/highlight/filetypes.conf for more details.\n +\n For example if repositories you are hosting use \"phtml\" extension for\n PHP files, and you want to have correct syntax-highlighting for those\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 33d701d..44094f4 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3913,7 +3913,7 @@ sub blob_contenttype {\n # guess file syntax for syntax highlighting; return undef if no highlighting\n # the name of syntax can (in the future) depend on syntax highlighter used\n sub guess_file_syntax {\n-\tmy ($highlight, $mimetype, $file_name) = @_;\n+\tmy ($highlight, $file_name) = @_;\n \treturn undef unless ($highlight && defined $file_name);\n \tmy $basename = basename($file_name, '.in');\n \treturn $highlight_basename{$basename}\n@@ -3931,15 +3931,16 @@ sub guess_file_syntax {\n # or return original FD if no highlighting\n sub run_highlighter {\n \tmy ($fd, $highlight, $syntax) = @_;\n-\treturn $fd unless ($highlight && defined $syntax);\n+\treturn $fd unless ($highlight);\n \n \tclose $fd;\n+\tmy $syntax_arg = (defined $syntax) ? \"--syntax $syntax\" : \"--force\";\n \topen $fd, quote_command(git_cmd(), \"cat-file\", \"blob\", $hash).\" | \".\n \t quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',\n \t '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',\n \t '--', \"-fe=$fallback_encoding\").\" | \".\n \t quote_command($highlight_bin).\n-\t \" --replace-tabs=8 --fragment --syntax $syntax |\"\n+\t \" --replace-tabs=8 --fragment $syntax_arg |\"\n \t\tor die_error(500, \"Couldn't open file or run syntax highlighter\");\n \treturn $fd;\n }\n@@ -7062,9 +7063,8 @@ sub git_blob {\n \t$have_blame &&= ($mimetype =~ m!^text/!);\n \n \tmy $highlight = gitweb_check_feature('highlight');\n-\tmy $syntax = guess_file_syntax($highlight, $mimetype, $file_name);\n-\t$fd = run_highlighter($fd, $highlight, $syntax)\n-\t\tif $syntax;\n+\tmy $syntax = guess_file_syntax($highlight, $file_name);\n+\t$fd = run_highlighter($fd, $highlight, $syntax);\n \n \tgit_header_html(undef, $expires);\n \tmy $formats_nav = '';\n@@ -7117,7 +7117,7 @@ sub git_blob {\n \t\t\t$line = untabify($line);\n \t\t\tprintf qq!
%4i %s
\\n!,\n \t\t\t $nr, esc_attr(href(-replay => 1)), $nr, $nr,\n-\t\t\t $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);\n+\t\t\t $highlight ? sanitize($line) : esc_html($line, -nbsp=>1);\n \t\t}\n \t}\n \tclose $fd\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex e94b2f1..576db6d 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -709,6 +709,14 @@ test_expect_success HIGHLIGHT \\\n \t git commit -m \"Add test.sh\" &&\n \t gitweb_run \"p=.git;a=blob;f=test.sh\"'\n \n+test_expect_success HIGHLIGHT \\\n+\t'syntax highlighting (highlighter language autodetection)' \\\n+\t'git config gitweb.highlight yes &&\n+\t echo \"#!/usr/bin/ruby\" > test &&\n+\t git add test &&\n+\t git commit -m \"Add test\" &&\n+\t gitweb_run \"p=.git;a=blob;f=test\"'\n+\n # ----------------------------------------------------------------------\n # forks of projects\n \n-- \n2.9.3\n\n"},{"id":"302341","messageId":"1474496681.402026.733160001.6DBB5A8F@webmail.messagingengine.com","threadId":"44027","inReplyTo":"20160921221856.27830-1-ian@iankelling.org","subject":"Re: [PATCH] gitweb: use highlight's shebang detection","fromName":"Ian Kelling","fromEmail":"ian@iankelling.org","sentAt":"2016-09-21T22:24:41Z","receivedAt":"2016-09-21T22:24:46Z","isPatch":true,"sender":{"key":"ian@iankelling.org","avatar":"https://avatars.githubusercontent.com/u/54232?v=4"},"body":"fyi: I mistakenly did not include v2 in the subject of the last message.\n"},{"id":"302418","messageId":"2a5aead0-d521-85eb-f304-7091683d8b40@gmail.com","threadId":"44027","inReplyTo":"20160921221856.27830-1-ian@iankelling.org","subject":"Re: [PATCH v2] gitweb: use highlight's shebang detection","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-09-22T22:50:00Z","receivedAt":"2016-09-22T22:50:27Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 22.09.2016 o 00:18, Ian Kelling napisał:\n\n> The highlight binary can detect language by shebang when we can't tell\n> the syntax type by the name of the file. In that case, pass the blob\n> to \"highlight --force\" and the resulting html will have markup for\n> highlighting if the language was detected.\n\nThis description feels a bit convoluted. Perhaps something like this:\n\n The \"highlight\" binary can, in some cases, determine the language type\n by the means of file contents, for example the shebang in the first line\n for some scripting languages. Make use of this autodetection for files\n which syntax is not known by gitweb. In that case, pass the blob\n contents to \"highlight --force\"; the parameter is needed to make it\n always generate HTML output (which includes HTML-escaping).\n\nAlso, we might want to have the information about performance of this\nsolution either in the commit message, or in commit comments.\n\n> \n> Document the feature and improve syntax highlight documentation, add\n> test to ensure gitweb doesn't crash when language detection is used,\n\nAll right.\n\n> and remove an unused parameter from gitweb_check_feature().\n\nFirst, that is guess_file_syntax(), not gitweb_check_feature().\nSecond, this change could be made into independent patch, for example\npreparatory one.\n\n> \n> Signed-off-by: Ian Kelling \n> ---\n> Documentation/gitweb.conf.txt | 21 ++++++++++++++-------\n> gitweb/gitweb.perl | 14 +++++++-------\n> t/t9500-gitweb-standalone-no-errors.sh | 8 ++++++++\n> 3 files changed, 29 insertions(+), 14 deletions(-)\n> \n> diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\n> index a79e350..e632089 100644\n> --- a/Documentation/gitweb.conf.txt\n> +++ b/Documentation/gitweb.conf.txt\n> @@ -246,13 +246,20 @@ $highlight_bin::\n> \tNote that 'highlight' feature must be set for gitweb to actually\n> \tuse syntax highlighting.\n> +\n> -*NOTE*: if you want to add support for new file type (supported by\n> -\"highlight\" but not used by gitweb), you need to modify `%highlight_ext`\n> -or `%highlight_basename`, depending on whether you detect type of file\n> -based on extension (for example \"sh\") or on its basename (for example\n> -\"Makefile\"). The keys of these hashes are extension and basename,\n> -respectively, and value for given key is name of syntax to be passed via\n> -`--syntax ` to highlighter.\n> +*NOTE*: for a file to be highlighted, its syntax type must be detected\n> +and that syntax must be supported by \"highlight\". The default syntax\n> +detection is minimal, and there are many supported syntax types with no\n> +detection by default. There are three options for adding syntax\n> +detection. The first and second priority are `%highlight_basename` and\n> +`%highlight_ext`, which detect based on basename (the full filename, for\n> +example \"Makefile\") and extension (for example \"sh\"). The keys of these\n> +hashes are the basename and extension, respectively, and the value for a\n> +given key is the name of the syntax to be passed via `--syntax `\n> +to \"highlight\". The last priority is the \"highlight\" configuration of\n> +`Shebang` regular expressions to detect the language based on the first\n> +line in the file, (for example, matching the line \"#!/bin/bash\"). See\n> +the highlight documentation and the default config at\n> +/etc/highlight/filetypes.conf for more details.\n> +\n\nI think the rewrite is a bit more readable.\n\n> For example if repositories you are hosting use \"phtml\" extension for\n> PHP files, and you want to have correct syntax-highlighting for those\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 33d701d..44094f4 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -3913,7 +3913,7 @@ sub blob_contenttype {\n> # guess file syntax for syntax highlighting; return undef if no highlighting\n> # the name of syntax can (in the future) depend on syntax highlighter used\n> sub guess_file_syntax {\n> -\tmy ($highlight, $mimetype, $file_name) = @_;\n> +\tmy ($highlight, $file_name) = @_;\n\nRight.\n\n> \treturn undef unless ($highlight && defined $file_name);\n> \tmy $basename = basename($file_name, '.in');\n> \treturn $highlight_basename{$basename}\n> @@ -3931,15 +3931,16 @@ sub guess_file_syntax {\n> # or return original FD if no highlighting\n> sub run_highlighter {\n> \tmy ($fd, $highlight, $syntax) = @_;\n> -\treturn $fd unless ($highlight && defined $syntax);\n> +\treturn $fd unless ($highlight);\n\nRun highlighter if it is defined, even if gitweb doesn't know syntax, right.\n\n> \n> \tclose $fd;\n> +\tmy $syntax_arg = (defined $syntax) ? \"--syntax $syntax\" : \"--force\";\n> \topen $fd, quote_command(git_cmd(), \"cat-file\", \"blob\", $hash).\" | \".\n> \t quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',\n> \t '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',\n> \t '--', \"-fe=$fallback_encoding\").\" | \".\n> \t quote_command($highlight_bin).\n> -\t \" --replace-tabs=8 --fragment --syntax $syntax |\"\n> +\t \" --replace-tabs=8 --fragment $syntax_arg |\"\n\nUse '--force' if syntax is unknown, right.\n\n> \t\tor die_error(500, \"Couldn't open file or run syntax highlighter\");\n> \treturn $fd;\n> }\n> @@ -7062,9 +7063,8 @@ sub git_blob {\n> \t$have_blame &&= ($mimetype =~ m!^text/!);\n> \n> \tmy $highlight = gitweb_check_feature('highlight');\n> -\tmy $syntax = guess_file_syntax($highlight, $mimetype, $file_name);\n> -\t$fd = run_highlighter($fd, $highlight, $syntax)\n> -\t\tif $syntax;\n> +\tmy $syntax = guess_file_syntax($highlight, $file_name);\n> +\t$fd = run_highlighter($fd, $highlight, $syntax);\n\nRemove unused parameter from callsite, *and* run highlighter even if we\ndon't know syntax.\n\n> \n> \tgit_header_html(undef, $expires);\n> \tmy $formats_nav = '';\n> @@ -7117,7 +7117,7 @@ sub git_blob {\n> \t\t\t$line = untabify($line);\n> \t\t\tprintf qq!
%4i %s
\\n!,\n> \t\t\t $nr, esc_attr(href(-replay => 1)), $nr, $nr,\n> -\t\t\t $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);\n> +\t\t\t $highlight ? sanitize($line) : esc_html($line, -nbsp=>1);\n\nThis is a bit of code duplication / sync from run_highlighter(), but\nit is not your fault; it was there (and I don't know how to improve it).\n\n> \t\t}\n> \t}\n> \tclose $fd\n> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\n> index e94b2f1..576db6d 100755\n> --- a/t/t9500-gitweb-standalone-no-errors.sh\n> +++ b/t/t9500-gitweb-standalone-no-errors.sh\n\nNice.\n\n> @@ -709,6 +709,14 @@ test_expect_success HIGHLIGHT \\\n> \t git commit -m \"Add test.sh\" &&\n> \t gitweb_run \"p=.git;a=blob;f=test.sh\"'\n> \n> +test_expect_success HIGHLIGHT \\\n> +\t'syntax highlighting (highlighter language autodetection)' \\\n> +\t'git config gitweb.highlight yes &&\n\nModern way would be\n\n +\t'test_config gitweb.highlight yes &&\n\nbut other tests in this file do not use it.\n\n> +\t echo \"#!/usr/bin/ruby\" > test &&\n\nPreferred style would be\n\n +\t echo \"#!/usr/bin/ruby\" >test &&\n\nbut other tests in this file do not use it.\n\nSidenote: why Ruby, and not sh / bash, Perl or Python?\n\n> +\t git add test &&\n> +\t git commit -m \"Add test\" &&\n> +\t gitweb_run \"p=.git;a=blob;f=test\"'\n> +\n> # ----------------------------------------------------------------------\n> # forks of projects\n> \n> \n\nThank you for your work.\n-- \nJakub Narębski\n\n"},{"id":"302434","messageId":"1474621710.919704.734654545.799DCFC3@webmail.messagingengine.com","threadId":"44027","inReplyTo":"2a5aead0-d521-85eb-f304-7091683d8b40@gmail.com","subject":"Re: [PATCH v2] gitweb: use highlight's shebang detection","fromName":"Ian Kelling","fromEmail":"ian@iankelling.org","sentAt":"2016-09-23T09:08:30Z","receivedAt":"2016-09-23T09:08:40Z","isPatch":true,"sender":{"key":"ian@iankelling.org","avatar":"https://avatars.githubusercontent.com/u/54232?v=4"},"body":"On Thu, Sep 22, 2016, at 03:50 PM, Jakub Narębski wrote:\n> W dniu 22.09.2016 o 00:18, Ian Kelling napisał:\n>\n> > The highlight binary can detect language by shebang when we can't tell\n> > the syntax type by the name of the file. In that case, pass the blob\n> > to \"highlight --force\" and the resulting html will have markup for\n> > highlighting if the language was detected.\n>\n> This description feels a bit convoluted. Perhaps something like this:\n>\n> The \"highlight\" binary can, in some cases, determine the language type\n> by the means of file contents, for example the shebang in the first\n> line\n> for some scripting languages. Make use of this autodetection for files\n> which syntax is not known by gitweb. In that case, pass the blob\n> contents to \"highlight --force\"; the parameter is needed to make it\n> always generate HTML output (which includes HTML-escaping).\n\nNice. Using it in v3.\n\n>\n> Also, we might want to have the information about performance of this\n> solution either in the commit message, or in commit comments.\n\nI tested it more rigorously and added to v3 commit message.\n\n>\n> >\n> > Document the feature and improve syntax highlight documentation, add\n> > test to ensure gitweb doesn't crash when language detection is used,\n>\n> All right.\n>\n> > and remove an unused parameter from gitweb_check_feature().\n>\n> First, that is guess_file_syntax(), not gitweb_check_feature().\n> Second, this change could be made into independent patch, for example\n> preparatory one.\n\n\nOops. I split it out in v3.\n\n>\n> >\n> > Signed-off-by: Ian Kelling \n> > ---\n> > Documentation/gitweb.conf.txt | 21 ++++++++++++++-------\n> > gitweb/gitweb.perl | 14 +++++++-------\n> > t/t9500-gitweb-standalone-no-errors.sh | 8 ++++++++\n> > 3 files changed, 29 insertions(+), 14 deletions(-)\n> >\n> > diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\n> > index a79e350..e632089 100644\n> > --- a/Documentation/gitweb.conf.txt\n> > +++ b/Documentation/gitweb.conf.txt\n> > @@ -246,13 +246,20 @@ $highlight_bin::\n> > \tNote that 'highlight' feature must be set for gitweb to actually\n> > \tuse syntax highlighting.\n> > +\n> > -*NOTE*: if you want to add support for new file type (supported by\n> > -\"highlight\" but not used by gitweb), you need to modify `%highlight_ext`\n> > -or `%highlight_basename`, depending on whether you detect type of file\n> > -based on extension (for example \"sh\") or on its basename (for example\n> > -\"Makefile\"). The keys of these hashes are extension and basename,\n> > -respectively, and value for given key is name of syntax to be passed via\n> > -`--syntax ` to highlighter.\n> > +*NOTE*: for a file to be highlighted, its syntax type must be detected\n> > +and that syntax must be supported by \"highlight\". The default syntax\n> > +detection is minimal, and there are many supported syntax types with no\n> > +detection by default. There are three options for adding syntax\n> > +detection. The first and second priority are `%highlight_basename` and\n> > +`%highlight_ext`, which detect based on basename (the full filename, for\n> > +example \"Makefile\") and extension (for example \"sh\"). The keys of these\n> > +hashes are the basename and extension, respectively, and the value for a\n> > +given key is the name of the syntax to be passed via `--syntax `\n> > +to \"highlight\". The last priority is the \"highlight\" configuration of\n> > +`Shebang` regular expressions to detect the language based on the first\n> > +line in the file, (for example, matching the line \"#!/bin/bash\"). See\n> > +the highlight documentation and the default config at\n> > +/etc/highlight/filetypes.conf for more details.\n> > +\n>\n> I think the rewrite is a bit more readable.\n>\n> > For example if repositories you are hosting use \"phtml\" extension for\n> > PHP files, and you want to have correct syntax-highlighting for those\n> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> > index 33d701d..44094f4 100755\n> > --- a/gitweb/gitweb.perl\n> > +++ b/gitweb/gitweb.perl\n> > @@ -3913,7 +3913,7 @@ sub blob_contenttype {\n> > # guess file syntax for syntax highlighting; return undef if no highlighting\n> > # the name of syntax can (in the future) depend on syntax highlighter used\n> > sub guess_file_syntax {\n> > -\tmy ($highlight, $mimetype, $file_name) = @_;\n> > +\tmy ($highlight, $file_name) = @_;\n>\n> Right.\n>\n> > \treturn undef unless ($highlight && defined $file_name);\n> > \tmy $basename = basename($file_name, '.in');\n> > \treturn $highlight_basename{$basename}\n> > @@ -3931,15 +3931,16 @@ sub guess_file_syntax {\n> > # or return original FD if no highlighting\n> > sub run_highlighter {\n> > \tmy ($fd, $highlight, $syntax) = @_;\n> > -\treturn $fd unless ($highlight && defined $syntax);\n> > +\treturn $fd unless ($highlight);\n>\n> Run highlighter if it is defined, even if gitweb doesn't know syntax,\n> right.\n>\n> >\n> > \tclose $fd;\n> > +\tmy $syntax_arg = (defined $syntax) ? \"--syntax $syntax\" : \"--force\";\n> > \topen $fd, quote_command(git_cmd(), \"cat-file\", \"blob\", $hash).\" | \".\n> > \t quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',\n> > \t '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',\n> > \t '--', \"-fe=$fallback_encoding\").\" | \".\n> > \t quote_command($highlight_bin).\n> > -\t \" --replace-tabs=8 --fragment --syntax $syntax |\"\n> > +\t \" --replace-tabs=8 --fragment $syntax_arg |\"\n>\n> Use '--force' if syntax is unknown, right.\n>\n> > \t\tor die_error(500, \"Couldn't open file or run syntax highlighter\");\n> > \treturn $fd;\n> > }\n> > @@ -7062,9 +7063,8 @@ sub git_blob {\n> > \t$have_blame &&= ($mimetype =~ m!^text/!);\n> >\n> > \tmy $highlight = gitweb_check_feature('highlight');\n> > -\tmy $syntax = guess_file_syntax($highlight, $mimetype, $file_name);\n> > -\t$fd = run_highlighter($fd, $highlight, $syntax)\n> > -\t\tif $syntax;\n> > +\tmy $syntax = guess_file_syntax($highlight, $file_name);\n> > +\t$fd = run_highlighter($fd, $highlight, $syntax);\n>\n> Remove unused parameter from callsite, *and* run highlighter even if we\n> don't know syntax.\n>\n> >\n> > \tgit_header_html(undef, $expires);\n> > \tmy $formats_nav = '';\n> > @@ -7117,7 +7117,7 @@ sub git_blob {\n> > \t\t\t$line = untabify($line);\n> > \t\t\tprintf qq!
%4i %s
\\n!,\n> > \t\t\t $nr, esc_attr(href(-replay => 1)), $nr, $nr,\n> > -\t\t\t $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);\n> > +\t\t\t $highlight ? sanitize($line) : esc_html($line, -nbsp=>1);\n>\n> This is a bit of code duplication / sync from run_highlighter(), but\n> it is not your fault; it was there (and I don't know how to improve it).\n>\n> > \t\t}\n> > \t}\n> > \tclose $fd\n> > diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\n> > index e94b2f1..576db6d 100755\n> > --- a/t/t9500-gitweb-standalone-no-errors.sh\n> > +++ b/t/t9500-gitweb-standalone-no-errors.sh\n>\n> Nice.\n>\n> > @@ -709,6 +709,14 @@ test_expect_success HIGHLIGHT \\\n> > \t git commit -m \"Add test.sh\" &&\n> > \t gitweb_run \"p=.git;a=blob;f=test.sh\"'\n> >\n> > +test_expect_success HIGHLIGHT \\\n> > +\t'syntax highlighting (highlighter language autodetection)' \\\n> > +\t'git config gitweb.highlight yes &&\n>\n> Modern way would be\n>\n> + 'test_config gitweb.highlight yes &&\n>\n> but other tests in this file do not use it.\n>\n> > +\t echo \"#!/usr/bin/ruby\" > test &&\n>\n> Preferred style would be\n>\n> + echo \"#!/usr/bin/ruby\" >test &&\n>\n> but other tests in this file do not use it.\n\nAgreed, but leaving it as is for consistency.\n\n>\n> Sidenote: why Ruby, and not sh / bash, Perl or Python?\n\nNot sh / bash, just to exercise more functionality of highlight by using\na different language than the other test. ruby just was the first thing\nto come to mind since I've worked with it recently, but since you made\nme think of it, perl is more likely to exist in the builtin config for\nlonger, and it seems a bit more fitting with gitweb, so its perl in v3.\n\n>\n> > +\t git add test &&\n> > +\t git commit -m \"Add test\" &&\n> > +\t gitweb_run \"p=.git;a=blob;f=test\"'\n> > +\n> > # ----------------------------------------------------------------------\n> > # forks of projects\n> >\n> >\n>\n> Thank you for your work.\n> --\n> Jakub Narębski\n>\n\nThe only changes in v3 are the ones I described here.\n\nThank you for reviewing this.\n--\nIan Kelling\n"},{"id":"302435","messageId":"20160923090846.3086-1-ian@iankelling.org","threadId":"44027","inReplyTo":"2a5aead0-d521-85eb-f304-7091683d8b40@gmail.com","subject":"[PATCH v3 1/2] gitweb: remove unused function parameter","fromName":"Ian Kelling","fromEmail":"ian@iankelling.org","sentAt":"2016-09-23T09:08:45Z","receivedAt":"2016-09-23T09:08:54Z","isPatch":true,"sender":{"key":"ian@iankelling.org","avatar":"https://avatars.githubusercontent.com/u/54232?v=4"},"body":"Signed-off-by: Ian Kelling \n---\n gitweb/gitweb.perl | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 33d701d..6cb4280 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3913,7 +3913,7 @@ sub blob_contenttype {\n # guess file syntax for syntax highlighting; return undef if no highlighting\n # the name of syntax can (in the future) depend on syntax highlighter used\n sub guess_file_syntax {\n-\tmy ($highlight, $mimetype, $file_name) = @_;\n+\tmy ($highlight, $file_name) = @_;\n \treturn undef unless ($highlight && defined $file_name);\n \tmy $basename = basename($file_name, '.in');\n \treturn $highlight_basename{$basename}\n@@ -7062,7 +7062,7 @@ sub git_blob {\n \t$have_blame &&= ($mimetype =~ m!^text/!);\n \n \tmy $highlight = gitweb_check_feature('highlight');\n-\tmy $syntax = guess_file_syntax($highlight, $mimetype, $file_name);\n+\tmy $syntax = guess_file_syntax($highlight, $file_name);\n \t$fd = run_highlighter($fd, $highlight, $syntax)\n \t\tif $syntax;\n \n-- \n2.9.3\n\n"},{"id":"302436","messageId":"20160923090846.3086-2-ian@iankelling.org","threadId":"44027","inReplyTo":"20160923090846.3086-1-ian@iankelling.org","subject":"[PATCH v3 2/2] gitweb: use highlight's shebang detection","fromName":"Ian Kelling","fromEmail":"ian@iankelling.org","sentAt":"2016-09-23T09:08:46Z","receivedAt":"2016-09-23T09:08:58Z","isPatch":true,"sender":{"key":"ian@iankelling.org","avatar":"https://avatars.githubusercontent.com/u/54232?v=4"},"body":"The \"highlight\" binary can, in some cases, determine the language type\nby the means of file contents, for example the shebang in the first line\nfor some scripting languages. Make use of this autodetection for files\nwhich syntax is not known by gitweb. In that case, pass the blob\ncontents to \"highlight --force\"; the parameter is needed to make it\nalways generate HTML output (which includes HTML-escaping).\n\nAlthough we now run highlight on files which do not end up highlighted,\nperformance is virtually unaffected because when we call highlight, we\nalso call sanitize() instead of esc_html(), which is significantly\nslower. After curling blob view of unhighlighted large and small text\nfiles of perl code and license text 100 times each on a local\nApache/2.4.23 (Debian) instance, it's logs indicate +-1% difference in\nrequest time for all file types.\n\nDocument the feature and improve syntax highlight documentation, add\ntest to ensure gitweb doesn't crash when language detection is used.\n\nSigned-off-by: Ian Kelling \n---\n Documentation/gitweb.conf.txt | 21 ++++++++++++++-------\n gitweb/gitweb.perl | 10 +++++-----\n t/t9500-gitweb-standalone-no-errors.sh | 8 ++++++++\n 3 files changed, 27 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\nindex a79e350..e632089 100644\n--- a/Documentation/gitweb.conf.txt\n+++ b/Documentation/gitweb.conf.txt\n@@ -246,13 +246,20 @@ $highlight_bin::\n \tNote that 'highlight' feature must be set for gitweb to actually\n \tuse syntax highlighting.\n +\n-*NOTE*: if you want to add support for new file type (supported by\n-\"highlight\" but not used by gitweb), you need to modify `%highlight_ext`\n-or `%highlight_basename`, depending on whether you detect type of file\n-based on extension (for example \"sh\") or on its basename (for example\n-\"Makefile\"). The keys of these hashes are extension and basename,\n-respectively, and value for given key is name of syntax to be passed via\n-`--syntax ` to highlighter.\n+*NOTE*: for a file to be highlighted, its syntax type must be detected\n+and that syntax must be supported by \"highlight\". The default syntax\n+detection is minimal, and there are many supported syntax types with no\n+detection by default. There are three options for adding syntax\n+detection. The first and second priority are `%highlight_basename` and\n+`%highlight_ext`, which detect based on basename (the full filename, for\n+example \"Makefile\") and extension (for example \"sh\"). The keys of these\n+hashes are the basename and extension, respectively, and the value for a\n+given key is the name of the syntax to be passed via `--syntax `\n+to \"highlight\". The last priority is the \"highlight\" configuration of\n+`Shebang` regular expressions to detect the language based on the first\n+line in the file, (for example, matching the line \"#!/bin/bash\"). See\n+the highlight documentation and the default config at\n+/etc/highlight/filetypes.conf for more details.\n +\n For example if repositories you are hosting use \"phtml\" extension for\n PHP files, and you want to have correct syntax-highlighting for those\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 6cb4280..44094f4 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3931,15 +3931,16 @@ sub guess_file_syntax {\n # or return original FD if no highlighting\n sub run_highlighter {\n \tmy ($fd, $highlight, $syntax) = @_;\n-\treturn $fd unless ($highlight && defined $syntax);\n+\treturn $fd unless ($highlight);\n \n \tclose $fd;\n+\tmy $syntax_arg = (defined $syntax) ? \"--syntax $syntax\" : \"--force\";\n \topen $fd, quote_command(git_cmd(), \"cat-file\", \"blob\", $hash).\" | \".\n \t quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',\n \t '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',\n \t '--', \"-fe=$fallback_encoding\").\" | \".\n \t quote_command($highlight_bin).\n-\t \" --replace-tabs=8 --fragment --syntax $syntax |\"\n+\t \" --replace-tabs=8 --fragment $syntax_arg |\"\n \t\tor die_error(500, \"Couldn't open file or run syntax highlighter\");\n \treturn $fd;\n }\n@@ -7063,8 +7064,7 @@ sub git_blob {\n \n \tmy $highlight = gitweb_check_feature('highlight');\n \tmy $syntax = guess_file_syntax($highlight, $file_name);\n-\t$fd = run_highlighter($fd, $highlight, $syntax)\n-\t\tif $syntax;\n+\t$fd = run_highlighter($fd, $highlight, $syntax);\n \n \tgit_header_html(undef, $expires);\n \tmy $formats_nav = '';\n@@ -7117,7 +7117,7 @@ sub git_blob {\n \t\t\t$line = untabify($line);\n \t\t\tprintf qq!
%4i %s
\\n!,\n \t\t\t $nr, esc_attr(href(-replay => 1)), $nr, $nr,\n-\t\t\t $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);\n+\t\t\t $highlight ? sanitize($line) : esc_html($line, -nbsp=>1);\n \t\t}\n \t}\n \tclose $fd\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex e94b2f1..6d06ed9 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -709,6 +709,14 @@ test_expect_success HIGHLIGHT \\\n \t git commit -m \"Add test.sh\" &&\n \t gitweb_run \"p=.git;a=blob;f=test.sh\"'\n \n+test_expect_success HIGHLIGHT \\\n+\t'syntax highlighting (highlighter language autodetection)' \\\n+\t'git config gitweb.highlight yes &&\n+\t echo \"#!/usr/bin/perl\" > test &&\n+\t git add test &&\n+\t git commit -m \"Add test\" &&\n+\t gitweb_run \"p=.git;a=blob;f=test\"'\n+\n # ----------------------------------------------------------------------\n # forks of projects\n \n-- \n2.9.3\n\n"},{"id":"302460","messageId":"4a616b43-cd22-e2fe-6085-a65c48687560@gmail.com","threadId":"44027","inReplyTo":"20160923090846.3086-1-ian@iankelling.org","subject":"Re: [PATCH v3 1/2] gitweb: remove unused function parameter","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-09-23T19:44:56Z","receivedAt":"2016-09-23T19:45:24Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 23.09.2016 o 11:08, Ian Kelling napisał:\n>\n> Subject: [PATCH v3 1/2] gitweb: remove unused function parameter\n\nI think it would be better to be more descriptive, and say:\n\n Subject: [PATCH v3 1/2] gitweb: remove unused parameter from guess_file_syntax()\n\nBut that might be too long...\n\n>\n> Signed-off-by: Ian Kelling \n\nWith, or without this change, it's nice.\n\nAcked-by: Jakub Narębski \n\n> ---\n> gitweb/gitweb.perl | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 33d701d..6cb4280 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -3913,7 +3913,7 @@ sub blob_contenttype {\n> # guess file syntax for syntax highlighting; return undef if no highlighting\n> # the name of syntax can (in the future) depend on syntax highlighter used\n> sub guess_file_syntax {\n> -\tmy ($highlight, $mimetype, $file_name) = @_;\n> +\tmy ($highlight, $file_name) = @_;\n> \treturn undef unless ($highlight && defined $file_name);\n> \tmy $basename = basename($file_name, '.in');\n> \treturn $highlight_basename{$basename}\n> @@ -7062,7 +7062,7 @@ sub git_blob {\n> \t$have_blame &&= ($mimetype =~ m!^text/!);\n> \n> \tmy $highlight = gitweb_check_feature('highlight');\n> -\tmy $syntax = guess_file_syntax($highlight, $mimetype, $file_name);\n> +\tmy $syntax = guess_file_syntax($highlight, $file_name);\n> \t$fd = run_highlighter($fd, $highlight, $syntax)\n> \t\tif $syntax;\n> \n> \n\n"},{"id":"302461","messageId":"xmqqwpi2z8wq.fsf@gitster.mtv.corp.google.com","threadId":"44027","inReplyTo":"4a616b43-cd22-e2fe-6085-a65c48687560@gmail.com","subject":"Re: [PATCH v3 1/2] gitweb: remove unused function parameter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-23T19:57:25Z","receivedAt":"2016-09-23T19:57:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narębski writes:\n\n> I think it would be better to be more descriptive, and say:\n>\n> Subject: [PATCH v3 1/2] gitweb: remove unused parameter from guess_file_syntax()\n> Acked-by: Jakub Narębski \n\nThanks.\n"},{"id":"302466","messageId":"2a4c3efb-2145-b699-c980-3079f165a6e1@gmail.com","threadId":"44027","inReplyTo":"20160923090846.3086-2-ian@iankelling.org","subject":"Re: [PATCH v3 2/2] gitweb: use highlight's shebang detection","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-09-23T22:15:46Z","receivedAt":"2016-09-23T22:16:14Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 23.09.2016 o 11:08, Ian Kelling napisał:\n\n> The \"highlight\" binary can, in some cases, determine the language type\n> by the means of file contents, for example the shebang in the first line\n> for some scripting languages. Make use of this autodetection for files\n> which syntax is not known by gitweb. In that case, pass the blob\n> contents to \"highlight --force\"; the parameter is needed to make it\n> always generate HTML output (which includes HTML-escaping).\n\nRight.\n\n> \n> Although we now run highlight on files which do not end up highlighted,\n> performance is virtually unaffected because when we call highlight, we\n> also call sanitize() instead of esc_html(), which is significantly\n> slower. \n\nThis paragraph is a bit unclear, for example it is not obvious what\n\"..., which is significantly slower\" refers to: sanitize() or esc_html().\n\nI think it would be better to write:\n\n Although we now run highlight on files which do not end up highlighted,\n performance is virtually unaffected because when we call highlight, it\n is used for escaping HTML. In the case that highlight is used, gitweb\n calls sanitize() instead of esc_html(), and the latter is significantly\n slower (it does more, being roughly a superset of sanitize()).\n\n> After curling blob view of unhighlighted large and small text\n> files of perl code and license text 100 times each on a local\n> Apache/2.4.23 (Debian) instance, it's logs indicate +-1% difference in\n> request time for all file types.\n\nAlso, \"curling\" is not the word I would like to see. I would say:\n\n Simple benchmark comparing performance of 'blob' view of files without\n syntax highlighting in gitweb before and after this change indicates\n ±1% difference in request time for all file types. Benchmark was\n performed on local instance on Debian, using Apache/2.4.23 web server\n and CGI/PSGI/FCGI/mod_perl.\n\n ^^^^^^^^^^^^^^^^^^^^^^--- select one\n\nOr something like that; I'm not sure how detailed this should be.\nBut it is nice to have such benchmark in the commit message.\n\nAnyway I think that adding yet another configuration toggle for selecting\nwhether to use \"highlight\" syntax autodetection or not would be just an\nunnecessary complication.\n\nNote that the performance loss might be quite higher on MS Windows, with\nits higher cost of fork. But then they probably do not configure\nserver-side highligher anyway.\n\n> \n> Document the feature and improve syntax highlight documentation, add\n> test to ensure gitweb doesn't crash when language detection is used.\n\nGood.\n\n> \n> Signed-off-by: Ian Kelling \n> ---\n> Documentation/gitweb.conf.txt | 21 ++++++++++++++-------\n> gitweb/gitweb.perl | 10 +++++-----\n> t/t9500-gitweb-standalone-no-errors.sh | 8 ++++++++\n> 3 files changed, 27 insertions(+), 12 deletions(-)\n> \n> diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\n> index a79e350..e632089 100644\n> --- a/Documentation/gitweb.conf.txt\n> +++ b/Documentation/gitweb.conf.txt\n> @@ -246,13 +246,20 @@ $highlight_bin::\n\nWe should probably say what does it mean to be \"highlight\"[1] compatible,\nbut it is outside of scope for this patch, and I think also out of scope\nof this series.\n\n> \tNote that 'highlight' feature must be set for gitweb to actually\n> \tuse syntax highlighting.\n> +\n> -*NOTE*: if you want to add support for new file type (supported by\n> -\"highlight\" but not used by gitweb), you need to modify `%highlight_ext`\n> -or `%highlight_basename`, depending on whether you detect type of file\n> -based on extension (for example \"sh\") or on its basename (for example\n> -\"Makefile\"). The keys of these hashes are extension and basename,\n> -respectively, and value for given key is name of syntax to be passed via\n> -`--syntax ` to highlighter.\n> +*NOTE*: for a file to be highlighted, its syntax type must be detected\n> +and that syntax must be supported by \"highlight\". The default syntax\n> +detection is minimal, and there are many supported syntax types with no\n> +detection by default. There are three options for adding syntax\n> +detection. The first and second priority are `%highlight_basename` and\n> +`%highlight_ext`, which detect based on basename (the full filename, for\n> +example \"Makefile\") and extension (for example \"sh\"). The keys of these\n> +hashes are the basename and extension, respectively, and the value for a\n> +given key is the name of the syntax to be passed via `--syntax `\n> +to \"highlight\". The last priority is the \"highlight\" configuration of\n> +`Shebang` regular expressions to detect the language based on the first\n> +line in the file, (for example, matching the line \"#!/bin/bash\"). See\n> +the highlight documentation and the default config at\n> +/etc/highlight/filetypes.conf for more details.\n\nAll right. I guess /etc/highlight/filetypes.conf is the standard location?\n\n> +\n> For example if repositories you are hosting use \"phtml\" extension for\n> PHP files, and you want to have correct syntax-highlighting for those\n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 6cb4280..44094f4 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -3931,15 +3931,16 @@ sub guess_file_syntax {\n> # or return original FD if no highlighting\n> sub run_highlighter {\n> \tmy ($fd, $highlight, $syntax) = @_;\n> -\treturn $fd unless ($highlight && defined $syntax);\n> +\treturn $fd unless ($highlight);\n> \n> \tclose $fd;\n> +\tmy $syntax_arg = (defined $syntax) ? \"--syntax $syntax\" : \"--force\";\n> \topen $fd, quote_command(git_cmd(), \"cat-file\", \"blob\", $hash).\" | \".\n> \t quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',\n> \t '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',\n> \t '--', \"-fe=$fallback_encoding\").\" | \".\n> \t quote_command($highlight_bin).\n> -\t \" --replace-tabs=8 --fragment --syntax $syntax |\"\n> +\t \" --replace-tabs=8 --fragment $syntax_arg |\"\n> \t\tor die_error(500, \"Couldn't open file or run syntax highlighter\");\n> \treturn $fd;\n> }\n\nAll right, nice and understandable.\n\n> @@ -7063,8 +7064,7 @@ sub git_blob {\n> \n> \tmy $highlight = gitweb_check_feature('highlight');\n> \tmy $syntax = guess_file_syntax($highlight, $file_name);\n> -\t$fd = run_highlighter($fd, $highlight, $syntax)\n> -\t\tif $syntax;\n> +\t$fd = run_highlighter($fd, $highlight, $syntax);\n> \n> \tgit_header_html(undef, $expires);\n> \tmy $formats_nav = '';\n\nGood, run unconditionally.\n\n> @@ -7117,7 +7117,7 @@ sub git_blob {\n> \t\t\t$line = untabify($line);\n> \t\t\tprintf qq!
%4i %s
\\n!,\n> \t\t\t $nr, esc_attr(href(-replay => 1)), $nr, $nr,\n> -\t\t\t $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);\n> +\t\t\t $highlight ? sanitize($line) : esc_html($line, -nbsp=>1);\n> \t\t}\n\nGood, use highlighter if possible, not only if syntax is known\nand highlighter is turned on.\n\nNice and easy to understand after earlier change.\n\n> \t}\n> \tclose $fd\n> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\n> index e94b2f1..6d06ed9 100755\n> --- a/t/t9500-gitweb-standalone-no-errors.sh\n> +++ b/t/t9500-gitweb-standalone-no-errors.sh\n> @@ -709,6 +709,14 @@ test_expect_success HIGHLIGHT \\\n> \t git commit -m \"Add test.sh\" &&\n> \t gitweb_run \"p=.git;a=blob;f=test.sh\"'\n> \n> +test_expect_success HIGHLIGHT \\\n> +\t'syntax highlighting (highlighter language autodetection)' \\\n> +\t'git config gitweb.highlight yes &&\n> +\t echo \"#!/usr/bin/perl\" > test &&\n> +\t git add test &&\n> +\t git commit -m \"Add test\" &&\n> +\t gitweb_run \"p=.git;a=blob;f=test\"'\n\nNice.\n\n> +\n> # ----------------------------------------------------------------------\n> # forks of projects\n> \n> \n\n"},{"id":"302480","messageId":"946807ff-1570-2d81-1026-06529164f8ef@gmail.com","threadId":"44027","inReplyTo":"2a4c3efb-2145-b699-c980-3079f165a6e1@gmail.com","subject":"Re: [PATCH v3 2/2] gitweb: use highlight's shebang detection","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-09-24T16:21:57Z","receivedAt":"2016-09-24T16:22:24Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 24.09.2016 o 00:15, Jakub Narębski pisze:\n> W dniu 23.09.2016 o 11:08, Ian Kelling napisał: \n\n>> After curling blob view of unhighlighted large and small text\n>> files of perl code and license text 100 times each on a local\n>> Apache/2.4.23 (Debian) instance, it's logs indicate +-1% difference in\n>> request time for all file types.\n> \n> Also, \"curling\" is not the word I would like to see. I would say:\n> \n> Simple benchmark comparing performance of 'blob' view of files without\n> syntax highlighting in gitweb before and after this change indicates\n> ±1% difference in request time for all file types. Benchmark was\n> performed on local instance on Debian, using Apache/2.4.23 web server\n> and CGI/PSGI/FCGI/mod_perl.\n> \n> ^^^^^^^^^^^^^^^^^^^^^^--- select one\n> \n> Or something like that; I'm not sure how detailed this should be.\n> But it is nice to have such benchmark in the commit message.\n\nSidenote: this way of benchmarking of gitweb falls between two ways of\ndoing a benchmark.\n\nThe first method is to simply run gitweb as a standalone script, passing\nits parameters in CGI environment variables; just like the test suite\ndoes it. You would 'time' / 'times' it a few times, drop outliers, and\ntake average or a median. With this method you don't even need to set\nup a web server.\n\nThe second is to use a specialized program to benchmark the server-side\nof a web page, for example 'ab' (ApacheBench), httperf, curl-loader\nor JMeter. The first one is usually distributed together with Apache\nweb server, so you probably have it installed already. Those tools\nprovide timing statistics.\n\n[...]\n> Note that the performance loss might be quite higher on MS Windows, with\n> its higher cost of fork. But then they probably do not configure\n> server-side highligher anyway.\n\n"},{"id":"302481","messageId":"xmqqh995yyl4.fsf@gitster.mtv.corp.google.com","threadId":"44027","inReplyTo":"946807ff-1570-2d81-1026-06529164f8ef@gmail.com","subject":"Re: [PATCH v3 2/2] gitweb: use highlight's shebang detection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-24T17:52:39Z","receivedAt":"2016-09-24T17:52:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narębski writes:\n\n>> Also, \"curling\" is not the word I would like to see. I would say:\n>> \n>> Simple benchmark comparing performance of 'blob' view of files without\n>> syntax highlighting in gitweb before and after this change indicates\n>> ±1% difference in request time for all file types. Benchmark was\n>> performed on local instance on Debian, using Apache/2.4.23 web server\n>> and CGI/PSGI/FCGI/mod_perl.\n>> \n>> ^^^^^^^^^^^^^^^^^^^^^^--- select one\n\nor state that all of them produced similar results ;-)\n\n>> Or something like that; I'm not sure how detailed this should be.\n>> But it is nice to have such benchmark in the commit message.\n>\n> Sidenote: this way of benchmarking of gitweb falls between two ways of\n> doing a benchmark.\n\nAll good comments. Thanks.\n"},{"id":"302492","messageId":"20160924223258.9449-1-ian@iankelling.org","threadId":"44027","inReplyTo":"20160923090846.3086-2-ian@iankelling.org","subject":"[PATCH v4 1/2] gitweb: remove unused guess_file_syntax() parameter","fromName":"Ian Kelling","fromEmail":"ian@iankelling.org","sentAt":"2016-09-24T22:32:57Z","receivedAt":"2016-09-24T22:33:04Z","isPatch":true,"sender":{"key":"ian@iankelling.org","avatar":"https://avatars.githubusercontent.com/u/54232?v=4"},"body":"Signed-off-by: Ian Kelling \n---\n\nNotes:\n The only change from v3 is a more descriptive commit message\n\n gitweb/gitweb.perl | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 33d701d..6cb4280 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3913,7 +3913,7 @@ sub blob_contenttype {\n # guess file syntax for syntax highlighting; return undef if no highlighting\n # the name of syntax can (in the future) depend on syntax highlighter used\n sub guess_file_syntax {\n-\tmy ($highlight, $mimetype, $file_name) = @_;\n+\tmy ($highlight, $file_name) = @_;\n \treturn undef unless ($highlight && defined $file_name);\n \tmy $basename = basename($file_name, '.in');\n \treturn $highlight_basename{$basename}\n@@ -7062,7 +7062,7 @@ sub git_blob {\n \t$have_blame &&= ($mimetype =~ m!^text/!);\n \n \tmy $highlight = gitweb_check_feature('highlight');\n-\tmy $syntax = guess_file_syntax($highlight, $mimetype, $file_name);\n+\tmy $syntax = guess_file_syntax($highlight, $file_name);\n \t$fd = run_highlighter($fd, $highlight, $syntax)\n \t\tif $syntax;\n \n-- \n2.9.3\n\n"},{"id":"302493","messageId":"20160924223258.9449-2-ian@iankelling.org","threadId":"44027","inReplyTo":"20160924223258.9449-1-ian@iankelling.org","subject":"[PATCH v4 2/2] gitweb: use highlight's shebang detection","fromName":"Ian Kelling","fromEmail":"ian@iankelling.org","sentAt":"2016-09-24T22:32:58Z","receivedAt":"2016-09-24T22:33:08Z","isPatch":true,"sender":{"key":"ian@iankelling.org","avatar":"https://avatars.githubusercontent.com/u/54232?v=4"},"body":"The \"highlight\" binary can, in some cases, determine the language type\nby the means of file contents, for example the shebang in the first line\nfor some scripting languages. Make use of this autodetection for files\nwhich syntax is not known by gitweb. In that case, pass the blob\ncontents to \"highlight --force\"; the parameter is needed to make it\nalways generate HTML output (which includes HTML-escaping).\n\nAlthough we now run highlight on files which do not end up highlighted,\nperformance is virtually unaffected because when we call highlight, it\nis used for escaping HTML. In the case that highlight is used, gitweb\ncalls sanitize() instead of esc_html(), and the latter is significantly\nslower (it does more, being roughly a superset of sanitize()). Simple\nbenchmark comparing performance of 'blob' view of files without syntax\nhighlighting in gitweb before and after this change indicates ±1%\ndifference in request time for all file types. Benchmark was performed\non local instance on Debian, using Apache/2.4.23 web server and CGI.\n\nDocument the feature and improve syntax highlight documentation, add\ntest to ensure gitweb doesn't crash when language detection is used.\n\nSigned-off-by: Ian Kelling \n---\n\nNotes:\n The only change from v3 is the commit message as suggested by Jakub\n Narębski\n\n Documentation/gitweb.conf.txt | 21 ++++++++++++++-------\n gitweb/gitweb.perl | 10 +++++-----\n t/t9500-gitweb-standalone-no-errors.sh | 8 ++++++++\n 3 files changed, 27 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\nindex a79e350..e632089 100644\n--- a/Documentation/gitweb.conf.txt\n+++ b/Documentation/gitweb.conf.txt\n@@ -246,13 +246,20 @@ $highlight_bin::\n \tNote that 'highlight' feature must be set for gitweb to actually\n \tuse syntax highlighting.\n +\n-*NOTE*: if you want to add support for new file type (supported by\n-\"highlight\" but not used by gitweb), you need to modify `%highlight_ext`\n-or `%highlight_basename`, depending on whether you detect type of file\n-based on extension (for example \"sh\") or on its basename (for example\n-\"Makefile\"). The keys of these hashes are extension and basename,\n-respectively, and value for given key is name of syntax to be passed via\n-`--syntax ` to highlighter.\n+*NOTE*: for a file to be highlighted, its syntax type must be detected\n+and that syntax must be supported by \"highlight\". The default syntax\n+detection is minimal, and there are many supported syntax types with no\n+detection by default. There are three options for adding syntax\n+detection. The first and second priority are `%highlight_basename` and\n+`%highlight_ext`, which detect based on basename (the full filename, for\n+example \"Makefile\") and extension (for example \"sh\"). The keys of these\n+hashes are the basename and extension, respectively, and the value for a\n+given key is the name of the syntax to be passed via `--syntax `\n+to \"highlight\". The last priority is the \"highlight\" configuration of\n+`Shebang` regular expressions to detect the language based on the first\n+line in the file, (for example, matching the line \"#!/bin/bash\"). See\n+the highlight documentation and the default config at\n+/etc/highlight/filetypes.conf for more details.\n +\n For example if repositories you are hosting use \"phtml\" extension for\n PHP files, and you want to have correct syntax-highlighting for those\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 6cb4280..44094f4 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3931,15 +3931,16 @@ sub guess_file_syntax {\n # or return original FD if no highlighting\n sub run_highlighter {\n \tmy ($fd, $highlight, $syntax) = @_;\n-\treturn $fd unless ($highlight && defined $syntax);\n+\treturn $fd unless ($highlight);\n \n \tclose $fd;\n+\tmy $syntax_arg = (defined $syntax) ? \"--syntax $syntax\" : \"--force\";\n \topen $fd, quote_command(git_cmd(), \"cat-file\", \"blob\", $hash).\" | \".\n \t quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',\n \t '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',\n \t '--', \"-fe=$fallback_encoding\").\" | \".\n \t quote_command($highlight_bin).\n-\t \" --replace-tabs=8 --fragment --syntax $syntax |\"\n+\t \" --replace-tabs=8 --fragment $syntax_arg |\"\n \t\tor die_error(500, \"Couldn't open file or run syntax highlighter\");\n \treturn $fd;\n }\n@@ -7063,8 +7064,7 @@ sub git_blob {\n \n \tmy $highlight = gitweb_check_feature('highlight');\n \tmy $syntax = guess_file_syntax($highlight, $file_name);\n-\t$fd = run_highlighter($fd, $highlight, $syntax)\n-\t\tif $syntax;\n+\t$fd = run_highlighter($fd, $highlight, $syntax);\n \n \tgit_header_html(undef, $expires);\n \tmy $formats_nav = '';\n@@ -7117,7 +7117,7 @@ sub git_blob {\n \t\t\t$line = untabify($line);\n \t\t\tprintf qq!
%4i %s
\\n!,\n \t\t\t $nr, esc_attr(href(-replay => 1)), $nr, $nr,\n-\t\t\t $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);\n+\t\t\t $highlight ? sanitize($line) : esc_html($line, -nbsp=>1);\n \t\t}\n \t}\n \tclose $fd\ndiff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\nindex e94b2f1..6d06ed9 100755\n--- a/t/t9500-gitweb-standalone-no-errors.sh\n+++ b/t/t9500-gitweb-standalone-no-errors.sh\n@@ -709,6 +709,14 @@ test_expect_success HIGHLIGHT \\\n \t git commit -m \"Add test.sh\" &&\n \t gitweb_run \"p=.git;a=blob;f=test.sh\"'\n \n+test_expect_success HIGHLIGHT \\\n+\t'syntax highlighting (highlighter language autodetection)' \\\n+\t'git config gitweb.highlight yes &&\n+\t echo \"#!/usr/bin/perl\" > test &&\n+\t git add test &&\n+\t git commit -m \"Add test\" &&\n+\t gitweb_run \"p=.git;a=blob;f=test\"'\n+\n # ----------------------------------------------------------------------\n # forks of projects\n \n-- \n2.9.3\n\n"},{"id":"302494","messageId":"1474756463.3745393.735971833.7FD5B5C9@webmail.messagingengine.com","threadId":"44027","inReplyTo":"2a4c3efb-2145-b699-c980-3079f165a6e1@gmail.com","subject":"Re: [PATCH v3 2/2] gitweb: use highlight's shebang detection","fromName":"Ian Kelling","fromEmail":"ian@iankelling.org","sentAt":"2016-09-24T22:34:23Z","receivedAt":"2016-09-24T22:34:29Z","isPatch":true,"sender":{"key":"ian@iankelling.org","avatar":"https://avatars.githubusercontent.com/u/54232?v=4"},"body":"On Fri, Sep 23, 2016, at 03:15 PM, Jakub Narębski wrote:\n> W dniu 23.09.2016 o 11:08, Ian Kelling napisał:\n>\n> > The \"highlight\" binary can, in some cases, determine the language type\n> > by the means of file contents, for example the shebang in the first line\n> > for some scripting languages. Make use of this autodetection for files\n> > which syntax is not known by gitweb. In that case, pass the blob\n> > contents to \"highlight --force\"; the parameter is needed to make it\n> > always generate HTML output (which includes HTML-escaping).\n>\n> Right.\n>\n> >\n> > Although we now run highlight on files which do not end up highlighted,\n> > performance is virtually unaffected because when we call highlight, we\n> > also call sanitize() instead of esc_html(), which is significantly\n> > slower.\n>\n> This paragraph is a bit unclear, for example it is not obvious what\n> \"..., which is significantly slower\" refers to: sanitize() or esc_html().\n>\n> I think it would be better to write:\n>\n> Although we now run highlight on files which do not end up highlighted,\n> performance is virtually unaffected because when we call highlight, it\n> is used for escaping HTML. In the case that highlight is used, gitweb\n> calls sanitize() instead of esc_html(), and the latter is significantly\n> slower (it does more, being roughly a superset of sanitize()).\n\nAgree. Done in v4.\n\n>\n> > After curling blob view of unhighlighted large and small text\n> > files of perl code and license text 100 times each on a local\n> > Apache/2.4.23 (Debian) instance, it's logs indicate +-1% difference in\n> > request time for all file types.\n>\n> Also, \"curling\" is not the word I would like to see. I would say:\n>\n> Simple benchmark comparing performance of 'blob' view of files without\n> syntax highlighting in gitweb before and after this change indicates\n> ±1% difference in request time for all file types. Benchmark was\n> performed on local instance on Debian, using Apache/2.4.23 web server\n> and CGI/PSGI/FCGI/mod_perl.\n>\n> ^^^^^^^^^^^^^^^^^^^^^^--- select one\n>\n> Or something like that; I'm not sure how detailed this should be.\n> But it is nice to have such benchmark in the commit message.\n\n\nSounds good. Used it in v4.\n\n>\n> Anyway I think that adding yet another configuration toggle for selecting\n> whether to use \"highlight\" syntax autodetection or not would be just an\n> unnecessary complication.\n>\n> Note that the performance loss might be quite higher on MS Windows, with\n> its higher cost of fork. But then they probably do not configure\n> server-side highligher anyway.\n>\n> >\n> > Document the feature and improve syntax highlight documentation, add\n> > test to ensure gitweb doesn't crash when language detection is used.\n>\n> Good.\n>\n> >\n> > Signed-off-by: Ian Kelling \n> > ---\n> > Documentation/gitweb.conf.txt | 21 ++++++++++++++-------\n> > gitweb/gitweb.perl | 10 +++++-----\n> > t/t9500-gitweb-standalone-no-errors.sh | 8 ++++++++\n> > 3 files changed, 27 insertions(+), 12 deletions(-)\n> >\n> > diff --git a/Documentation/gitweb.conf.txt b/Documentation/gitweb.conf.txt\n> > index a79e350..e632089 100644\n> > --- a/Documentation/gitweb.conf.txt\n> > +++ b/Documentation/gitweb.conf.txt\n> > @@ -246,13 +246,20 @@ $highlight_bin::\n>\n> We should probably say what does it mean to be \"highlight\"[1] compatible,\n> but it is outside of scope for this patch, and I think also out of scope\n> of this series.\n>\n> > \tNote that 'highlight' feature must be set for gitweb to actually\n> > \tuse syntax highlighting.\n> > +\n> > -*NOTE*: if you want to add support for new file type (supported by\n> > -\"highlight\" but not used by gitweb), you need to modify `%highlight_ext`\n> > -or `%highlight_basename`, depending on whether you detect type of file\n> > -based on extension (for example \"sh\") or on its basename (for example\n> > -\"Makefile\"). The keys of these hashes are extension and basename,\n> > -respectively, and value for given key is name of syntax to be passed via\n> > -`--syntax ` to highlighter.\n> > +*NOTE*: for a file to be highlighted, its syntax type must be detected\n> > +and that syntax must be supported by \"highlight\". The default syntax\n> > +detection is minimal, and there are many supported syntax types with no\n> > +detection by default. There are three options for adding syntax\n> > +detection. The first and second priority are `%highlight_basename` and\n> > +`%highlight_ext`, which detect based on basename (the full filename, for\n> > +example \"Makefile\") and extension (for example \"sh\"). The keys of these\n> > +hashes are the basename and extension, respectively, and the value for a\n> > +given key is the name of the syntax to be passed via `--syntax `\n> > +to \"highlight\". The last priority is the \"highlight\" configuration of\n> > +`Shebang` regular expressions to detect the language based on the first\n> > +line in the file, (for example, matching the line \"#!/bin/bash\"). See\n> > +the highlight documentation and the default config at\n> > +/etc/highlight/filetypes.conf for more details.\n>\n> All right. I guess /etc/highlight/filetypes.conf is the standard\n> location?\n\nI think so. I checked packages from homebrew, fedora, suse, debian to\nconfirm.\n\n>\n> > +\n> > For example if repositories you are hosting use \"phtml\" extension for\n> > PHP files, and you want to have correct syntax-highlighting for those\n> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> > index 6cb4280..44094f4 100755\n> > --- a/gitweb/gitweb.perl\n> > +++ b/gitweb/gitweb.perl\n> > @@ -3931,15 +3931,16 @@ sub guess_file_syntax {\n> > # or return original FD if no highlighting\n> > sub run_highlighter {\n> > \tmy ($fd, $highlight, $syntax) = @_;\n> > -\treturn $fd unless ($highlight && defined $syntax);\n> > +\treturn $fd unless ($highlight);\n> >\n> > \tclose $fd;\n> > +\tmy $syntax_arg = (defined $syntax) ? \"--syntax $syntax\" : \"--force\";\n> > \topen $fd, quote_command(git_cmd(), \"cat-file\", \"blob\", $hash).\" | \".\n> > \t quote_command($^X, '-CO', '-MEncode=decode,FB_DEFAULT', '-pse',\n> > \t '$_ = decode($fe, $_, FB_DEFAULT) if !utf8::decode($_);',\n> > \t '--', \"-fe=$fallback_encoding\").\" | \".\n> > \t quote_command($highlight_bin).\n> > -\t \" --replace-tabs=8 --fragment --syntax $syntax |\"\n> > +\t \" --replace-tabs=8 --fragment $syntax_arg |\"\n> > \t\tor die_error(500, \"Couldn't open file or run syntax highlighter\");\n> > \treturn $fd;\n> > }\n>\n> All right, nice and understandable.\n>\n> > @@ -7063,8 +7064,7 @@ sub git_blob {\n> >\n> > \tmy $highlight = gitweb_check_feature('highlight');\n> > \tmy $syntax = guess_file_syntax($highlight, $file_name);\n> > -\t$fd = run_highlighter($fd, $highlight, $syntax)\n> > -\t\tif $syntax;\n> > +\t$fd = run_highlighter($fd, $highlight, $syntax);\n> >\n> > \tgit_header_html(undef, $expires);\n> > \tmy $formats_nav = '';\n>\n> Good, run unconditionally.\n>\n> > @@ -7117,7 +7117,7 @@ sub git_blob {\n> > \t\t\t$line = untabify($line);\n> > \t\t\tprintf qq!
%4i %s
\\n!,\n> > \t\t\t $nr, esc_attr(href(-replay => 1)), $nr, $nr,\n> > -\t\t\t $syntax ? sanitize($line) : esc_html($line, -nbsp=>1);\n> > +\t\t\t $highlight ? sanitize($line) : esc_html($line, -nbsp=>1);\n> > \t\t}\n>\n> Good, use highlighter if possible, not only if syntax is known\n> and highlighter is turned on.\n>\n> Nice and easy to understand after earlier change.\n>\n> > \t}\n> > \tclose $fd\n> > diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh\n> > index e94b2f1..6d06ed9 100755\n> > --- a/t/t9500-gitweb-standalone-no-errors.sh\n> > +++ b/t/t9500-gitweb-standalone-no-errors.sh\n> > @@ -709,6 +709,14 @@ test_expect_success HIGHLIGHT \\\n> > \t git commit -m \"Add test.sh\" &&\n> > \t gitweb_run \"p=.git;a=blob;f=test.sh\"'\n> >\n> > +test_expect_success HIGHLIGHT \\\n> > +\t'syntax highlighting (highlighter language autodetection)' \\\n> > +\t'git config gitweb.highlight yes &&\n> > +\t echo \"#!/usr/bin/perl\" > test &&\n> > +\t git add test &&\n> > +\t git commit -m \"Add test\" &&\n> > +\t gitweb_run \"p=.git;a=blob;f=test\"'\n>\n> Nice.\n>\n> > +\n> > # ----------------------------------------------------------------------\n> > # forks of projects\n> >\n> >\n>\n\nThanks for great suggestions.\n"},{"id":"302495","messageId":"1474756557.3745593.735971977.2A054EBA@webmail.messagingengine.com","threadId":"44027","inReplyTo":"946807ff-1570-2d81-1026-06529164f8ef@gmail.com","subject":"Re: [PATCH v3 2/2] gitweb: use highlight's shebang detection","fromName":"Ian Kelling","fromEmail":"ian@iankelling.org","sentAt":"2016-09-24T22:35:57Z","receivedAt":"2016-09-24T22:36:03Z","isPatch":true,"sender":{"key":"ian@iankelling.org","avatar":"https://avatars.githubusercontent.com/u/54232?v=4"},"body":"On Sat, Sep 24, 2016, at 09:21 AM, Jakub Narębski wrote:\n> W dniu 24.09.2016 o 00:15, Jakub Narębski pisze:\n> \n> Sidenote: this way of benchmarking of gitweb falls between two ways of\n> doing a benchmark.\n> \n> The first method is to simply run gitweb as a standalone script, passing\n> its parameters in CGI environment variables; just like the test suite\n> does it. You would 'time' / 'times' it a few times, drop outliers, and\n> take average or a median. With this method you don't even need to set\n> up a web server.\n> \n> The second is to use a specialized program to benchmark the server-side\n> of a web page, for example 'ab' (ApacheBench), httperf, curl-loader\n> or JMeter. The first one is usually distributed together with Apache\n> web server, so you probably have it installed already. Those tools\n> provide timing statistics.\n\nGood to know. Thanks.\n"},{"id":"302526","messageId":"c2ae28f8-bb3a-b780-020e-c4a9adc803da@gmail.com","threadId":"44027","inReplyTo":"20160924223258.9449-1-ian@iankelling.org","subject":"Re: [PATCH v4 1/2] gitweb: remove unused guess_file_syntax() parameter","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-09-25T17:57:12Z","receivedAt":"2016-09-25T17:57:22Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 25.09.2016 o 00:32, Ian Kelling pisze:\n\n> Subject: gitweb: remove unused guess_file_syntax() parameter\n>\n> Signed-off-by: Ian Kelling \n\nAcked-by: Jakub Narębski \n\n> ---\n> \n> Notes:\n> The only change from v3 is a more descriptive commit message\n> \n> gitweb/gitweb.perl | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index 33d701d..6cb4280 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -3913,7 +3913,7 @@ sub blob_contenttype {\n> # guess file syntax for syntax highlighting; return undef if no highlighting\n> # the name of syntax can (in the future) depend on syntax highlighter used\n> sub guess_file_syntax {\n> -\tmy ($highlight, $mimetype, $file_name) = @_;\n> +\tmy ($highlight, $file_name) = @_;\n> \treturn undef unless ($highlight && defined $file_name);\n> \tmy $basename = basename($file_name, '.in');\n> \treturn $highlight_basename{$basename}\n> @@ -7062,7 +7062,7 @@ sub git_blob {\n> \t$have_blame &&= ($mimetype =~ m!^text/!);\n> \n> \tmy $highlight = gitweb_check_feature('highlight');\n> -\tmy $syntax = guess_file_syntax($highlight, $mimetype, $file_name);\n> +\tmy $syntax = guess_file_syntax($highlight, $file_name);\n> \t$fd = run_highlighter($fd, $highlight, $syntax)\n> \t\tif $syntax;\n> \n> \n\n"},{"id":"302527","messageId":"c35fc760-ae64-5df3-5985-7eecd06d937f@gmail.com","threadId":"44027","inReplyTo":"20160924223258.9449-2-ian@iankelling.org","subject":"Re: [PATCH v4 2/2] gitweb: use highlight's shebang detection","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-09-25T18:04:30Z","receivedAt":"2016-09-25T18:04:41Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 25.09.2016 o 00:32, Ian Kelling pisze:\n> The \"highlight\" binary can, in some cases, determine the language type\n> by the means of file contents, for example the shebang in the first line\n> for some scripting languages. Make use of this autodetection for files\n> which syntax is not known by gitweb. In that case, pass the blob\n> contents to \"highlight --force\"; the parameter is needed to make it\n> always generate HTML output (which includes HTML-escaping).\n> \n> Although we now run highlight on files which do not end up highlighted,\n> performance is virtually unaffected because when we call highlight, it\n> is used for escaping HTML. In the case that highlight is used, gitweb\n> calls sanitize() instead of esc_html(), and the latter is significantly\n> slower (it does more, being roughly a superset of sanitize()). Simple\n> benchmark comparing performance of 'blob' view of files without syntax\n> highlighting in gitweb before and after this change indicates ±1%\n> difference in request time for all file types. Benchmark was performed\n> on local instance on Debian, using Apache/2.4.23 web server and CGI.\n> \n> Document the feature and improve syntax highlight documentation, add\n> test to ensure gitweb doesn't crash when language detection is used.\n> \n> Signed-off-by: Ian Kelling \n\nFor what it is worth it:\n\nAcked-by: Jakub Narębski \n\n(but unfortunately *not* tested by).\n\n> ---\n> \n> Notes:\n> The only change from v3 is the commit message as suggested by Jakub\n> Narębski\n> \n> Documentation/gitweb.conf.txt | 21 ++++++++++++++-------\n> gitweb/gitweb.perl | 10 +++++-----\n> t/t9500-gitweb-standalone-no-errors.sh | 8 ++++++++\n> 3 files changed, 27 insertions(+), 12 deletions(-)\n\n"},{"id":"302800","messageId":"1475048253.906881.739334657.11813C91@webmail.messagingengine.com","threadId":"44027","inReplyTo":"c35fc760-ae64-5df3-5985-7eecd06d937f@gmail.com","subject":"Re: [PATCH v4 2/2] gitweb: use highlight's shebang detection","fromName":"Ian Kelling","fromEmail":"ian@iankelling.org","sentAt":"2016-09-28T07:37:33Z","receivedAt":"2016-09-28T07:37:41Z","isPatch":true,"sender":{"key":"ian@iankelling.org","avatar":"https://avatars.githubusercontent.com/u/54232?v=4"},"body":"On Sun, Sep 25, 2016, at 11:04 AM, Jakub Narębski wrote:\n> \n> For what it is worth it:\n> \n> Acked-by: Jakub Narębski \n> \n> (but unfortunately *not* tested by).\n\nThank you for all your help.\n--\nIan Kelling\n"}]}