Re: About [PATCH 2/2] gitweb: Create a perl module to handle gitweb cgi params and vars

Shouldn't evaluate_query_params(), evaluate_path_info(), and the
subroutine that ties them together evaluate_and_validate_params() be
in Gitweb::Request too?

quoted

 sub evaluate_git_dir {

[....]

quoted

 }

Ditto with evaluate_git_dir()?

Well, evaluate_and_validate_params() and evaluate_path_info() contains
calls to subroutines which are not yet moved into any package. So,
what do you want to in such a case ?

BTW. as I said in comment to previous patch, vriables such as $project
should be put in Gitweb::Request in my opinion, not in Gitweb::Config.
Perhaps they are, but it is not obvious from this patch.

quoted

 our (@snapshot_fmts, $git_avatar);

@@ -643,32 +605,32 @@ set_message(\&handle_errors_html);

 # dispatch
 sub dispatch {

[...]

quoted

 }

 sub run_request {

Those two subroutines should not, I guess, be put in Gitweb::Request.
They would be in catch-all Gitweb module, I guess, or perhaps in the
later post-GSoC future in Gitweb::Dispatch or something.

Yes.

quoted

@@ -689,7 +651,6 @@ sub run_request {

 our $is_last_request = sub { 1 };
 our ($pre_dispatch_hook, $post_dispatch_hook, $pre_listen_hook);
 our $CGI = 'CGI';
-our $cgi;
 sub evaluate_argv {
      return unless (@ARGV);

Ah, so $CGI is left in gitweb.perl, $cgi is moved to Gitweb::Request.

[cut more than half of patch, which sould not be needed with exporting
 variables and not using fully qualified variable names]

quoted

diff --git a/gitweb/lib/Gitweb/Request.pm b/gitweb/lib/Gitweb/Request.pm
new file mode 100644
index 0000000..91cc492
--- /dev/null
+++ b/gitweb/lib/Gitweb/Request.pm

@@ -0,0 +1,58 @@
+#!/usr/bin/perl
+#
+# Gitweb::Request -- gitweb request(cgi) package
+#
+# This program is licensed under the GPLv2

I don't remember what is git policy on copyright statements, and on
GPLv2 vs GPLv2+...

quoted

+
+package Gitweb::Request;
+
+use strict;
+
+BEGIN {
+     use Exporter();
+
+     @Gitweb::Request::ISA = qw(Exporter);
+     @Gitweb::Request::EXPORT = qw();
+}

Same comment as for previous patch.

  use Exporter qw(import);
  our @EXPORT = qw($cgi, $my_url, $my_uri, $base_url, ...);

BTW with exporting variables you can easily see here that
Gitweb::Request does not use any variable from Gitweb::Config.

quoted

+
+our ($cgi, $my_url, $my_uri, $base_url, $path_info, $home_link);
+our ($action, $project, $file_name, $file_parent, $hash, $hash_parent, $hash_base,
+     $hash_parent_base, @extra_options, $page, $git_dir);
+our ($searchtype, $search_use_regexp, $searchtext, $search_regexp);
+
+sub evaluate_uri {

[...]

quoted

+}

Straightforward code movement.  But see comment above on which
subroutines I feel should be also put in Gitweb::Request.

Please answer the question regarding calls to not-yet-packaged subroutines.

Thanks,
Pavan.