{"thread":{"id":"21501","subject":"[PATCH] gitweb: Refactor project list routines","startedAt":"2009-11-06T15:10:46Z","lastAt":"2009-11-16T17:56:30Z","messageCount":3,"participants":["Petr Baudis","J.H."],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"126982","messageId":"1257520246-6548-1-git-send-email-pasky@suse.cz","threadId":"21501","inReplyTo":null,"subject":"[PATCH] gitweb: Refactor project list routines","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2009-11-06T15:10:46Z","receivedAt":"2009-11-06T15:10:46Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"This is a preparatory patch for separation of project list and frontpage\nactions; it factors out most logic of git_project_list():\n\n\t* git_project_list_all() as a git_project_list_body() wrapper for\n\t  complete project listing\n\t* git_project_search_form() for printing the project search form\n\nAlso, git_project_list_ctags() is now separated out of\ngit_project_list_body(), showing tag cloud for given project list.\n\nSigned-off-by: Petr Baudis <pasky@suse.cz>\n\n---\n gitweb/gitweb.perl |   69 +++++++++++++++++++++++++++++++---------------------\n 1 files changed, 41 insertions(+), 28 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex e4cbfc3..e82ca45 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -4201,10 +4201,9 @@ sub git_patchset_body {\n # project in the list, removing invalid projects from returned list\n # NOTE: modifies $projlist, but does not remove entries from it\n sub fill_project_list_info {\n-\tmy ($projlist, $check_forks) = @_;\n+\tmy ($projlist, $check_forks, $show_ctags) = @_;\n \tmy @projects;\n \n-\tmy $show_ctags = gitweb_check_feature('ctags');\n  PROJECT:\n \tforeach my $pr (@$projlist) {\n \t\tmy (@activity) = git_get_last_activity($pr->{'path'});\n@@ -4254,12 +4253,26 @@ sub print_sort_th {\n \t}\n }\n \n+sub git_project_list_ctags {\n+\tmy ($projects) = @_;\n+\n+\tmy %ctags;\n+\tforeach my $p (@$projects) {\n+\t\tforeach my $ct (keys %{$p->{'ctags'}}) {\n+\t\t\t$ctags{$ct} += $p->{'ctags'}->{$ct};\n+\t\t}\n+\t}\n+\tmy $cloud = git_populate_project_tagcloud(\\%ctags);\n+\tprint git_show_project_tagcloud($cloud, 64);\n+}\n+\n sub git_project_list_body {\n \t# actually uses global variable $project\n \tmy ($projlist, $order, $from, $to, $extra, $no_header) = @_;\n \n \tmy $check_forks = gitweb_check_feature('forks');\n-\tmy @projects = fill_project_list_info($projlist, $check_forks);\n+\tmy $show_ctags = gitweb_check_feature('ctags');\n+\tmy @projects = fill_project_list_info($projlist, $check_forks, $show_ctags);\n \n \t$order ||= $default_projects_order;\n \t$from = 0 unless defined $from;\n@@ -4278,16 +4291,8 @@ sub git_project_list_body {\n \t\t@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;\n \t}\n \n-\tmy $show_ctags = gitweb_check_feature('ctags');\n \tif ($show_ctags) {\n-\t\tmy %ctags;\n-\t\tforeach my $p (@projects) {\n-\t\t\tforeach my $ct (keys %{$p->{'ctags'}}) {\n-\t\t\t\t$ctags{$ct} += $p->{'ctags'}->{$ct};\n-\t\t\t}\n-\t\t}\n-\t\tmy $cloud = git_populate_project_tagcloud(\\%ctags);\n-\t\tprint git_show_project_tagcloud($cloud, 64);\n+\t\tgit_project_list_ctags(\\@projects);\n \t}\n \n \tprint \"<table class=\\\"project_list\\\">\\n\";\n@@ -4361,6 +4366,28 @@ sub git_project_list_body {\n \tprint \"</table>\\n\";\n }\n \n+sub git_project_search_form {\n+\tprint $cgi->startform(-method => \"get\") .\n+\t      \"<p class=\\\"projsearch\\\">Search:\\n\" .\n+\t      $cgi->textfield(-name => \"s\", -value => $searchtext) . \"\\n\" .\n+\t      \"</p>\" .\n+\t      $cgi->end_form() . \"\\n\";\n+}\n+\n+sub git_project_list_all {\n+\tmy $order = $input_params{'order'};\n+\tif (defined $order && $order !~ m/none|project|descr|owner|age/) {\n+\t\tdie_error(400, \"Unknown order parameter\");\n+\t}\n+\n+\tmy @list = git_get_projects_list();\n+\tif (!@list) {\n+\t\tdie_error(404, \"No projects found\");\n+\t}\n+\n+\tgit_project_list_body(\\@list, $order);\n+}\n+\n sub git_shortlog_body {\n \t# uses global variable $project\n \tmy ($commitlist, $from, $to, $refs, $extra) = @_;\n@@ -4630,28 +4657,14 @@ sub git_search_grep_body {\n ## actions\n \n sub git_project_list {\n-\tmy $order = $input_params{'order'};\n-\tif (defined $order && $order !~ m/none|project|descr|owner|age/) {\n-\t\tdie_error(400, \"Unknown order parameter\");\n-\t}\n-\n-\tmy @list = git_get_projects_list();\n-\tif (!@list) {\n-\t\tdie_error(404, \"No projects found\");\n-\t}\n-\n \tgit_header_html();\n \tif (-f $home_text) {\n \t\tprint \"<div class=\\\"index_include\\\">\\n\";\n \t\tinsert_file($home_text);\n \t\tprint \"</div>\\n\";\n \t}\n-\tprint $cgi->startform(-method => \"get\") .\n-\t      \"<p class=\\\"projsearch\\\">Search:\\n\" .\n-\t      $cgi->textfield(-name => \"s\", -value => $searchtext) . \"\\n\" .\n-\t      \"</p>\" .\n-\t      $cgi->end_form() . \"\\n\";\n-\tgit_project_list_body(\\@list, $order);\n+\tgit_project_search_form();\n+\tgit_project_list_all();\n \tgit_footer_html();\n }\n \n-- \ntg: (8cc62c1..) t/frontpage/refactor (depends on: vanilla/master)\n"},{"id":"127002","messageId":"4AF47142.4010609@eaglescrag.net","threadId":"21501","inReplyTo":"1257520246-6548-1-git-send-email-pasky@suse.cz","subject":"Re: [PATCH] gitweb: Refactor project list routines","fromName":"J.H.","fromEmail":"warthog19@eaglescrag.net","sentAt":"2009-11-06T18:56:02Z","receivedAt":"2009-11-06T18:56:02Z","isPatch":true,"sender":{"key":"warthog19@eaglescrag.net","avatar":null},"body":"Petr,\n\nAfter digging into some of the ctags stuff recently looking at it, I \nhave some serious concerns over making it a more primary interface \ninside of Gitweb.\n\n1) The mechanism to assign ctags and it's associated documentation is \ncryptic at best.  It took me reverse engineering the code to figure out \nhow to add tags to a repository, and even then there are very simple \nmeans of trivially breaking them (Like put 0 inside of a ctag file, the \ndivide by zero errors are kinda concerning for instance).\n\n2) If the repository is cloned the ctag information is not retained, \nwhich means there is no real way for the original developer to manage or \nmove this information between different hosting sites, I.E. repo.or.cz \nand kernel.org (though I'll admit I have it turned off)\n\nSo if your going to eliminate the project listing, with the general \nintention of using the tag cloud as more of a primary search mechanism, \nincluding the search box, I think there's some serious work that needs \nto be put into the ctags system because in it's current state, for the \nlikes of kernel.org, it's unusable, unstable and not something I would \nrecommend to anyone to run in production.  I like the idea, I just have \nconcerns over it's current implementation.\n\n- John 'Warthog9' Hawley\n\nPetr Baudis wrote:\n> This is a preparatory patch for separation of project list and frontpage\n> actions; it factors out most logic of git_project_list():\n> \n> \t* git_project_list_all() as a git_project_list_body() wrapper for\n> \t  complete project listing\n> \t* git_project_search_form() for printing the project search form\n> \n> Also, git_project_list_ctags() is now separated out of\n> git_project_list_body(), showing tag cloud for given project list.\n> \n> Signed-off-by: Petr Baudis <pasky@suse.cz>\n> \n> ---\n>  gitweb/gitweb.perl |   69 +++++++++++++++++++++++++++++++---------------------\n>  1 files changed, 41 insertions(+), 28 deletions(-)\n> \n> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n> index e4cbfc3..e82ca45 100755\n> --- a/gitweb/gitweb.perl\n> +++ b/gitweb/gitweb.perl\n> @@ -4201,10 +4201,9 @@ sub git_patchset_body {\n>  # project in the list, removing invalid projects from returned list\n>  # NOTE: modifies $projlist, but does not remove entries from it\n>  sub fill_project_list_info {\n> -\tmy ($projlist, $check_forks) = @_;\n> +\tmy ($projlist, $check_forks, $show_ctags) = @_;\n>  \tmy @projects;\n>  \n> -\tmy $show_ctags = gitweb_check_feature('ctags');\n>   PROJECT:\n>  \tforeach my $pr (@$projlist) {\n>  \t\tmy (@activity) = git_get_last_activity($pr->{'path'});\n> @@ -4254,12 +4253,26 @@ sub print_sort_th {\n>  \t}\n>  }\n>  \n> +sub git_project_list_ctags {\n> +\tmy ($projects) = @_;\n> +\n> +\tmy %ctags;\n> +\tforeach my $p (@$projects) {\n> +\t\tforeach my $ct (keys %{$p->{'ctags'}}) {\n> +\t\t\t$ctags{$ct} += $p->{'ctags'}->{$ct};\n> +\t\t}\n> +\t}\n> +\tmy $cloud = git_populate_project_tagcloud(\\%ctags);\n> +\tprint git_show_project_tagcloud($cloud, 64);\n> +}\n> +\n>  sub git_project_list_body {\n>  \t# actually uses global variable $project\n>  \tmy ($projlist, $order, $from, $to, $extra, $no_header) = @_;\n>  \n>  \tmy $check_forks = gitweb_check_feature('forks');\n> -\tmy @projects = fill_project_list_info($projlist, $check_forks);\n> +\tmy $show_ctags = gitweb_check_feature('ctags');\n> +\tmy @projects = fill_project_list_info($projlist, $check_forks, $show_ctags);\n>  \n>  \t$order ||= $default_projects_order;\n>  \t$from = 0 unless defined $from;\n> @@ -4278,16 +4291,8 @@ sub git_project_list_body {\n>  \t\t@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;\n>  \t}\n>  \n> -\tmy $show_ctags = gitweb_check_feature('ctags');\n>  \tif ($show_ctags) {\n> -\t\tmy %ctags;\n> -\t\tforeach my $p (@projects) {\n> -\t\t\tforeach my $ct (keys %{$p->{'ctags'}}) {\n> -\t\t\t\t$ctags{$ct} += $p->{'ctags'}->{$ct};\n> -\t\t\t}\n> -\t\t}\n> -\t\tmy $cloud = git_populate_project_tagcloud(\\%ctags);\n> -\t\tprint git_show_project_tagcloud($cloud, 64);\n> +\t\tgit_project_list_ctags(\\@projects);\n>  \t}\n>  \n>  \tprint \"<table class=\\\"project_list\\\">\\n\";\n> @@ -4361,6 +4366,28 @@ sub git_project_list_body {\n>  \tprint \"</table>\\n\";\n>  }\n>  \n> +sub git_project_search_form {\n> +\tprint $cgi->startform(-method => \"get\") .\n> +\t      \"<p class=\\\"projsearch\\\">Search:\\n\" .\n> +\t      $cgi->textfield(-name => \"s\", -value => $searchtext) . \"\\n\" .\n> +\t      \"</p>\" .\n> +\t      $cgi->end_form() . \"\\n\";\n> +}\n> +\n> +sub git_project_list_all {\n> +\tmy $order = $input_params{'order'};\n> +\tif (defined $order && $order !~ m/none|project|descr|owner|age/) {\n> +\t\tdie_error(400, \"Unknown order parameter\");\n> +\t}\n> +\n> +\tmy @list = git_get_projects_list();\n> +\tif (!@list) {\n> +\t\tdie_error(404, \"No projects found\");\n> +\t}\n> +\n> +\tgit_project_list_body(\\@list, $order);\n> +}\n> +\n>  sub git_shortlog_body {\n>  \t# uses global variable $project\n>  \tmy ($commitlist, $from, $to, $refs, $extra) = @_;\n> @@ -4630,28 +4657,14 @@ sub git_search_grep_body {\n>  ## actions\n>  \n>  sub git_project_list {\n> -\tmy $order = $input_params{'order'};\n> -\tif (defined $order && $order !~ m/none|project|descr|owner|age/) {\n> -\t\tdie_error(400, \"Unknown order parameter\");\n> -\t}\n> -\n> -\tmy @list = git_get_projects_list();\n> -\tif (!@list) {\n> -\t\tdie_error(404, \"No projects found\");\n> -\t}\n> -\n>  \tgit_header_html();\n>  \tif (-f $home_text) {\n>  \t\tprint \"<div class=\\\"index_include\\\">\\n\";\n>  \t\tinsert_file($home_text);\n>  \t\tprint \"</div>\\n\";\n>  \t}\n> -\tprint $cgi->startform(-method => \"get\") .\n> -\t      \"<p class=\\\"projsearch\\\">Search:\\n\" .\n> -\t      $cgi->textfield(-name => \"s\", -value => $searchtext) . \"\\n\" .\n> -\t      \"</p>\" .\n> -\t      $cgi->end_form() . \"\\n\";\n> -\tgit_project_list_body(\\@list, $order);\n> +\tgit_project_search_form();\n> +\tgit_project_list_all();\n>  \tgit_footer_html();\n>  }\n>  \n"},{"id":"127692","messageId":"20091116175630.GK17748@machine.or.cz","threadId":"21501","inReplyTo":"4AF47142.4010609@eaglescrag.net","subject":"Re: [PATCH] gitweb: Refactor project list routines","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2009-11-16T17:56:30Z","receivedAt":"2009-11-16T17:56:30Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"  Hi!\n\nOn Fri, Nov 06, 2009 at 10:56:02AM -0800, J.H. wrote:\n> 2) If the repository is cloned the ctag information is not retained,\n> which means there is no real way for the original developer to\n> manage or move this information between different hosting sites,\n> I.E. repo.or.cz and kernel.org (though I'll admit I have it turned\n> off)\n\n  This is interesting argument, I have always thought about ctags being\na rather site-specific mechanism and I think it would've meet with a lot\nof user opposition to e.g. keep ctags in a specific branch; I can think\nof no other good way to do it either.\n\n> So if your going to eliminate the project listing, with the general\n> intention of using the tag cloud as more of a primary search\n> mechanism, including the search box, I think there's some serious\n> work that needs to be put into the ctags system because in it's\n> current state, for the likes of kernel.org, it's unusable, unstable\n> and not something I would recommend to anyone to run in production.\n> I like the idea, I just have concerns over it's current\n> implementation.\n\n  This patch is orthogonal to that, I believe. I agree that the ctags\nmechanism is somewhat hackish; unfortunately, I personally don't have\nthe time to fix it up properly. I think it is still good enough for\nmany users, and the frontpage mechanism would be quite useful even for\npeople who don't want to use content tags.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nA lot of people have my books on their bookshelves.\nThat's the problem, they need to read them. -- Don Knuth\n"}]}