On Sat, Nov 1, 2008 at 1:18 AM, Jakub Narebski <jnareb@xxxxxxxxx> wrote: > On Sun, 19 Oct 2008, Giuseppe Bilotta wrote: > > I'm sorry for the delay. No problem. >> When PATH_INFO is active, get rid of the sf CGI parameter by embedding >> the snapshot format information in the PATH_INFO URL, in the form of an >> appropriate extension. > > The question is: should we use format suffix (e.g. 'tar.gz'), > or format name (e.g. 'tgz')? The latter is later easier to parse, > see comments to first patch in better snapshot support for path_info. I think it's essentially a matter of deciding which extension to use by default on output, and accepting either extension on input. >> + # since we destructively absorb parameters, we keep this >> + # boolean that remembers if we're handling a snapshot >> + my $is_snapshot = $params{'action'} eq 'snapshot'; >> + > > Side note: we destructively absorb parameters, because parameters > which are not absorbed are then used to generate query string part > of URL. > > Deleting parameter but remembering the fact that it was used is one > (but not only) solution. Well, in the specific case of the 'action' parameter we could just keep it around and only discard it at the end. >> + if (!$fmt) { >> + my @snapshot_fmts = gitweb_check_feature('snapshot'); >> + @snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts); >> + $fmt = $snapshot_fmts[0]; >> + } > > I anderstand that the above code is improved with new patch? Yup, I followed your suggestion of putting the refactoring earlier. >> + $href .= $known_snapshot_formats{$fmt}{'suffix'}; > > Again: should we use snapshot prefix, or snapshot name, which means here > do we use $known_snapshot_formats{$fmt}{'suffix'}; or just $fmt; ? I really don't know, either is fine for me. Maybe using $fmt has the benefit that it copes better with some buggy versions of some browser (like Opera) that tend to strip the .gz from .tar.gz files ... -- Giuseppe "Oblomov" Bilotta -- To unsubscribe from this list: 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