W dniu 2013-12-02 13:06, Krzesimir Nowak pisze:
On Mon, 2013-12-02 at 01:21 +0100, Jakub Narębski wrote:
On Thu, Nov 28, 2013 at 12:44 PM, Krzesimir Nowak
<krzesimir@xxxxxxxxxxxx> wrote:
Allow @additional_branch_refs configuration variable to tell gitweb to
show refs from additional hierarchies in addition to branches in the
list-of-branches view.
Signed-off-by: Krzesimir Nowak<krzesimir@xxxxxxxxxxxx>
Why not use %feature hash instead of adding new configuration variable?
I think that this option is similar enough to 'remote_heads' feature
(which BTW should be 'remote-heads'), and could conceivably enabled
on a per-repository basis, i.e. with repository configuration override,
isn't it?
I'd like to see some consensus on it before I start changing the patch
again.
%feature hash is mainly (but not only) about options that can be
configured on per-repository basis. Configuration variables are
about options that are per-instance (per gitweb).
Usually %feature hash is preferred over adding new configuration variable
but this is not some hard rule. Note however that patches adding new config
are met with more scrutiny, as it is harder to fix mistakes because of
requirement of backwards compatibility of configuration files.
I don't know what kind of backwards compatibility you mention. Whether
you want gitweb to survive reading old config file or to honor
deprecated/old config variables.
I meant here honoring deprecated/old variables, i.e. honoring existing
configuration files. See for example backward compatibility for old
$stylesheet variable vs new @stylesheets in print_header_links().
Though in this case it shouldn't be much of a problem; it would be
easy to honor @additional_branch_refs by setting 'default' for
'extra-branch-refs' feature to it.
BTW. there really should be gitweb/CodingGuidelines...
Yes, would be useful. As in every other project. :)
Well, Git itself *has* Documentation/CodingGuidelines, but perhaps
gitweb subsystem should have it's own...
[...]
@@ -3662,7 +3701,8 @@ sub git_get_heads_list {
my ($committer, $epoch, $tz) =
($committerinfo =~ /^(.*) ([0-9]+) (.*)$/);
$ref_item{'fullname'} = $name;
- $name =~ s!^refs/(?:head|remote)s/!!;
+ my $strip_refs = join '|', map { quotemeta } get_branch_refs();
+ $name =~ s!^refs/(?:$strip_refs|remotes)/!!;
$ref_item{'name'} = $name;
$ref_item{'id'} = $hash;
@@ -7179,7 +7219,8 @@ sub snapshot_name {
$ver = $1;
} else {
# branches and other need shortened SHA-1 hash
- if ($hash =~ m!^refs/(?:heads|remotes)/(.*)$!) {
+ my $strip_refs = join '|', map { quotemeta } get_branch_refs();
+ if ($hash =~ m!^refs/(?:$strip_refs|remotes)/(.*)$!) {
$ver = $1;
}
$ver .= '-' . git_get_short_hash($project, $hash);
One one hand, it is about threating extra branch refs the same way as 'head'.
On the other hand we loose distinction between 'refs/heads/foo' and e.g.
'refs/wip/foo'. But maybe that's all right...
In git_get_heads_list sub I could append a " ($ref_dir)" to refs which
are in neither 'heads' nor 'remotes', so heads view would look like:
master
old-stable
some-work-in-progress (wip)
some-other-branch (other)
where both master and old-stable are in refs/heads/,
some-work-in-progress in refs/wip/ and some-other-branch in refs/other/.
In case of branch snapshot names (snapshot_name sub) I could change it,
so names for branches mentioned above would be
"Project-master-<short-hash>.tgz",
"Project-old_stable-<short-hash>.tgz",
"Project-wip-some-work-in-progress-<short-hash>.tgz"
"Project-other-some-other-branch-<short-hash>.tgz"
What do you think?
That is, I think, a very good idea. Though perhaps it would be more
readable to add this extra feature as a separate patch, on top of main one.
--
Jakub Narebski
--
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