Re: [PATCH 2/8] gitweb: We do longer need the --parents flag in rev-list.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Robert Fitzsimons wrote:

> We only want to know the direct parents of a given commit object,
> these parents are available in the --header output of rev-list.  If
> --parents is supplied with --full-history the output includes merge
> commits that aren't relevant.

Actually --header output gives us original parents. Rewritten parents
(available with --parents) include also grafts and shallow clone grafts.
For parse_commit we want --parents, for parse_commits we don't want it
because --parents affects --full-history.

The problem is that we cannot detect if git-rev-list was called with
--parents and commit is root commit (parentless), or we didn't use
--parents option.

In few other places we pass options specifying subroutine behavior
as hash after all other requred parameters, e.g. 
  esc_html($line, -nbsp=>1),
  parse_ls_tree_line($line, -z=>1), 
  git_print_log($co{'comment'}, -final_empty_line=> 1, -remove_title => 1);
In this case it wouldn't work (unless we pass reference to array,
via parse_commit_text( [ <$fd> ], -parents=>1);

Perhaps it would be better to use reference to hash of options as _first_
parameter, e.g. parse_commit_text({-parents=>1}, <$fd>);, and use something
like if (ref($[0]) == 'HASH') { $opts = shift @_; } to get options.

So for now gitweb might not show what we want in very rare cases of
repositories with grafts or shallow clones.


But apart from this small matter, this series is excellent work. Thanks! 
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git


-
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]