{"thread":{"id":"24034","subject":"[RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module","startedAt":"2010-06-07T20:50:41Z","lastAt":"2010-06-12T01:41:38Z","messageCount":17,"participants":["Pavan Kumar Sunkara","Jakub Narebski","Ævar Arnfjörð Bjarmason","Petr Baudis"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"143186","messageId":"1275943844-24991-1-git-send-email-pavan.sss1991@gmail.com","threadId":"24034","inReplyTo":null,"subject":"[RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2010-06-07T20:50:41Z","receivedAt":"2010-06-07T20:50:41Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"Subroutines moved:\n\tgitweb_get_feature\n\tgitweb_check_feature\n\tfilter_snapshot_fmts\n\tconfigure_gitweb_features\n\nSubroutines yet to move: (Contains not yet packaged subs & vars)\n\tfeature_bool\n\tfeature_avatar\n\tfeature_snapshot\n\tfeature_pathces\n\nSigned-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>\n---\n gitweb/gitweb.perl          |   67 --------------------------------------\n gitweb/lib/Gitweb/Config.pm |   74 +++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 72 insertions(+), 69 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 9b2fe09..3931064 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -67,40 +67,6 @@ $strict_export = \"++GITWEB_STRICT_EXPORT++\";\n $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n $GITWEB_CONFIG_SYSTEM = $ENV{'GITWEB_CONFIG_SYSTEM'} || \"++GITWEB_CONFIG_SYSTEM++\";\n \n-sub gitweb_get_feature {\n-\tmy ($name) = @_;\n-\treturn unless exists $feature{$name};\n-\tmy ($sub, $override, @defaults) = (\n-\t\t$feature{$name}{'sub'},\n-\t\t$feature{$name}{'override'},\n-\t\t@{$feature{$name}{'default'}});\n-\t# project specific override is possible only if we have project\n-\tif (!$override || !defined $git_dir) {\n-\t\treturn @defaults;\n-\t}\n-\tif (!defined $sub) {\n-\t\twarn \"feature $name is not overridable\";\n-\t\treturn @defaults;\n-\t}\n-\treturn $sub->(@defaults);\n-}\n-\n-# A wrapper to check if a given feature is enabled.\n-# With this, you can say\n-#\n-#   my $bool_feat = gitweb_check_feature('bool_feat');\n-#   gitweb_check_feature('bool_feat') or somecode;\n-#\n-# instead of\n-#\n-#   my ($bool_feat) = gitweb_get_feature('bool_feat');\n-#   (gitweb_get_feature('bool_feat'))[0] or somecode;\n-#\n-sub gitweb_check_feature {\n-\treturn (gitweb_get_feature(@_))[0];\n-}\n-\n-\n sub feature_bool {\n \tmy $key = shift;\n \tmy ($val) = git_get_project_config($key, '--bool');\n@@ -159,19 +125,6 @@ sub check_export_ok {\n \t\t(!$export_auth_hook || $export_auth_hook->($dir)));\n }\n \n-# process alternate names for backward compatibility\n-# filter out unsupported (unknown) snapshot formats\n-sub filter_snapshot_fmts {\n-\tmy @fmts = @_;\n-\n-\t@fmts = map {\n-\t\texists $known_snapshot_format_aliases{$_} ?\n-\t\t       $known_snapshot_format_aliases{$_} : $_} @fmts;\n-\t@fmts = grep {\n-\t\texists $known_snapshot_formats{$_} &&\n-\t\t!$known_snapshot_formats{$_}{'disabled'}} @fmts;\n-}\n-\n # Get loadavg of system, to compare against $maxload.\n # Currently it requires '/proc/loadavg' present to get loadavg;\n # if it is not present it returns 0, which means no load checking.\n@@ -486,26 +439,6 @@ sub evaluate_git_dir {\n \t$git_dir = \"$projectroot/$project\" if $project;\n }\n \n-our (@snapshot_fmts, $git_avatar);\n-sub configure_gitweb_features {\n-\t# list of supported snapshot formats\n-\tour @snapshot_fmts = gitweb_get_feature('snapshot');\n-\t@snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);\n-\n-\t# check that the avatar feature is set to a known provider name,\n-\t# and for each provider check if the dependencies are satisfied.\n-\t# if the provider name is invalid or the dependencies are not met,\n-\t# reset $git_avatar to the empty string.\n-\tour ($git_avatar) = gitweb_get_feature('avatar');\n-\tif ($git_avatar eq 'gravatar') {\n-\t\t$git_avatar = '' unless (eval { require Digest::MD5; 1; });\n-\t} elsif ($git_avatar eq 'picon') {\n-\t\t# no dependencies\n-\t} else {\n-\t\t$git_avatar = '';\n-\t}\n-}\n-\n # custom error handler: 'die <message>' is Internal Server Error\n sub handle_errors_html {\n \tmy $msg = shift; # it is already HTML escaped\ndiff --git a/gitweb/lib/Gitweb/Config.pm b/gitweb/lib/Gitweb/Config.pm\nindex fdab9f7..3810fda 100644\n--- a/gitweb/lib/Gitweb/Config.pm\n+++ b/gitweb/lib/Gitweb/Config.pm\n@@ -10,13 +10,16 @@ use strict;\n use warnings;\n use Exporter qw(import);\n \n-our @EXPORT = qw(evaluate_gitweb_config $version $projectroot $project_maxdepth $mimetypes_file\n+our @EXPORT = qw(evaluate_gitweb_config gitweb_check_feature gitweb_get_feature configure_gitweb_features\n+                 filter_snapshot_fmts $version $projectroot $project_maxdepth $mimetypes_file $git_avatar\n                  $projects_list @git_base_url_list $export_ok $strict_export $home_link_str $site_name\n                  $site_header $site_footer $home_text @stylesheets $stylesheet $logo $favicon $javascript\n                  $GITWEB_CONFIG $GITWEB_CONFIG_SYSTEM $logo_url $logo_label $export_auth_hook\n                  $projects_list_description_width $default_projects_order $default_blob_plain_mimetype\n                  $default_text_plain_charset $fallback_encoding @diff_opts $prevent_xss $maxload\n-                 %avatar_size %known_snapshot_formats %known_snapshot_format_aliases %feature);\n+                 %avatar_size %known_snapshot_formats %feature @snapshot_fmts);\n+\n+use Gitweb::Git qw($git_dir);\n \n # The following variables are affected by build-time configuration\n # and hence their initialisation is put in gitweb.perl script\n@@ -425,4 +428,71 @@ sub evaluate_gitweb_config {\n \t}\n }\n \n+\n+sub gitweb_get_feature {\n+\tmy ($name) = @_;\n+\treturn unless exists $feature{$name};\n+\tmy ($sub, $override, @defaults) = (\n+\t\t$feature{$name}{'sub'},\n+\t\t$feature{$name}{'override'},\n+\t\t@{$feature{$name}{'default'}});\n+\t# project specific override is possible only if we have project\n+\tif (!$override || !defined $git_dir) {\n+\t\treturn @defaults;\n+\t}\n+\tif (!defined $sub) {\n+\t\twarn \"feature $name is not overridable\";\n+\t\treturn @defaults;\n+\t}\n+\treturn $sub->(@defaults);\n+}\n+\n+# A wrapper to check if a given feature is enabled.\n+# With this, you can say\n+#\n+#   my $bool_feat = gitweb_check_feature('bool_feat');\n+#   gitweb_check_feature('bool_feat') or somecode;\n+#\n+# instead of\n+#\n+#   my ($bool_feat) = gitweb_get_feature('bool_feat');\n+#   (gitweb_get_feature('bool_feat'))[0] or somecode;\n+#\n+sub gitweb_check_feature {\n+\treturn (gitweb_get_feature(@_))[0];\n+}\n+\n+# process alternate names for backward compatibility\n+# filter out unsupported (unknown) snapshot formats\n+sub filter_snapshot_fmts {\n+\tmy @fmts = @_;\n+\n+\t@fmts = map {\n+\t\texists $known_snapshot_format_aliases{$_} ?\n+\t\t       $known_snapshot_format_aliases{$_} : $_} @fmts;\n+\t@fmts = grep {\n+\t\texists $known_snapshot_formats{$_} &&\n+\t\t!$known_snapshot_formats{$_}{'disabled'}} @fmts;\n+}\n+\n+our (@snapshot_fmts, $git_avatar);\n+sub configure_gitweb_features {\n+\t# list of supported snapshot formats\n+\tour @snapshot_fmts = gitweb_get_feature('snapshot');\n+\t@snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);\n+\n+\t# check that the avatar feature is set to a known provider name,\n+\t# and for each provider check if the dependencies are satisfied.\n+\t# if the provider name is invalid or the dependencies are not met,\n+\t# reset $git_avatar to the empty string.\n+\tour ($git_avatar) = gitweb_get_feature('avatar');\n+\tif ($git_avatar eq 'gravatar') {\n+\t\t$git_avatar = '' unless (eval { require Digest::MD5; 1; });\n+\t} elsif ($git_avatar eq 'picon') {\n+\t\t# no dependencies\n+\t} else {\n+\t\t$git_avatar = '';\n+\t}\n+}\n+\n 1;\n-- \n1.7.1.454.ga8c50c\n"},{"id":"143187","messageId":"1275943844-24991-2-git-send-email-pavan.sss1991@gmail.com","threadId":"24034","inReplyTo":"1275943844-24991-1-git-send-email-pavan.sss1991@gmail.com","subject":"[RFC/PATCH 2/4] gitweb: Create Gitweb::HTML::Link module","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2010-06-07T20:50:42Z","receivedAt":"2010-06-07T20:50:42Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"Create Gitweb::HTML::Link module in 'gitweb/lib/Gitweb/HTML/Link.pm'\nto store the subroutines from the section 'action links' in the\nprevious gitweb.perl\n\nSubroutines moved:\n\thref\n\nUpdate 'gitweb/Makefile' to install this module alongside gitweb.\n\nSigned-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>\n---\n gitweb/Makefile                |    5 +-\n gitweb/gitweb.perl             |  123 +-----------------------------------\n gitweb/lib/Gitweb/HTML/Link.pm |  137 ++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 142 insertions(+), 123 deletions(-)\n create mode 100644 gitweb/lib/Gitweb/HTML/Link.pm\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex fcd4042..28f0858 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -116,6 +116,8 @@ GITWEB_LIB_GITWEB += lib/Gitweb/Config.pm\n GITWEB_LIB_GITWEB += lib/Gitweb/Request.pm\n GITWEB_LIB_GITWEB += lib/Gitweb/Escape.pm\n GITWEB_LIB_GITWEB += lib/Gitweb/Git.pm\n+# Files: gitweb/lib/Gitweb/HTML\n+GITWEB_LIB_GITWEB_HTML += lib/Gitweb/HTML/Link.pm\n \n GITWEB_REPLACE = \\\n \t-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \\\n@@ -157,8 +159,9 @@ install: all\n \t$(INSTALL) -m 755 $(GITWEB_PROGRAMS) '$(DESTDIR_SQ)$(gitwebdir_SQ)'\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n \t$(INSTALL) -m 644 $(GITWEB_FILES) '$(DESTDIR_SQ)$(gitwebstaticdir_SQ)'\n-\t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitweblibdir_SQ)/Gitweb'\n+\t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitweblibdir_SQ)/Gitweb/HTML'\n \t$(INSTALL) -m 644 $(GITWEB_LIB_GITWEB) '$(DESTDIR_SQ)$(gitweblibdir_SQ)/Gitweb'\n+\t$(INSTALL) -m 644 $(GITWEB_LIB_GITWEB_HTML) '$(DESTDIR_SQ)$(gitweblibdir_SQ)/Gitweb/HTML'\n \n ### Cleaning rules\n \ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 3931064..bd11ae0 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -28,6 +28,7 @@ use Gitweb::Git;\n use Gitweb::Config;\n use Gitweb::Request;\n use Gitweb::Escape;\n+use Gitweb::HTML::Link;\n \n BEGIN {\n \tCGI->compile() if $ENV{'MOD_PERL'};\n@@ -554,128 +555,6 @@ sub run {\n run();\n \n ## ======================================================================\n-## action links\n-\n-# possible values of extra options\n-# -full => 0|1      - use absolute/full URL ($my_uri/$my_url as base)\n-# -replay => 1      - start from a current view (replay with modifications)\n-# -path_info => 0|1 - don't use/use path_info URL (if possible)\n-sub href {\n-\tmy %params = @_;\n-\t# default is to use -absolute url() i.e. $my_uri\n-\tmy $href = $params{-full} ? $my_url : $my_uri;\n-\n-\t$params{'project'} = $project unless exists $params{'project'};\n-\n-\tif ($params{-replay}) {\n-\t\twhile (my ($name, $symbol) = each %cgi_param_mapping) {\n-\t\t\tif (!exists $params{$name}) {\n-\t\t\t\t$params{$name} = $input_params{$name};\n-\t\t\t}\n-\t\t}\n-\t}\n-\n-\tmy $use_pathinfo = gitweb_check_feature('pathinfo');\n-\tif (defined $params{'project'} &&\n-\t    (exists $params{-path_info} ? $params{-path_info} : $use_pathinfo)) {\n-\t\t# try to put as many parameters as possible in PATH_INFO:\n-\t\t#   - project name\n-\t\t#   - action\n-\t\t#   - hash_parent or hash_parent_base:/file_parent\n-\t\t#   - hash or hash_base:/filename\n-\t\t#   - the snapshot_format as an appropriate suffix\n-\n-\t\t# When the script is the root DirectoryIndex for the domain,\n-\t\t# $href here would be something like http://gitweb.example.com/\n-\t\t# Thus, we strip any trailing / from $href, to spare us double\n-\t\t# slashes in the final URL\n-\t\t$href =~ s,/$,,;\n-\n-\t\t# Then add the project name, if present\n-\t\t$href .= \"/\".esc_url($params{'project'});\n-\t\tdelete $params{'project'};\n-\n-\t\t# since we destructively absorb parameters, we keep this\n-\t\t# boolean that remembers if we're handling a snapshot\n-\t\tmy $is_snapshot = $params{'action'} eq 'snapshot';\n-\n-\t\t# Summary just uses the project path URL, any other action is\n-\t\t# added to the URL\n-\t\tif (defined $params{'action'}) {\n-\t\t\t$href .= \"/\".esc_url($params{'action'}) unless $params{'action'} eq 'summary';\n-\t\t\tdelete $params{'action'};\n-\t\t}\n-\n-\t\t# Next, we put hash_parent_base:/file_parent..hash_base:/file_name,\n-\t\t# stripping nonexistent or useless pieces\n-\t\t$href .= \"/\" if ($params{'hash_base'} || $params{'hash_parent_base'}\n-\t\t\t|| $params{'hash_parent'} || $params{'hash'});\n-\t\tif (defined $params{'hash_base'}) {\n-\t\t\tif (defined $params{'hash_parent_base'}) {\n-\t\t\t\t$href .= esc_url($params{'hash_parent_base'});\n-\t\t\t\t# skip the file_parent if it's the same as the file_name\n-\t\t\t\tif (defined $params{'file_parent'}) {\n-\t\t\t\t\tif (defined $params{'file_name'} && $params{'file_parent'} eq $params{'file_name'}) {\n-\t\t\t\t\t\tdelete $params{'file_parent'};\n-\t\t\t\t\t} elsif ($params{'file_parent'} !~ /\\.\\./) {\n-\t\t\t\t\t\t$href .= \":/\".esc_url($params{'file_parent'});\n-\t\t\t\t\t\tdelete $params{'file_parent'};\n-\t\t\t\t\t}\n-\t\t\t\t}\n-\t\t\t\t$href .= \"..\";\n-\t\t\t\tdelete $params{'hash_parent'};\n-\t\t\t\tdelete $params{'hash_parent_base'};\n-\t\t\t} elsif (defined $params{'hash_parent'}) {\n-\t\t\t\t$href .= esc_url($params{'hash_parent'}). \"..\";\n-\t\t\t\tdelete $params{'hash_parent'};\n-\t\t\t}\n-\n-\t\t\t$href .= esc_url($params{'hash_base'});\n-\t\t\tif (defined $params{'file_name'} && $params{'file_name'} !~ /\\.\\./) {\n-\t\t\t\t$href .= \":/\".esc_url($params{'file_name'});\n-\t\t\t\tdelete $params{'file_name'};\n-\t\t\t}\n-\t\t\tdelete $params{'hash'};\n-\t\t\tdelete $params{'hash_base'};\n-\t\t} elsif (defined $params{'hash'}) {\n-\t\t\t$href .= esc_url($params{'hash'});\n-\t\t\tdelete $params{'hash'};\n-\t\t}\n-\n-\t\t# If the action was a snapshot, we can absorb the\n-\t\t# snapshot_format parameter too\n-\t\tif ($is_snapshot) {\n-\t\t\tmy $fmt = $params{'snapshot_format'};\n-\t\t\t# snapshot_format should always be defined when href()\n-\t\t\t# is called, but just in case some code forgets, we\n-\t\t\t# fall back to the default\n-\t\t\t$fmt ||= $snapshot_fmts[0];\n-\t\t\t$href .= $known_snapshot_formats{$fmt}{'suffix'};\n-\t\t\tdelete $params{'snapshot_format'};\n-\t\t}\n-\t}\n-\n-\t# now encode the parameters explicitly\n-\tmy @result = ();\n-\tfor (my $i = 0; $i < @cgi_param_mapping; $i += 2) {\n-\t\tmy ($name, $symbol) = ($cgi_param_mapping[$i], $cgi_param_mapping[$i+1]);\n-\t\tif (defined $params{$name}) {\n-\t\t\tif (ref($params{$name}) eq \"ARRAY\") {\n-\t\t\t\tforeach my $par (@{$params{$name}}) {\n-\t\t\t\t\tpush @result, $symbol . \"=\" . esc_param($par);\n-\t\t\t\t}\n-\t\t\t} else {\n-\t\t\t\tpush @result, $symbol . \"=\" . esc_param($params{$name});\n-\t\t\t}\n-\t\t}\n-\t}\n-\t$href .= \"?\" . join(';', @result) if scalar @result;\n-\n-\treturn $href;\n-}\n-\n-\n-## ======================================================================\n ## validation, quoting/unquoting and escaping\n \n sub validate_action {\ndiff --git a/gitweb/lib/Gitweb/HTML/Link.pm b/gitweb/lib/Gitweb/HTML/Link.pm\nnew file mode 100644\nindex 0000000..086809f\n--- /dev/null\n+++ b/gitweb/lib/Gitweb/HTML/Link.pm\n@@ -0,0 +1,137 @@\n+#!/usr/bin/perl\n+#\n+# Gitweb::HTML::Link -- gitweb's action links package\n+#\n+# This program is licensed under the GPLv2\n+\n+package Gitweb::HTML::Link;\n+\n+use strict;\n+use warnings;\n+use Exporter qw(import);\n+\n+our @EXPORT = qw(href);\n+\n+use Gitweb::Config qw(gitweb_check_feature %known_snapshot_formats @snapshot_fmts);\n+use Gitweb::Request qw($project %cgi_param_mapping @cgi_param_mapping $my_url $my_uri %input_params);\n+use Gitweb::Escape qw(esc_url esc_param);\n+\n+# possible values of extra options\n+# -full => 0|1      - use absolute/full URL ($my_uri/$my_url as base)\n+# -replay => 1      - start from a current view (replay with modifications)\n+# -path_info => 0|1 - don't use/use path_info URL (if possible)\n+sub href {\n+\tmy %params = @_;\n+\t# default is to use -absolute url() i.e. $my_uri\n+\tmy $href = $params{-full} ? $my_url : $my_uri;\n+\n+\t$params{'project'} = $project unless exists $params{'project'};\n+\n+\tif ($params{-replay}) {\n+\t\twhile (my ($name, $symbol) = each %cgi_param_mapping) {\n+\t\t\tif (!exists $params{$name}) {\n+\t\t\t\t$params{$name} = $input_params{$name};\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tmy $use_pathinfo = gitweb_check_feature('pathinfo');\n+\tif (defined $params{'project'} &&\n+\t    (exists $params{-path_info} ? $params{-path_info} : $use_pathinfo)) {\n+\t\t# try to put as many parameters as possible in PATH_INFO:\n+\t\t#   - project name\n+\t\t#   - action\n+\t\t#   - hash_parent or hash_parent_base:/file_parent\n+\t\t#   - hash or hash_base:/filename\n+\t\t#   - the snapshot_format as an appropriate suffix\n+\n+\t\t# When the script is the root DirectoryIndex for the domain,\n+\t\t# $href here would be something like http://gitweb.example.com/\n+\t\t# Thus, we strip any trailing / from $href, to spare us double\n+\t\t# slashes in the final URL\n+\t\t$href =~ s,/$,,;\n+\n+\t\t# Then add the project name, if present\n+\t\t$href .= \"/\".esc_url($params{'project'});\n+\t\tdelete $params{'project'};\n+\n+\t\t# since we destructively absorb parameters, we keep this\n+\t\t# boolean that remembers if we're handling a snapshot\n+\t\tmy $is_snapshot = $params{'action'} eq 'snapshot';\n+\n+\t\t# Summary just uses the project path URL, any other action is\n+\t\t# added to the URL\n+\t\tif (defined $params{'action'}) {\n+\t\t\t$href .= \"/\".esc_url($params{'action'}) unless $params{'action'} eq 'summary';\n+\t\t\tdelete $params{'action'};\n+\t\t}\n+\n+\t\t# Next, we put hash_parent_base:/file_parent..hash_base:/file_name,\n+\t\t# stripping nonexistent or useless pieces\n+\t\t$href .= \"/\" if ($params{'hash_base'} || $params{'hash_parent_base'}\n+\t\t\t|| $params{'hash_parent'} || $params{'hash'});\n+\t\tif (defined $params{'hash_base'}) {\n+\t\t\tif (defined $params{'hash_parent_base'}) {\n+\t\t\t\t$href .= esc_url($params{'hash_parent_base'});\n+\t\t\t\t# skip the file_parent if it's the same as the file_name\n+\t\t\t\tif (defined $params{'file_parent'}) {\n+\t\t\t\t\tif (defined $params{'file_name'} && $params{'file_parent'} eq $params{'file_name'}) {\n+\t\t\t\t\t\tdelete $params{'file_parent'};\n+\t\t\t\t\t} elsif ($params{'file_parent'} !~ /\\.\\./) {\n+\t\t\t\t\t\t$href .= \":/\".esc_url($params{'file_parent'});\n+\t\t\t\t\t\tdelete $params{'file_parent'};\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t\t$href .= \"..\";\n+\t\t\t\tdelete $params{'hash_parent'};\n+\t\t\t\tdelete $params{'hash_parent_base'};\n+\t\t\t} elsif (defined $params{'hash_parent'}) {\n+\t\t\t\t$href .= esc_url($params{'hash_parent'}). \"..\";\n+\t\t\t\tdelete $params{'hash_parent'};\n+\t\t\t}\n+\n+\t\t\t$href .= esc_url($params{'hash_base'});\n+\t\t\tif (defined $params{'file_name'} && $params{'file_name'} !~ /\\.\\./) {\n+\t\t\t\t$href .= \":/\".esc_url($params{'file_name'});\n+\t\t\t\tdelete $params{'file_name'};\n+\t\t\t}\n+\t\t\tdelete $params{'hash'};\n+\t\t\tdelete $params{'hash_base'};\n+\t\t} elsif (defined $params{'hash'}) {\n+\t\t\t$href .= esc_url($params{'hash'});\n+\t\t\tdelete $params{'hash'};\n+\t\t}\n+\n+\t\t# If the action was a snapshot, we can absorb the\n+\t\t# snapshot_format parameter too\n+\t\tif ($is_snapshot) {\n+\t\t\tmy $fmt = $params{'snapshot_format'};\n+\t\t\t# snapshot_format should always be defined when href()\n+\t\t\t# is called, but just in case some code forgets, we\n+\t\t\t# fall back to the default\n+\t\t\t$fmt ||= $snapshot_fmts[0];\n+\t\t\t$href .= $known_snapshot_formats{$fmt}{'suffix'};\n+\t\t\tdelete $params{'snapshot_format'};\n+\t\t}\n+\t}\n+\n+\t# now encode the parameters explicitly\n+\tmy @result = ();\n+\tfor (my $i = 0; $i < @cgi_param_mapping; $i += 2) {\n+\t\tmy ($name, $symbol) = ($cgi_param_mapping[$i], $cgi_param_mapping[$i+1]);\n+\t\tif (defined $params{$name}) {\n+\t\t\tif (ref($params{$name}) eq \"ARRAY\") {\n+\t\t\t\tforeach my $par (@{$params{$name}}) {\n+\t\t\t\t\tpush @result, $symbol . \"=\" . esc_param($par);\n+\t\t\t\t}\n+\t\t\t} else {\n+\t\t\t\tpush @result, $symbol . \"=\" . esc_param($params{$name});\n+\t\t\t}\n+\t\t}\n+\t}\n+\t$href .= \"?\" . join(';', @result) if scalar @result;\n+\n+\treturn $href;\n+}\n+\n+1;\n-- \n1.7.1.454.ga8c50c\n"},{"id":"143188","messageId":"1275943844-24991-3-git-send-email-pavan.sss1991@gmail.com","threadId":"24034","inReplyTo":"1275943844-24991-1-git-send-email-pavan.sss1991@gmail.com","subject":"[RFC/PATCH 3/4] gitweb: Create Gitweb::HTML module","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2010-06-07T20:50:43Z","receivedAt":"2010-06-07T20:50:43Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"Create Gitweb::HTML module in 'gitweb/lib/Gitweb/HTML.pm'\nto import all the Gitweb::HTML::* modules into it.\n\nUpdate gitweb/Makefile to install Gitweb::HTML alongside gitweb.\n\nSigned-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>\n---\n gitweb/Makefile           |    1 +\n gitweb/gitweb.perl        |    2 +-\n gitweb/lib/Gitweb/HTML.pm |   17 +++++++++++++++++\n 3 files changed, 19 insertions(+), 1 deletions(-)\n create mode 100644 gitweb/lib/Gitweb/HTML.pm\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex 28f0858..5e44ace 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -116,6 +116,7 @@ GITWEB_LIB_GITWEB += lib/Gitweb/Config.pm\n GITWEB_LIB_GITWEB += lib/Gitweb/Request.pm\n GITWEB_LIB_GITWEB += lib/Gitweb/Escape.pm\n GITWEB_LIB_GITWEB += lib/Gitweb/Git.pm\n+GITWEB_LIB_GITWEB += lib/Gitweb/HTML.pm\n # Files: gitweb/lib/Gitweb/HTML\n GITWEB_LIB_GITWEB_HTML += lib/Gitweb/HTML/Link.pm\n \ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex bd11ae0..12646c0 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -28,7 +28,7 @@ use Gitweb::Git;\n use Gitweb::Config;\n use Gitweb::Request;\n use Gitweb::Escape;\n-use Gitweb::HTML::Link;\n+use Gitweb::HTML;\n \n BEGIN {\n \tCGI->compile() if $ENV{'MOD_PERL'};\ndiff --git a/gitweb/lib/Gitweb/HTML.pm b/gitweb/lib/Gitweb/HTML.pm\nnew file mode 100644\nindex 0000000..a0a1606\n--- /dev/null\n+++ b/gitweb/lib/Gitweb/HTML.pm\n@@ -0,0 +1,17 @@\n+#!/usr/bin/perl\n+#\n+# Gitweb::HTML -- gitweb's HTML subs package\n+#\n+# This program is licensed under the GPLv2\n+\n+package Gitweb::HTML;\n+\n+use strict;\n+use warnings;\n+use Exporter qw(import);\n+\n+our @EXPORT = qw(href);\n+\n+use Gitweb::HTML::Link;\n+\n+1;\n-- \n1.7.1.454.ga8c50c\n"},{"id":"143189","messageId":"1275943844-24991-4-git-send-email-pavan.sss1991@gmail.com","threadId":"24034","inReplyTo":"1275943844-24991-1-git-send-email-pavan.sss1991@gmail.com","subject":"[RFC/PATCH 4/4] gitweb: Create Gitweb::HTML::String module","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2010-06-07T20:50:44Z","receivedAt":"2010-06-07T20:50:44Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"Create Gitweb::HTML::String module in 'gitweb/lib/Gitweb/HTML/String.pm'\nto store all the subs involving string manipulation and those returning\nshort strings regarding gitweb.perl.\n\nSubroutines moved:\n\tchop_str\n\tchop_and_escape_str\n\tS_ISGITLINK\n\tmode_str\n\tfile_type\n\tfile_type_long\n\tage_class\n\tage_string\n\nImport Gitweb::HTML:String into Gitweb::HTML module.\n\nUpdare gitweb/Makefile to install this module alongside gitweb.\n\nSigned-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>\n---\n gitweb/Makefile                  |    1 +\n gitweb/gitweb.perl               |  213 ----------------------------------\n gitweb/lib/Gitweb/HTML.pm        |    4 +-\n gitweb/lib/Gitweb/HTML/String.pm |  232 ++++++++++++++++++++++++++++++++++++++\n 4 files changed, 236 insertions(+), 214 deletions(-)\n create mode 100644 gitweb/lib/Gitweb/HTML/String.pm\n\ndiff --git a/gitweb/Makefile b/gitweb/Makefile\nindex 5e44ace..9b4b718 100644\n--- a/gitweb/Makefile\n+++ b/gitweb/Makefile\n@@ -119,6 +119,7 @@ GITWEB_LIB_GITWEB += lib/Gitweb/Git.pm\n GITWEB_LIB_GITWEB += lib/Gitweb/HTML.pm\n # Files: gitweb/lib/Gitweb/HTML\n GITWEB_LIB_GITWEB_HTML += lib/Gitweb/HTML/Link.pm\n+GITWEB_LIB_GITWEB_HTML += lib/Gitweb/HTML/String.pm\n \n GITWEB_REPLACE = \\\n \t-e 's|++GIT_VERSION++|$(GIT_VERSION)|g' \\\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 12646c0..f2fdcae 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -582,219 +582,6 @@ sub project_in_list {\n }\n \n ## ----------------------------------------------------------------------\n-## HTML aware string manipulation\n-\n-# Try to chop given string on a word boundary between position\n-# $len and $len+$add_len. If there is no word boundary there,\n-# chop at $len+$add_len. Do not chop if chopped part plus ellipsis\n-# (marking chopped part) would be longer than given string.\n-sub chop_str {\n-\tmy $str = shift;\n-\tmy $len = shift;\n-\tmy $add_len = shift || 10;\n-\tmy $where = shift || 'right'; # 'left' | 'center' | 'right'\n-\n-\t# Make sure perl knows it is utf8 encoded so we don't\n-\t# cut in the middle of a utf8 multibyte char.\n-\t$str = to_utf8($str);\n-\n-\t# allow only $len chars, but don't cut a word if it would fit in $add_len\n-\t# if it doesn't fit, cut it if it's still longer than the dots we would add\n-\t# remove chopped character entities entirely\n-\n-\t# when chopping in the middle, distribute $len into left and right part\n-\t# return early if chopping wouldn't make string shorter\n-\tif ($where eq 'center') {\n-\t\treturn $str if ($len + 5 >= length($str)); # filler is length 5\n-\t\t$len = int($len/2);\n-\t} else {\n-\t\treturn $str if ($len + 4 >= length($str)); # filler is length 4\n-\t}\n-\n-\t# regexps: ending and beginning with word part up to $add_len\n-\tmy $endre = qr/.{$len}\\w{0,$add_len}/;\n-\tmy $begre = qr/\\w{0,$add_len}.{$len}/;\n-\n-\tif ($where eq 'left') {\n-\t\t$str =~ m/^(.*?)($begre)$/;\n-\t\tmy ($lead, $body) = ($1, $2);\n-\t\tif (length($lead) > 4) {\n-\t\t\t$lead = \" ...\";\n-\t\t}\n-\t\treturn \"$lead$body\";\n-\n-\t} elsif ($where eq 'center') {\n-\t\t$str =~ m/^($endre)(.*)$/;\n-\t\tmy ($left, $str)  = ($1, $2);\n-\t\t$str =~ m/^(.*?)($begre)$/;\n-\t\tmy ($mid, $right) = ($1, $2);\n-\t\tif (length($mid) > 5) {\n-\t\t\t$mid = \" ... \";\n-\t\t}\n-\t\treturn \"$left$mid$right\";\n-\n-\t} else {\n-\t\t$str =~ m/^($endre)(.*)$/;\n-\t\tmy $body = $1;\n-\t\tmy $tail = $2;\n-\t\tif (length($tail) > 4) {\n-\t\t\t$tail = \"... \";\n-\t\t}\n-\t\treturn \"$body$tail\";\n-\t}\n-}\n-\n-# takes the same arguments as chop_str, but also wraps a <span> around the\n-# result with a title attribute if it does get chopped. Additionally, the\n-# string is HTML-escaped.\n-sub chop_and_escape_str {\n-\tmy ($str) = @_;\n-\n-\tmy $chopped = chop_str(@_);\n-\tif ($chopped eq $str) {\n-\t\treturn esc_html($chopped);\n-\t} else {\n-\t\t$str =~ s/[[:cntrl:]]/?/g;\n-\t\treturn $cgi->span({-title=>$str}, esc_html($chopped));\n-\t}\n-}\n-\n-## ----------------------------------------------------------------------\n-## functions returning short strings\n-\n-# CSS class for given age value (in seconds)\n-sub age_class {\n-\tmy $age = shift;\n-\n-\tif (!defined $age) {\n-\t\treturn \"noage\";\n-\t} elsif ($age < 60*60*2) {\n-\t\treturn \"age0\";\n-\t} elsif ($age < 60*60*24*2) {\n-\t\treturn \"age1\";\n-\t} else {\n-\t\treturn \"age2\";\n-\t}\n-}\n-\n-# convert age in seconds to \"nn units ago\" string\n-sub age_string {\n-\tmy $age = shift;\n-\tmy $age_str;\n-\n-\tif ($age > 60*60*24*365*2) {\n-\t\t$age_str = (int $age/60/60/24/365);\n-\t\t$age_str .= \" years ago\";\n-\t} elsif ($age > 60*60*24*(365/12)*2) {\n-\t\t$age_str = int $age/60/60/24/(365/12);\n-\t\t$age_str .= \" months ago\";\n-\t} elsif ($age > 60*60*24*7*2) {\n-\t\t$age_str = int $age/60/60/24/7;\n-\t\t$age_str .= \" weeks ago\";\n-\t} elsif ($age > 60*60*24*2) {\n-\t\t$age_str = int $age/60/60/24;\n-\t\t$age_str .= \" days ago\";\n-\t} elsif ($age > 60*60*2) {\n-\t\t$age_str = int $age/60/60;\n-\t\t$age_str .= \" hours ago\";\n-\t} elsif ($age > 60*2) {\n-\t\t$age_str = int $age/60;\n-\t\t$age_str .= \" min ago\";\n-\t} elsif ($age > 2) {\n-\t\t$age_str = int $age;\n-\t\t$age_str .= \" sec ago\";\n-\t} else {\n-\t\t$age_str .= \" right now\";\n-\t}\n-\treturn $age_str;\n-}\n-\n-use constant {\n-\tS_IFINVALID => 0030000,\n-\tS_IFGITLINK => 0160000,\n-};\n-\n-# submodule/subproject, a commit object reference\n-sub S_ISGITLINK {\n-\tmy $mode = shift;\n-\n-\treturn (($mode & S_IFMT) == S_IFGITLINK)\n-}\n-\n-# convert file mode in octal to symbolic file mode string\n-sub mode_str {\n-\tmy $mode = oct shift;\n-\n-\tif (S_ISGITLINK($mode)) {\n-\t\treturn 'm---------';\n-\t} elsif (S_ISDIR($mode & S_IFMT)) {\n-\t\treturn 'drwxr-xr-x';\n-\t} elsif (S_ISLNK($mode)) {\n-\t\treturn 'lrwxrwxrwx';\n-\t} elsif (S_ISREG($mode)) {\n-\t\t# git cares only about the executable bit\n-\t\tif ($mode & S_IXUSR) {\n-\t\t\treturn '-rwxr-xr-x';\n-\t\t} else {\n-\t\t\treturn '-rw-r--r--';\n-\t\t};\n-\t} else {\n-\t\treturn '----------';\n-\t}\n-}\n-\n-# convert file mode in octal to file type string\n-sub file_type {\n-\tmy $mode = shift;\n-\n-\tif ($mode !~ m/^[0-7]+$/) {\n-\t\treturn $mode;\n-\t} else {\n-\t\t$mode = oct $mode;\n-\t}\n-\n-\tif (S_ISGITLINK($mode)) {\n-\t\treturn \"submodule\";\n-\t} elsif (S_ISDIR($mode & S_IFMT)) {\n-\t\treturn \"directory\";\n-\t} elsif (S_ISLNK($mode)) {\n-\t\treturn \"symlink\";\n-\t} elsif (S_ISREG($mode)) {\n-\t\treturn \"file\";\n-\t} else {\n-\t\treturn \"unknown\";\n-\t}\n-}\n-\n-# convert file mode in octal to file type description string\n-sub file_type_long {\n-\tmy $mode = shift;\n-\n-\tif ($mode !~ m/^[0-7]+$/) {\n-\t\treturn $mode;\n-\t} else {\n-\t\t$mode = oct $mode;\n-\t}\n-\n-\tif (S_ISGITLINK($mode)) {\n-\t\treturn \"submodule\";\n-\t} elsif (S_ISDIR($mode & S_IFMT)) {\n-\t\treturn \"directory\";\n-\t} elsif (S_ISLNK($mode)) {\n-\t\treturn \"symlink\";\n-\t} elsif (S_ISREG($mode)) {\n-\t\tif ($mode & S_IXUSR) {\n-\t\t\treturn \"executable\";\n-\t\t} else {\n-\t\t\treturn \"file\";\n-\t\t};\n-\t} else {\n-\t\treturn \"unknown\";\n-\t}\n-}\n-\n-\n-## ----------------------------------------------------------------------\n ## functions returning short HTML fragments, or transforming HTML fragments\n ## which don't belong to other sections\n \ndiff --git a/gitweb/lib/Gitweb/HTML.pm b/gitweb/lib/Gitweb/HTML.pm\nindex a0a1606..49a3b54 100644\n--- a/gitweb/lib/Gitweb/HTML.pm\n+++ b/gitweb/lib/Gitweb/HTML.pm\n@@ -10,8 +10,10 @@ use strict;\n use warnings;\n use Exporter qw(import);\n \n-our @EXPORT = qw(href);\n+our @EXPORT = qw(href chop_str chop_and_escape_str age_class age_string mode_str\n+                 file_type file_type_long);\n \n use Gitweb::HTML::Link;\n+use Gitweb::HTML::String;\n \n 1;\ndiff --git a/gitweb/lib/Gitweb/HTML/String.pm b/gitweb/lib/Gitweb/HTML/String.pm\nnew file mode 100644\nindex 0000000..a8f6417\n--- /dev/null\n+++ b/gitweb/lib/Gitweb/HTML/String.pm\n@@ -0,0 +1,232 @@\n+#!/usr/bin/perl\n+#\n+# Gitweb::HTML::String -- gitweb's string manipulation & short string package\n+#\n+# This program is licensed under the GPLv2\n+\n+package Gitweb::HTML::String;\n+\n+use strict;\n+use warnings;\n+use Exporter qw(import);\n+\n+our @EXPORT = qw(chop_str chop_and_escape_str age_class age_string mode_str\n+                 file_type file_type_long);\n+\n+use Fcntl ':mode';\n+use Gitweb::Request qw($cgi);\n+use Gitweb::Escape qw(to_utf8 esc_html);\n+\n+## ----------------------------------------------------------------------\n+## HTML aware string manipulation\n+\n+# Try to chop given string on a word boundary between position\n+# $len and $len+$add_len. If there is no word boundary there,\n+# chop at $len+$add_len. Do not chop if chopped part plus ellipsis\n+# (marking chopped part) would be longer than given string.\n+sub chop_str {\n+\tmy $str = shift;\n+\tmy $len = shift;\n+\tmy $add_len = shift || 10;\n+\tmy $where = shift || 'right'; # 'left' | 'center' | 'right'\n+\n+\t# Make sure perl knows it is utf8 encoded so we don't\n+\t# cut in the middle of a utf8 multibyte char.\n+\t$str = to_utf8($str);\n+\n+\t# allow only $len chars, but don't cut a word if it would fit in $add_len\n+\t# if it doesn't fit, cut it if it's still longer than the dots we would add\n+\t# remove chopped character entities entirely\n+\n+\t# when chopping in the middle, distribute $len into left and right part\n+\t# return early if chopping wouldn't make string shorter\n+\tif ($where eq 'center') {\n+\t\treturn $str if ($len + 5 >= length($str)); # filler is length 5\n+\t\t$len = int($len/2);\n+\t} else {\n+\t\treturn $str if ($len + 4 >= length($str)); # filler is length 4\n+\t}\n+\n+\t# regexps: ending and beginning with word part up to $add_len\n+\tmy $endre = qr/.{$len}\\w{0,$add_len}/;\n+\tmy $begre = qr/\\w{0,$add_len}.{$len}/;\n+\n+\tif ($where eq 'left') {\n+\t\t$str =~ m/^(.*?)($begre)$/;\n+\t\tmy ($lead, $body) = ($1, $2);\n+\t\tif (length($lead) > 4) {\n+\t\t\t$lead = \" ...\";\n+\t\t}\n+\t\treturn \"$lead$body\";\n+\n+\t} elsif ($where eq 'center') {\n+\t\t$str =~ m/^($endre)(.*)$/;\n+\t\tmy ($left, $str)  = ($1, $2);\n+\t\t$str =~ m/^(.*?)($begre)$/;\n+\t\tmy ($mid, $right) = ($1, $2);\n+\t\tif (length($mid) > 5) {\n+\t\t\t$mid = \" ... \";\n+\t\t}\n+\t\treturn \"$left$mid$right\";\n+\n+\t} else {\n+\t\t$str =~ m/^($endre)(.*)$/;\n+\t\tmy $body = $1;\n+\t\tmy $tail = $2;\n+\t\tif (length($tail) > 4) {\n+\t\t\t$tail = \"... \";\n+\t\t}\n+\t\treturn \"$body$tail\";\n+\t}\n+}\n+\n+# takes the same arguments as chop_str, but also wraps a <span> around the\n+# result with a title attribute if it does get chopped. Additionally, the\n+# string is HTML-escaped.\n+sub chop_and_escape_str {\n+\tmy ($str) = @_;\n+\n+\tmy $chopped = chop_str(@_);\n+\tif ($chopped eq $str) {\n+\t\treturn esc_html($chopped);\n+\t} else {\n+\t\t$str =~ s/[[:cntrl:]]/?/g;\n+\t\treturn $cgi->span({-title=>$str}, esc_html($chopped));\n+\t}\n+}\n+\n+## ----------------------------------------------------------------------\n+## functions returning short strings\n+\n+# CSS class for given age value (in seconds)\n+sub age_class {\n+\tmy $age = shift;\n+\n+\tif (!defined $age) {\n+\t\treturn \"noage\";\n+\t} elsif ($age < 60*60*2) {\n+\t\treturn \"age0\";\n+\t} elsif ($age < 60*60*24*2) {\n+\t\treturn \"age1\";\n+\t} else {\n+\t\treturn \"age2\";\n+\t}\n+}\n+\n+# convert age in seconds to \"nn units ago\" string\n+sub age_string {\n+\tmy $age = shift;\n+\tmy $age_str;\n+\n+\tif ($age > 60*60*24*365*2) {\n+\t\t$age_str = (int $age/60/60/24/365);\n+\t\t$age_str .= \" years ago\";\n+\t} elsif ($age > 60*60*24*(365/12)*2) {\n+\t\t$age_str = int $age/60/60/24/(365/12);\n+\t\t$age_str .= \" months ago\";\n+\t} elsif ($age > 60*60*24*7*2) {\n+\t\t$age_str = int $age/60/60/24/7;\n+\t\t$age_str .= \" weeks ago\";\n+\t} elsif ($age > 60*60*24*2) {\n+\t\t$age_str = int $age/60/60/24;\n+\t\t$age_str .= \" days ago\";\n+\t} elsif ($age > 60*60*2) {\n+\t\t$age_str = int $age/60/60;\n+\t\t$age_str .= \" hours ago\";\n+\t} elsif ($age > 60*2) {\n+\t\t$age_str = int $age/60;\n+\t\t$age_str .= \" min ago\";\n+\t} elsif ($age > 2) {\n+\t\t$age_str = int $age;\n+\t\t$age_str .= \" sec ago\";\n+\t} else {\n+\t\t$age_str .= \" right now\";\n+\t}\n+\treturn $age_str;\n+}\n+\n+use constant {\n+\tS_IFINVALID => 0030000,\n+\tS_IFGITLINK => 0160000,\n+};\n+\n+# submodule/subproject, a commit object reference\n+sub S_ISGITLINK {\n+\tmy $mode = shift;\n+\n+\treturn (($mode & S_IFMT) == S_IFGITLINK)\n+}\n+\n+# convert file mode in octal to symbolic file mode string\n+sub mode_str {\n+\tmy $mode = oct shift;\n+\n+\tif (S_ISGITLINK($mode)) {\n+\t\treturn 'm---------';\n+\t} elsif (S_ISDIR($mode & S_IFMT)) {\n+\t\treturn 'drwxr-xr-x';\n+\t} elsif (S_ISLNK($mode)) {\n+\t\treturn 'lrwxrwxrwx';\n+\t} elsif (S_ISREG($mode)) {\n+\t\t# git cares only about the executable bit\n+\t\tif ($mode & S_IXUSR) {\n+\t\t\treturn '-rwxr-xr-x';\n+\t\t} else {\n+\t\t\treturn '-rw-r--r--';\n+\t\t};\n+\t} else {\n+\t\treturn '----------';\n+\t}\n+}\n+\n+# convert file mode in octal to file type string\n+sub file_type {\n+\tmy $mode = shift;\n+\n+\tif ($mode !~ m/^[0-7]+$/) {\n+\t\treturn $mode;\n+\t} else {\n+\t\t$mode = oct $mode;\n+\t}\n+\n+\tif (S_ISGITLINK($mode)) {\n+\t\treturn \"submodule\";\n+\t} elsif (S_ISDIR($mode & S_IFMT)) {\n+\t\treturn \"directory\";\n+\t} elsif (S_ISLNK($mode)) {\n+\t\treturn \"symlink\";\n+\t} elsif (S_ISREG($mode)) {\n+\t\treturn \"file\";\n+\t} else {\n+\t\treturn \"unknown\";\n+\t}\n+}\n+\n+# convert file mode in octal to file type description string\n+sub file_type_long {\n+\tmy $mode = shift;\n+\n+\tif ($mode !~ m/^[0-7]+$/) {\n+\t\treturn $mode;\n+\t} else {\n+\t\t$mode = oct $mode;\n+\t}\n+\n+\tif (S_ISGITLINK($mode)) {\n+\t\treturn \"submodule\";\n+\t} elsif (S_ISDIR($mode & S_IFMT)) {\n+\t\treturn \"directory\";\n+\t} elsif (S_ISLNK($mode)) {\n+\t\treturn \"symlink\";\n+\t} elsif (S_ISREG($mode)) {\n+\t\tif ($mode & S_IXUSR) {\n+\t\t\treturn \"executable\";\n+\t\t} else {\n+\t\t\treturn \"file\";\n+\t\t};\n+\t} else {\n+\t\treturn \"unknown\";\n+\t}\n+}\n+\n+1;\n-- \n1.7.1.454.ga8c50c\n"},{"id":"143190","messageId":"AANLkTimFkRYW2favJYzWasxgC06WFd-LmrcdyaABNwOg@mail.gmail.com","threadId":"24034","inReplyTo":"1275943844-24991-4-git-send-email-pavan.sss1991@gmail.com","subject":"Re: [RFC/PATCH 4/4] gitweb: Create Gitweb::HTML::String module","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2010-06-07T20:58:31Z","receivedAt":"2010-06-07T20:58:31Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"Would it be enough If I use CGI::span instead of using the $cgi\nvariable from Gitweb::Request ?\n\nThanks,\nPavan.\n"},{"id":"143228","messageId":"201006081446.22587.jnareb@gmail.com","threadId":"24034","inReplyTo":"1275943844-24991-1-git-send-email-pavan.sss1991@gmail.com","subject":"Re: [RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-06-08T12:46:20Z","receivedAt":"2010-06-08T12:46:20Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"A few generic comments about this whole series.\n\n\nFirst, when you are sending larger patch series (and I think this series\nof 4 patches qualify), you should provide cover letter, [RFC/PATCH 0/4]\nin this case, describing the series.  git-format-patch has --cover-letter\noption to make it easier to write such cover letters.  I could then write\ngeneric comments about whole series as comments to cover letter...\n\n\nSecond, all those commits about splitting gitweb should perhaps be\nreorganized (rebased).  Currently you have 'create Gitweb::Config',\n'create Gitweb::Git', 'move subs to Gitweb::Config'; if Gitweb::Git\nwas created before Gitweb::Config, you would not need two commits\nto create Gitweb::Config (to be exact even more than two, as not all\nsubroutines are moved, as you write yourself).\n\n\nThird, and I think most important, is that the whole splitting gitweb into\nmodules series seems to alck direction, some underlying architecture\ndesign.  For example Gitweb::HTML, Gitweb::HTML::Link, Gitweb::HTML::String\nseems to me too detailed, too fine-grained modules.\n\nIt was not visible at first, because Gitweb::Config, Gitweb::Request and to\na bit lesser extent Gitweb::Git fell out naturally.  But should there be\nfor example Gitweb::Escape module, or should its functionality be a part of\nGitweb::Util?  Those issues needs to be addressed.  Perhaps they were\ndiscussed with this GSoC project mentors (via IRC, private email, IM), but\nwe don't know what is the intended architecture design of gitweb.\n\nShould we try for Model-Viewer-Controller pattern without backing MVC\n(micro)framework?  (One of design decisions for gitweb was have it working\nout of the box if Perl and git are installed, without requiring to install\nextra modules; but now we can install extra Perl modules e.g. from CPAN\nunder lib/...).  How should we organize gitweb code into packages\n(modules)?\n\nPerhaps having gitweb.perl, Gitweb::Git, Gitweb::Config, Gitweb::Request,\nGitweb::Util and Gitweb would be enough?  Should it be Gitweb::HTML or\nGitweb::View?  Etc., etc.,...\n\nThat is a very important question.\n\n\nOn Mon, 7 June 2010, Pavan Kumar Sunkara wrote:\n\n> Subject: [RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module\n>\n\nThis summary doesn't tell us much.  What subroutines, or what kind of\nsubroutines were moved to Gitweb::Config?  Why they were moved, and why\nthey weren't there from beginning (perhaps it would be better to simply\nreorder patches, to have creation of Gitweb::Git before creation of\nGitweb::Config)?  Why can they be moved now?\n\nNot all of those questions can be answered in summary (subject line for\nemail), but it should tell us what this commit is about.\n\n> Subroutines moved:\n> \tgitweb_get_feature\n> \tgitweb_check_feature\n> \tfilter_snapshot_fmts\n> \tconfigure_gitweb_features\n\nI can find which subroutines were moved from the patch itself.  What\nI cannot find without good commit message is _why_ do you feel they\nbelong in Gitweb::Config (unless it is obvious), and why they can be\nmoved now and why they could not be moved earlier.\n\n> \n> Subroutines yet to move: (Contains not yet packaged subs & vars)\n> \tfeature_bool\n> \tfeature_avatar\n> \tfeature_snapshot\n> \tfeature_patches\n\nWhy they can't be moved; what is missing?  That is the question that commit\nmessage should answer, or at least hint about / point the direction.\n\n> \n> Signed-off-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>\n> ---\n\n> diff --git a/gitweb/lib/Gitweb/Config.pm b/gitweb/lib/Gitweb/Config.pm\n> index fdab9f7..3810fda 100644\n> --- a/gitweb/lib/Gitweb/Config.pm\n> +++ b/gitweb/lib/Gitweb/Config.pm\n> @@ -10,13 +10,16 @@ use strict;\n>  use warnings;\n>  use Exporter qw(import);\n>  \n> -our @EXPORT = qw(evaluate_gitweb_config $version $projectroot $project_maxdepth $mimetypes_file\n> +our @EXPORT = qw(evaluate_gitweb_config gitweb_check_feature gitweb_get_feature configure_gitweb_features\n> +                 filter_snapshot_fmts $version $projectroot $project_maxdepth $mimetypes_file $git_avatar\n>                   $projects_list @git_base_url_list $export_ok $strict_export $home_link_str $site_name\n>                   $site_header $site_footer $home_text @stylesheets $stylesheet $logo $favicon $javascript\n>                   $GITWEB_CONFIG $GITWEB_CONFIG_SYSTEM $logo_url $logo_label $export_auth_hook\n>                   $projects_list_description_width $default_projects_order $default_blob_plain_mimetype\n>                   $default_text_plain_charset $fallback_encoding @diff_opts $prevent_xss $maxload\n> -                 %avatar_size %known_snapshot_formats %known_snapshot_format_aliases %feature);\n> +                 %avatar_size %known_snapshot_formats %feature @snapshot_fmts);\n\nI think it would be better to have exported subroutines separated from\nexported variables at least in that exported variables should begin on new\nline.  This way it would be easier to see what was added, and (sometimes)\nwhat was removed.\n\nI had to look carefully to notice that you have removed the\n%known_snapshot_format_aliases variable.  This was not mentioned in the\ncommit message.  There was no reason for removing it given in the commit\nmessage (probably it was removed because it is internal to gitweb, and is\nused only for backward compatibility with older configs... but it might be\nnot so internal, and might be useful and should be exported).\n\n> +\n> +use Gitweb::Git qw($git_dir);\n\nO.K.\n\n\n> +sub gitweb_get_feature {\n[...]\n> +\t# project specific override is possible only if we have project\n> +\tif (!$override || !defined $git_dir) {\n> +\t\treturn @defaults;\n> +\t}\n\nNote that '!defined $git_dir' is a bit of hack, to check whether we are in\ngit repository and if we can run per-repository config.  This probably\ncould be solved better... but I don't mean that you would have to solve it.\nIt can be left for later.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"143236","messageId":"AANLkTimq-46ghYT6TqXn1AB0NQIobcDaufsSJ5AEFE5z@mail.gmail.com","threadId":"24034","inReplyTo":"201006081446.22587.jnareb@gmail.com","subject":"Re: [RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-06-08T13:50:23Z","receivedAt":"2010-06-08T13:50:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"2010/6/8 Jakub Narebski <jnareb@gmail.com>:\n> A few generic comments about this whole series.\n> [...]\n> Third, and I think most important, is that the whole splitting gitweb into\n> modules series seems to alck direction, some underlying architecture\n> design.  For example Gitweb::HTML, Gitweb::HTML::Link, Gitweb::HTML::String\n> seems to me too detailed, too fine-grained modules.\n>\n> It was not visible at first, because Gitweb::Config, Gitweb::Request and to\n> a bit lesser extent Gitweb::Git fell out naturally.  But should there be\n> for example Gitweb::Escape module, or should its functionality be a part of\n> Gitweb::Util?  Those issues needs to be addressed.  Perhaps they were\n> discussed with this GSoC project mentors (via IRC, private email, IM), but\n> we don't know what is the intended architecture design of gitweb.\n>\n> Should we try for Model-Viewer-Controller pattern without backing MVC\n> (micro)framework?  (One of design decisions for gitweb was have it working\n> out of the box if Perl and git are installed, without requiring to install\n> extra modules; but now we can install extra Perl modules e.g. from CPAN\n> under lib/...).  How should we organize gitweb code into packages\n> (modules)?\n>\n> Perhaps having gitweb.perl, Gitweb::Git, Gitweb::Config, Gitweb::Request,\n> Gitweb::Util and Gitweb would be enough?  Should it be Gitweb::HTML or\n> Gitweb::View?  Etc., etc.,...\n\nI haven't contributed to Gitweb, nor do I have to deal with it. But\nI've followed this series and reviewed most of the Perl code in\nGit. Take these with a grain of salt.\n\nIt would be very useful for the future of our Perl code if we had a\ndual-life system in Git. I.e. a cpan/ directory where we could drop\nCPAN modules that should be shipped with Git.\n\nWe already do this in a less sophisticated way for Error.pm, is there\nany reason not to expand it to install more CPAN modules if they\naren't present on the system? That'd allow us to use them, but still\nonly depend on vanilla Perl.\n\nThen we could just use e.g. Config::General (~3k lines of code)\ninstead of writing our own config system. There are probably lots of\nwheels that we're inventing (and are going to invent) that have been\ndone better elsewhere, with more testing.\n\nUnlike Python or Java, Perl's policy is for core modules is to only\ninclude those required to better bootstrap the CPAN toolchain. So if\nwe continue sticking to core Perl our code is only going to drift\nfurther away from Perl best practices.\n"},{"id":"143238","messageId":"20100608141321.GP20775@machine.or.cz","threadId":"24034","inReplyTo":"201006081446.22587.jnareb@gmail.com","subject":"Re: [RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2010-06-08T14:13:21Z","receivedAt":"2010-06-08T14:13:21Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"  Hi!\n\nOn Tue, Jun 08, 2010 at 02:46:20PM +0200, Jakub Narebski wrote:\n> Third, and I think most important, is that the whole splitting gitweb into\n> modules series seems to alck direction, some underlying architecture\n> design.  For example Gitweb::HTML, Gitweb::HTML::Link, Gitweb::HTML::String\n> seems to me too detailed, too fine-grained modules.\n\n  I agree!\n\n> It was not visible at first, because Gitweb::Config, Gitweb::Request and to\n> a bit lesser extent Gitweb::Git fell out naturally.  But should there be\n> for example Gitweb::Escape module, or should its functionality be a part of\n> Gitweb::Util?  Those issues needs to be addressed.  Perhaps they were\n> discussed with this GSoC project mentors (via IRC, private email, IM), but\n> we don't know what is the intended architecture design of gitweb.\n\n  I would expect Gitweb::Escape functionality to live in Gitweb::HTML\n(HTML escaping) and/or Gitweb::Request (URL escaping).\n\n> Should we try for Model-Viewer-Controller pattern without backing MVC\n> (micro)framework?  (One of design decisions for gitweb was have it working\n> out of the box if Perl and git are installed, without requiring to install\n> extra modules; but now we can install extra Perl modules e.g. from CPAN\n> under lib/...).  How should we organize gitweb code into packages\n> (modules)?\n\n  I thought we already discussed MVC and sort of agreed that it's an\noverkill at this point. At least that is still my opinion on it; I'm not\nopposed to MVC per se, but to me, this modularization is a good\nintermediate step even if we go the MVC way later, and doing MVC properly\nwould mean much huger large-scale refactoring than just naming a module\nGitweb::View instead of Gitweb::HTML. Let's do it not at all, or\nproperly sometime later. I think it's well out-of-scope for GSoC.\n\n> Perhaps having gitweb.perl, Gitweb::Git, Gitweb::Config, Gitweb::Request,\n> Gitweb::Util and Gitweb would be enough?\n\n  I'm not sure what would fall into Gitweb::Util. I think Gitweb::HTML\nmakes a lot of sense to have, but I don't see the advantage of finer\ngraining than that - I dislike the Gitweb::HTML::* submodules as well.\n\n  Pavan, can you outline your next plan on the other modules you aim to\ncreate, plus possibly a bit of rationale?\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nThe true meaning of life is to plant a tree under whose shade\nyou will never sit.\n"},{"id":"143265","messageId":"AANLkTiksOpUqxGc7Lo4clrLwOF6GvkT7CZH5CVeirtBr@mail.gmail.com","threadId":"24034","inReplyTo":"20100608141321.GP20775@machine.or.cz","subject":"Re: [RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2010-06-08T19:22:11Z","receivedAt":"2010-06-08T19:22:11Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"On Tue, Jun 8, 2010 at 7:43 PM, Petr Baudis <pasky@suse.cz> wrote:\n>  Hi!\n>\n> On Tue, Jun 08, 2010 at 02:46:20PM +0200, Jakub Narebski wrote:\n>> Third, and I think most important, is that the whole splitting gitweb into\n>> modules series seems to alck direction, some underlying architecture\n>> design.  For example Gitweb::HTML, Gitweb::HTML::Link, Gitweb::HTML::String\n>> seems to me too detailed, too fine-grained modules.\n>\n>  I agree!\n>\n>> It was not visible at first, because Gitweb::Config, Gitweb::Request and to\n>> a bit lesser extent Gitweb::Git fell out naturally.  But should there be\n>> for example Gitweb::Escape module, or should its functionality be a part of\n>> Gitweb::Util?  Those issues needs to be addressed.  Perhaps they were\n>> discussed with this GSoC project mentors (via IRC, private email, IM), but\n>> we don't know what is the intended architecture design of gitweb.\n>\n>  I would expect Gitweb::Escape functionality to live in Gitweb::HTML\n> (HTML escaping) and/or Gitweb::Request (URL escaping).\n>\n>> Should we try for Model-Viewer-Controller pattern without backing MVC\n>> (micro)framework?  (One of design decisions for gitweb was have it working\n>> out of the box if Perl and git are installed, without requiring to install\n>> extra modules; but now we can install extra Perl modules e.g. from CPAN\n>> under lib/...).  How should we organize gitweb code into packages\n>> (modules)?\n>\n>  I thought we already discussed MVC and sort of agreed that it's an\n> overkill at this point. At least that is still my opinion on it; I'm not\n> opposed to MVC per se, but to me, this modularization is a good\n> intermediate step even if we go the MVC way later, and doing MVC properly\n> would mean much huger large-scale refactoring than just naming a module\n> Gitweb::View instead of Gitweb::HTML. Let's do it not at all, or\n> properly sometime later. I think it's well out-of-scope for GSoC.\n>\n>> Perhaps having gitweb.perl, Gitweb::Git, Gitweb::Config, Gitweb::Request,\n>> Gitweb::Util and Gitweb would be enough?\n>\n>  I'm not sure what would fall into Gitweb::Util. I think Gitweb::HTML\n> makes a lot of sense to have, but I don't see the advantage of finer\n> graining than that - I dislike the Gitweb::HTML::* submodules as well.\n>\n>  Pavan, can you outline your next plan on the other modules you aim to\n> create, plus possibly a bit of rationale?\n\nI am graining Gitweb::HTML into Gitweb::HTML::* to reduce circular\ndependancies of the modules.\n\nIn the following days, I will be creating more modules\n  Gitweb::HTML::Navigation\n  Gitweb::HTML::Error\n  Gitweb::HTML::Page (Containing header and footer subs)\n  Gitweb::HTML::Format (format_* subs)\n  Gitweb::Parse\n  Gitweb::RepoConfig\n  Gitweb::Util\n  Gitweb::Action::* (All action subs like git_blame, git_log)\n\nThis is my architectural design for now. But It may change due to\nlater circular dependancies.\n\nI will be rebasing the whole series, edit them and send them once\nevery module has undergone as RFC in the mailing list.\n\nI have been stuck many times trying to workaround the circular module\ndependancies and believe me, the patches I am sending and the modules\nI am creating involves a lot of effort from my side and as long as you\nthink there's nothing wrong with the grouping of subroutines in my\nmodules and their names, you need not worry about the module\nstructure.\n\nThanks,\nPavan.\n"},{"id":"143267","messageId":"20100608195552.GA3408@machine.or.cz","threadId":"24034","inReplyTo":"AANLkTiksOpUqxGc7Lo4clrLwOF6GvkT7CZH5CVeirtBr@mail.gmail.com","subject":"Re: [RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2010-06-08T19:55:52Z","receivedAt":"2010-06-08T19:55:52Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Wed, Jun 09, 2010 at 12:52:11AM +0530, Pavan Kumar Sunkara wrote:\n> I am graining Gitweb::HTML into Gitweb::HTML::* to reduce circular\n> dependancies of the modules.\n\nI'm sorry, I don't understand. How is splitting up Gitweb::HTML to\nsubmodules helping to reduce circular dependencies? I don't quite see\nthat right now. :-( Can you give a concrete example? Perhaps it would be\nbetter to refactor the few problematic users instead of convoluting the\nwhole module structure because of the offenders.\n\n>   Gitweb::Parse\n\nWhat will this module do?\n\n>   Gitweb::Util\n\nWhat will this module do?\n\n>   Gitweb::Action::* (All action subs like git_blame, git_log)\n\nDo we need to do this right now? I think moving huge chunks of the code\naround like this right now is unneccessary and it might just enlarge the\npatch queue and delay you in your main GSoC efforts; perhaps we could do\nthis later when the dust settles a bit and we are sure that the rest of\nthe modular structure we have introduced fits well?\n\n> I will be rebasing the whole series, edit them and send them once\n> every module has undergone as RFC in the mailing list.\n\nOk! I have meant to ask about that.\n\n> I have been stuck many times trying to workaround the circular module\n> dependancies and believe me, the patches I am sending and the modules\n> I am creating involves a lot of effort from my side and as long as you\n> think there's nothing wrong with the grouping of subroutines in my\n> modules and their names, you need not worry about the module\n> structure.\n\nI'm sorry but it doesn't work like that - if you have put a lot of\neffort behind it, you need to present us with the rationale you have\nreached and convince us that this is the right way to take.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nThe true meaning of life is to plant a tree under whose shade\nyou will never sit.\n"},{"id":"143271","messageId":"AANLkTimKsdn8Vww_4U4YQDPlpr_BgbVszwG64lEYl-cE@mail.gmail.com","threadId":"24034","inReplyTo":"20100608195552.GA3408@machine.or.cz","subject":"Re: [RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module","fromName":"Pavan Kumar Sunkara","fromEmail":"pavan.sss1991@gmail.com","sentAt":"2010-06-08T20:24:34Z","receivedAt":"2010-06-08T20:24:34Z","isPatch":true,"sender":{"key":"pavan.sss1991@gmail.com","avatar":"https://avatars.githubusercontent.com/u/174703?v=4"},"body":"On Wed, Jun 9, 2010 at 1:25 AM, Petr Baudis <pasky@suse.cz> wrote:\n> On Wed, Jun 09, 2010 at 12:52:11AM +0530, Pavan Kumar Sunkara wrote:\n>> I am graining Gitweb::HTML into Gitweb::HTML::* to reduce circular\n>> dependancies of the modules.\n>\n> I'm sorry, I don't understand. How is splitting up Gitweb::HTML to\n> submodules helping to reduce circular dependencies? I don't quite see\n> that right now. :-( Can you give a concrete example? Perhaps it would be\n> better to refactor the few problematic users instead of convoluting the\n> whole module structure because of the offenders.\n\nOk. I will be combining them into a single module.\n\n>>   Gitweb::Parse\n>\n> What will this module do?\n\nThis module contains all the parse_* subroutines\n\nGitweb::Format contains all the format_* subroutines\n\n>\n>>   Gitweb::Util\n>\n> What will this module do?\n\nThis modules contains all the git utility functions.\n\n>>   Gitweb::Action::* (All action subs like git_blame, git_log)\n>\n> Do we need to do this right now? I think moving huge chunks of the code\n> around like this right now is unneccessary and it might just enlarge the\n> patch queue and delay you in your main GSoC efforts; perhaps we could do\n> this later when the dust settles a bit and we are sure that the rest of\n> the modular structure we have introduced fits well?\n\nI still have until this week in the timeline. Don't I ?\nI strongly hope that I will be able to finalise the patch queue by\nthis week and will move on to develop write functionalities.\n\nThanks,\nPavan.\n"},{"id":"143277","messageId":"20100608205029.GB3408@machine.or.cz","threadId":"24034","inReplyTo":"AANLkTimKsdn8Vww_4U4YQDPlpr_BgbVszwG64lEYl-cE@mail.gmail.com","subject":"Re: [RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2010-06-08T20:50:29Z","receivedAt":"2010-06-08T20:50:29Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Wed, Jun 09, 2010 at 01:54:34AM +0530, Pavan Kumar Sunkara wrote:\n> On Wed, Jun 9, 2010 at 1:25 AM, Petr Baudis <pasky@suse.cz> wrote:\n> > On Wed, Jun 09, 2010 at 12:52:11AM +0530, Pavan Kumar Sunkara wrote:\n> >>   Gitweb::Parse\n> >\n> > What will this module do?\n> \n> This module contains all the parse_* subroutines\n\nOk, that makes sense. It might be also possible to have them in\nGitweb::Git, but I see href() invocations and such that would probably\ncreate layering violations.\n\n> Gitweb::Format contains all the format_* subroutines\n\nHere, I'm less decided. I would have put these in Gitweb::HTML, but I\nhave no hard opinion, maybe that's clumping things too much - so no nack\nfrom me personally.\n\n> >>   Gitweb::Util\n> >\n> > What will this module do?\n> \n> This modules contains all the git utility functions.\n\nCan you give an example, please?\n\n> I still have until this week in the timeline. Don't I ?\n> I strongly hope that I will be able to finalise the patch queue by\n> this week and will move on to develop write functionalities.\n\nSure, my only concern is that if the queue of patches your future work\nwill depend on gets too long and gets delayed too much in merging in,\nit will get much more difficult to produce further patches, get them\nreviewed and get them on the merging track.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nThe true meaning of life is to plant a tree under whose shade\nyou will never sit.\n"},{"id":"143297","messageId":"201006090138.52125.jnareb@gmail.com","threadId":"24034","inReplyTo":"AANLkTiksOpUqxGc7Lo4clrLwOF6GvkT7CZH5CVeirtBr@mail.gmail.com","subject":"Re: [RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-06-08T23:38:50Z","receivedAt":"2010-06-08T23:38:50Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 8 Jun 2010, Petr Baudis wrote:\n> On Tue, Jun 08, 2010 at 02:46:20PM +0200, Jakub Narebski wrote:\n> >\n> > Third, and I think most important, is that the whole splitting gitweb into\n> > modules series seems to alck direction, some underlying architecture\n> > design.  For example Gitweb::HTML, Gitweb::HTML::Link, Gitweb::HTML::String\n> > seems to me too detailed, too fine-grained modules.\n> \n>   I agree!\n> \n> > It was not visible at first, because Gitweb::Config, Gitweb::Request and to\n> > a bit lesser extent Gitweb::Git fell out naturally.  But should there be\n> > for example Gitweb::Escape module, or should its functionality be a part of\n> > Gitweb::Util?  Those issues needs to be addressed.  Perhaps they were\n> > discussed with this GSoC project mentors (via IRC, private email, IM), but\n> > we don't know what is the intended architecture design of gitweb.\n> \n>   I would expect Gitweb::Escape functionality to live in Gitweb::HTML\n> (HTML escaping) and/or Gitweb::Request (URL escaping).\n\nI agree... well, partially, depending if we allow Gitweb::Request to be\nonly about request i.e. inbound links, or also with generating outbound\nlinks.  Thos two are tied together, e.g. with %cgi_param_mapping.\n\n>From what Pavan says, it seems that the problem with splitting gitweb is\ninterdependency of various commands, and overdepenency on global variables.\nThat was caused by the fact that gitweb was (well, is currently) big large\nmonolithic script, and there were no mechanism protecting against adding\n(inter)dependencies.\n\nPerhaps instead of fine-splitting gitweb into tiny modules to avoid\ncircular dependencies (module A needs module B because of variable a,\nmodule B needs module A because of variable b) we should fix overdependence\non global variables by passing more as parameters - at least where it makes\nsense.\n\n> > Should we try for Model-Viewer-Controller pattern without backing MVC\n> > (micro)framework?  (One of design decisions for gitweb was have it working\n> > out of the box if Perl and git are installed, without requiring to install\n> > extra modules; but now we can install extra Perl modules e.g. from CPAN\n> > under lib/...).  How should we organize gitweb code into packages\n> > (modules)?\n> \n>   I thought we already discussed MVC and sort of agreed that it's an\n> overkill at this point. At least that is still my opinion on it; I'm not\n> opposed to MVC per se, but to me, this modularization is a good\n> intermediate step even if we go the MVC way later, and doing MVC properly\n> would mean much huger large-scale refactoring than just naming a module\n> Gitweb::View instead of Gitweb::HTML. Let's do it not at all, or\n> properly sometime later. I think it's well out-of-scope for GSoC.\n\nWell, let's not forget that the whole reason behind including splitting\ngitweb into project that is about adding write capabilities to gitweb,\nmaking a web interface equivalent to git-gui.  Splitting gitweb was meant\nto make it easier to add new functionality without having gitweb become too\nlarge, and maintenance nightmare.\n\nSo it would be enough to have Gitweb with core of gitweb, gitweb.perl\ntop-level script, Gitweb::Write or something like that for new write\nfunctionality and Gitweb::Util containing things that are needed by Gitweb\nand by Gitweb::Write(r).\n\n> \n> > Perhaps having gitweb.perl, Gitweb::Git, Gitweb::Config, Gitweb::Request,\n> > Gitweb::Util and Gitweb would be enough?\n> \n>   I'm not sure what would fall into Gitweb::Util. I think Gitweb::HTML\n> makes a lot of sense to have, but I don't see the advantage of finer\n> graining than that - I dislike the Gitweb::HTML::* submodules as well.\n\nGitweb does its work in the following way: first it parses path_info,\nquery parameters etc. and saves them in global variables.  This is in\nGitweb::Request.  But to parse-out project name from pathinfo it needs\n$projectsroot from Gitweb::Config.  Then it runs appropriate git commands\nusing subroutines from Gitweb::Cmd / Gitweb::Git, setting $git_dir first.\nThen it parses output of git commands (probably Gitweb::Git too).\nFinally it composes a response, usually HTML, but also feed (OPML, RSS,\nAtom), plain text, and other output (blob_plain, snapshot); probably\nGitweb::Response, or Gitweb::Output, or Gitweb::View.\n\nThere are of course some problems.\n\nTo properly extract project from path_info in Gitweb::Request one needs\n$projectroot from Gitweb::Config.  Also generated gitweb links,\ni.e. href() and esc_param() etc., are tied with parsing request URL, at\nleast via %cgi_param_mapping.\n\nGitweb::Config isn't that simple either.  There is git configuration, e.g\n$GIT, there is gitweb configuration, e.g. $projectroot, and there is\nper-project configuration or configuration override.\n\nGitweb::Git / Gitweb::Cmd / Gitweb::Git::Cmd needs $GIT, but also\n$git_dir, which is set using $projectroot and $project from\nGitweb::Request.  Should it include parsing output of git commands, and\nauxiliary commands dealing directly with files in git repository?\n\nThen there is dispatch and generating response.  It uses many utility\nfunctions, like chop_str, or die_error.  Then there aresubroutines\nresponsible for actions / views.\n\n\nFnally there is a question which parts of those the futture write support\nwould need...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"143332","messageId":"AANLkTimXSfAqMYVIQcepibeDlJPOKE4hTlys9laEFTHo@mail.gmail.com","threadId":"24034","inReplyTo":"201006090138.52125.jnareb@gmail.com","subject":"Re: [RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-06-09T13:13:34Z","receivedAt":"2010-06-09T13:13:34Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, Jun 9, 2010 at 1:38 AM, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Tue, 8 Jun 2010, Petr Baudis wrote:\n>>\n>>   I thought we already discussed MVC and sort of agreed that it's an\n>> overkill at this point. At least that is still my opinion on it; I'm not\n>> opposed to MVC per se, but to me, this modularization is a good\n>> intermediate step even if we go the MVC way later, and doing MVC properly\n>> would mean much huger large-scale refactoring than just naming a module\n>> Gitweb::View instead of Gitweb::HTML. Let's do it not at all, or\n>> properly sometime later. I think it's well out-of-scope for GSoC.\n>\n> So it would be enough to have Gitweb with core of gitweb, gitweb.perl\n> top-level script, Gitweb::Write or something like that for new write\n> functionality and Gitweb::Util containing things that are needed by Gitweb\n> and by Gitweb::Write(r).\n\nSidenote (something I meant to write, but forgot): SVN::Web[1][2] and\nGitalist[3][4] might or might not be good example on how to split gitweb\ninto modules.\n\n[1] http://p3rl.org/SVN::Web\n    http://search.cpan.org/dist/SVN-Web/\n[2] http://jc.ngo.org.uk/svnweb/jc/browse/nik/CPAN/SVN-Web/trunk/\n\n[3] http://p3rl.org/Gitalist\n    http://search.cpan.org/dist/Gitalist/\n[4] http://github.com/broquaint/Gitalist\n-- \nJakub Narebski\n"},{"id":"143541","messageId":"201006120301.37931.jnareb@gmail.com","threadId":"24034","inReplyTo":"AANLkTimq-46ghYT6TqXn1AB0NQIobcDaufsSJ5AEFE5z@mail.gmail.com","subject":"Re: [RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-06-12T01:01:34Z","receivedAt":"2010-06-12T01:01:34Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 8 June 2010, Ævar Arnfjörð Bjarmason wrote:\n\n> I haven't contributed to Gitweb, nor do I have to deal with it. But\n> I've followed this series and reviewed most of the Perl code in\n> Git. Take these with a grain of salt.\n> \n> It would be very useful for the future of our Perl code if we had a\n> dual-life system in Git. I.e. a cpan/ directory where we could drop\n> CPAN modules that should be shipped with Git.\n\nThe standard name for such directory is 'inc/', I think.\n\nThere is for example 'inc::latest' module (unfortunately in core only\nsince latest Perl, i.e. version 5.12), which uses modules bundled in\n'inc/' if they are newer than installed ones.  It is in Module-Build\ndistribution.\n\nBTW. it's a pity that PAR (Perl Archiving Toolkit, par.perl.org)\nis not in core...\n\n> \n> We already do this in a less sophisticated way for Error.pm, is there\n> any reason not to expand it to install more CPAN modules if they\n> aren't present on the system? That'd allow us to use them, but still\n> only depend on vanilla Perl.\n\nSidenote about Error.pm: from what I understand modern consensus is\nthat Error.pm was a failed approach, and currently recommended way to\nuse exceptions in Perl is via block form of eval, e.g. Try::Tiny or\nTryCatch (this requires Moose and PPI), and not Error.\n\nThe Error documentation nowadays includes the following instructions:\n\n  WARNING\n\n  Using the \"Error\" module is no longer recommended due to the\n  black-magical nature of its syntactic sugar, which often tends to\n  break. Its maintainers have stopped actively writing code that uses\n  it, and discourage people from doing so. See the \"SEE ALSO\" section\n  below for better recommendations.\n\n  [...]\n\n  SEE ALSO\n\n  See Exception::Class for a different module providing Object-Oriented\n  exception handling, along with a convenient syntax for declaring\n  hierarchies for them. It doesn't provide Error's syntactic sugar of\n  'try { ... }, catch { ... }', etc. which may be a good thing or a bad\n  thing based on what you want. (Because Error's syntactic sugar tends\n  to break.)\n\n  Error::Exception aims to combine Error and Exception::Class \"with\n  correct stringification\".\n\n  TryCatch and Try::Tiny are similar in concept to Error.pm only\n  providing a syntax that hopefully breaks less.\n\n> \n> Then we could just use e.g. Config::General (~3k lines of code)\n> instead of writing our own config system. There are probably lots of\n> wheels that we're inventing (and are going to invent) that have been\n> done better elsewhere, with more testing.\n\nThe problem with _optional_ Config::General config is that people\nwould have incompatibile gitweb config files, some using Config::General\nsyntax, some current configuration in Perl.\n \n> Unlike Python or Java, Perl's policy is for core modules is to only\n> include those required to better bootstrap the CPAN toolchain. So if\n> we continue sticking to core Perl our code is only going to drift\n> further away from Perl best practices.\n\nRight.\n\nStill, we don't want to require having half of CPAN in 'inc/' to\ninstall gitweb.  Perhaps \"no non-core dependencies\" is too strict,\nand should be replaced by \"minimal non-core depencencies\".\n\n-- \nJakub Narebski\nPoland\n"},{"id":"143542","messageId":"AANLkTimfzjZ00ua23FDmUJLil-_OEPEkiS73syVCQ52f@mail.gmail.com","threadId":"24034","inReplyTo":"201006120301.37931.jnareb@gmail.com","subject":"Re: [RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-06-12T01:22:53Z","receivedAt":"2010-06-12T01:22:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sat, Jun 12, 2010 at 01:01, Jakub Narebski <jnareb@gmail.com> wrote:\n> On Tue, 8 June 2010, Ævar Arnfjörð Bjarmason wrote:\n>\n>> I haven't contributed to Gitweb, nor do I have to deal with it. But\n>> I've followed this series and reviewed most of the Perl code in\n>> Git. Take these with a grain of salt.\n>>\n>> It would be very useful for the future of our Perl code if we had a\n>> dual-life system in Git. I.e. a cpan/ directory where we could drop\n>> CPAN modules that should be shipped with Git.\n>\n> The standard name for such directory is 'inc/', I think.\n\nPerl itself uses cpan/, but Module::Install started the inc/. What we\ncall it really doesn't matter though.\n\n> There is for example 'inc::latest' module (unfortunately in core only\n> since latest Perl, i.e. version 5.12), which uses modules bundled in\n> 'inc/' if they are newer than installed ones.  It is in Module-Build\n> distribution.\n\nInteresting. I hadn't looked at it.\n\n> BTW. it's a pity that PAR (Perl Archiving Toolkit, par.perl.org)\n> is not in core...\n\nIt has a bunch of off edge cases unlike its namesake .jar, but in any\ncase using it wouldn't help us.\n\n>> We already do this in a less sophisticated way for Error.pm, is there\n>> any reason not to expand it to install more CPAN modules if they\n>> aren't present on the system? That'd allow us to use them, but still\n>> only depend on vanilla Perl.\n>\n> Sidenote about Error.pm: from what I understand modern consensus is\n> that Error.pm was a failed approach, and currently recommended way to\n> use exceptions in Perl is via block form of eval, e.g. Try::Tiny or\n> TryCatch (this requires Moose and PPI), and not Error.\n\nYeah, but it was just an example of how we can (and should) bundle\nmodules with Git.\n\n>> Then we could just use e.g. Config::General (~3k lines of code)\n>> instead of writing our own config system. There are probably lots of\n>> wheels that we're inventing (and are going to invent) that have been\n>> done better elsewhere, with more testing.\n>\n> The problem with _optional_ Config::General config is that people\n> would have incompatibile gitweb config files, some using Config::General\n> syntax, some current configuration in Perl.\n\nIsn't the current patch series the first attempt at config file\nsupport? I.e. it's always been editing the source until now.\n\nIn any case, proper non-executable config file support could easily be\nmade optional, hopefully with a transition the non-executable one.\n\n>> Unlike Python or Java, Perl's policy is for core modules is to only\n>> include those required to better bootstrap the CPAN toolchain. So if\n>> we continue sticking to core Perl our code is only going to drift\n>> further away from Perl best practices.\n>\n> Right.\n>\n> Still, we don't want to require having half of CPAN in 'inc/' to\n> install gitweb.  Perhaps \"no non-core dependencies\" is too strict,\n> and should be replaced by \"minimal non-core depencencies\".\n\nObviously we'd pick modules to include carefully.\n"},{"id":"143544","messageId":"201006120341.39843.jnareb@gmail.com","threadId":"24034","inReplyTo":"AANLkTimfzjZ00ua23FDmUJLil-_OEPEkiS73syVCQ52f@mail.gmail.com","subject":"Re: [RFC/PATCH 1/4] gitweb: Move subroutines to Gitweb::Config module","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-06-12T01:41:38Z","receivedAt":"2010-06-12T01:41:38Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 12 Jun 2010, Ævar Arnfjörð Bjarmason wrote:\n> On Sat, Jun 12, 2010 at 01:01, Jakub Narebski <jnareb@gmail.com> wrote:\n>> On Tue, 8 June 2010, Ævar Arnfjörð Bjarmason wrote:\n>>\n>>> I haven't contributed to Gitweb, nor do I have to deal with it. But\n>>> I've followed this series and reviewed most of the Perl code in\n>>> Git. Take these with a grain of salt.\n>>>\n>>> It would be very useful for the future of our Perl code if we had a\n>>> dual-life system in Git. I.e. a cpan/ directory where we could drop\n>>> CPAN modules that should be shipped with Git.\n>>\n>> The standard name for such directory is 'inc/', I think.\n> \n> Perl itself uses cpan/, but Module::Install started the inc/. What we\n> call it really doesn't matter though.\n\nRight.\n\nAlthough better example would be what modules on CPAN use, rather than\nwhat Perl itself uses.\n\n>>> Then we could just use e.g. Config::General (~3k lines of code)\n>>> instead of writing our own config system. There are probably lots of\n>>> wheels that we're inventing (and are going to invent) that have been\n>>> done better elsewhere, with more testing.\n>>\n>> The problem with _optional_ Config::General config is that people\n>> would have incompatibile gitweb config files, some using Config::General\n>> syntax, some current configuration in Perl.\n> \n> Isn't the current patch series the first attempt at config file\n> support? I.e. it's always been editing the source until now.\n\nErrr... no!\n\n$ git blame -C -C -w -L/^our.*'GITWEB_CONFIG'/,+12 gitweb/gitweb.perl\nshows that current config file in Perl (loaded using 'do $file') is\nwith gitweb since at least 2006-08-02.\n \n> In any case, proper non-executable config file support could easily be\n> made optional, hopefully with a transition the non-executable one.\n\nI'm not sure if it would be easy to translate currently used gitweb\nconfig files to non-executable file format.  I think that %feature\nhash make it so such config format would have to support nested \nstructures, which means e.g. JSON or YAML.\n\nBesides how would you store / define $export_auth_hook in non-executable\nfile format?\n\n  # show repository only if this subroutine returns true\n  # when given the path to the project, for example:\n  #    sub { return -e \"$_[0]/git-daemon-export-ok\"; }\n  our $export_auth_hook = undef;\n\n-- \nJakub Narebski\nPoland\n"}]}