Junio C Hamano wrote:
> Jakub Narebski <jnareb@xxxxxxxxx> writes:
> > Jakub Narebski wrote:
>>
>>> * Refactor generation of navigation bar. There are at least two
>>> implementations of that. With hash dispatch it would be easy to
>>> list all possibilities.
>>
>> Actually I think that whole gitweb.cgi needs refactoring, badly.
>> Generation of navigation bar is only one, admittedly worst,
>> example of code duplication.
>
> Yes. I liked what xmms2 folks did to the navbar exactly for
> that reason. We would be better off to first clean up what we
> currently have before starting to build too much on it.
So lets talk about gitweb refactoring.
Currently gitweb subroutines can be divided into following categories:
1. "Action" subroutines, which do all the work, i.e. they output whole
contents of the page. Currently they are named git_$action, e.g.
git_summary(), git_log(), git_blob(), git_heads(),... , git_blame().
The list includes "hidden" actions, namely git_logo(), git_opml(), and
git_project_list(). There is nothing 'git' about those subroutines.
How should they be named? What prefix to use? action_summary(),
write_summary(), output_summary(), out_summary(), gitweb_summary()?
2. Miscelanous "HTML" subroutines, outputting fragments of HTML code, like
git_header_html, git_footer_html and die_error; or returning fragment of
HTML code, like format_log_line_html. Refactoring should put all the work
into such subroutines.
3. Many helper subroutines:
- related o HTML or HTTP: esc_param, esc_html, mimetype_guess_file,
mimetype_guess, git_blob_plain_mimetype, age_class
- mangling git output: unquote, age_class, age_string, mode_str, file_type
- other helper subroutines: chop_str, date_str, get_file_owner,
validate_input
4. Subroutines which invoke git commands, usually using "-|" LIST magic
output call: git_get_type($hash), git_get_project_config($key) and
git_get_project_config_bool($key), git_get_hash_by_path($base, $path).
Including git_read_tag($hash) and git_read_commit($hash) which fills
%tag/%co object with information from parsing tag/commit.
Unusual in this set git_read_head($project) which sets $ENV{GIT_DIR}
temporarily and git_diff_print($from_id, $from_name, $to_id, $to_name,
$format = "html") which does the work for blobdiff and blobdiff_plain using
temporary files (new git probably can do this without need for temporaru
files)
5. Subroutines which directly access git repository, sometimes calling
subroutines mentioned above, including: git_read_hash (which needs
refactoring itself I think), git_read_description, git_read_projects,
read_info_ref, git_read_refs (instead of using git-branch or git-tag -l,
which also parses information into %ref_item hash).
6* Listed in the sets above are suroutines which parse info into hash or
array of hashes/hashrefs: git_read_tag, git_read_commit, git_read_refs.
Other multi-line output like 'ls-tree' output is parsed and output line by
line.
Some of those subroutines probably will find the way to Git.pm. But in all
they need to be cleanly separated into classed if possible, and reworked to
do one thing and do it well. Passing parameters instead of relying on
global variables would help porting to mod_perl, for example.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
-
: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html