Dnia sobota 6. grudnia 2008 14:25, Giuseppe Bilotta napisał:
> On Sat, Dec 6, 2008 at 1:53 AM, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
>> On Wed, 3 Dec 2008, Giuseppe Bilotta wrote:
>>
>>> In shortlog view, a link to the patchset is only offered when the number
>>> of commits shown is less than the allowed maximum number of patches.
>>>
>>> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@xxxxxxxxx>
>>> + if (gitweb_check_feature('patches')) {
>>> + $formats_nav .= " | " .
>>> + $cgi->a({-href => href(action=>"patch", -replay=>1)},
>>> + "patch");
>>> + }
>>>
>>> if (!defined $parent) {
>>> $parent = "--root";
>>> @@ -5415,6 +5420,11 @@ sub git_commitdiff {
>>> $formats_nav =
>>> $cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
>>> "raw");
>>> + if ($patch_max) {
>>> + $formats_nav .= " | " .
>>> + $cgi->a({-href => href(action=>"patch", -replay=>1)},
>>> + "patch");
>>> + }
>>>
>>> if (defined $hash_parent &&
>>> $hash_parent ne '-c' && $hash_parent ne '--cc') {
>>
>> In the above two hunks 'patch' view functions as "git show --pretty=email"
>> text/plain equivalent, but this duplicates a bit 'commitdiff_plain'
>> functionality. Well, 'commitdiff_plain' has currently some errors,
>> but...
>
> All things considered, for single commit view there is (modulo bugs)
> no factual difference between plain diff and patch view.
>
> Although we could merge them, I'm thinking that the plain diff view
> has somewhat too much information in it. For backwards compatibility
> it's probably not wise to change it, but we should consider it for the
> next major version, honestly.
I'm just wondering if we should add 'patch' link to 'commit' and
'commitdiff' views (as alternate view) at all...
>>> @@ -5949,6 +5959,14 @@ sub git_shortlog {
>>> $cgi->a({-href => href(-replay=>1, page=>$page+1),
>>> -accesskey => "n", -title => "Alt-n"}, "next");
>>> }
>>> + my $patch_max = gitweb_check_feature('patches');
>>> + if ($patch_max) {
>>> + if ($patch_max < 0 || @commitlist <= $patch_max) {
>>> + $paging_nav .= " ⋅ " .
>>> + $cgi->a({-href => href(action=>"patch", -replay=>1)},
>>> + @commitlist> 1 ? "patchset" : "patch");
>>> + }
>>> + }
>>
>> Here 'patch' view functions as "git format-patch", able to be downloaded
>> and fed to git-am. Perhaps the action should also be named 'patches'
>> here?; it could lead to the same function.
>
> I had half an idea to do so. 'patches' or 'patchset'?
Hmmm... I think 'patches'.
>> By the way, there is subtle bug in above link. If shortlog view is less
>> than $patch_max commits long, but it is because the history for a given
>> branch (or starting from given commit) is so short, and not because
>> there is cutoff $hash_parent set, the 'patchset' view wouldn't display
>> plain text equivalent view, but only patch for top commit.
>
> Ah, good point.
>
> Hm, not easy to solve. One way could be to add the hash_parent
> manually. Or we could make the 'patches' view different from the
> 'patch' view in the way it handles refspecs without ranges. I'm
> leaning towards the latter. What's your opinion?
I think simplest solution would be to add $hash_parent if it is not
set from the last commit, i.e. $commitlist[-1]{'id'}
>> I assume that the link is only for 'shortlog' view, and not also for
>> 'log' and 'history' views because 'shortlog' is the only log-like view
>> which support $hash_parent?
>
> The actual reason is that I never use log nor history view, but since
> they don't support hash_parent because of this (I was the one who sent
> the patch to support hash_parent in shortlog view) you could
> paralogistically say that's the reason ;-)
>
> I'm not sure about history view, but for log view I'm considering
> addiong also a 'patch' link next to each commit. I'll think about it.
Well, you can add it only for 'shortlog' view, and when the code for
all log-like views would get consolidated, you will get link to 'patches'
view automatically :-)
--
Jakub Narebski
Poland
--
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
