On Mon, Jul 25 2022, Julien Rouhaud wrote:
> On Sun, Jul 24, 2022 at 06:30:44PM -0700, Junio C Hamano wrote:
>> Julien Rouhaud <rjuju123@xxxxxxxxx> writes:
>>
>> > Otherwise, would it be acceptable to disable the whole block (the "remove
>> > leading stuff of merges to make the interesting part visible") with some new
>> > configuration option?
>>
>> I personally find that "shortening" logic way too specific to the
>> kernel project hosted at kernel.org and would be inappropriate to
>> use it anywhere else.
>>
>> if (length($title) > 50) {
>> $title =~ s/^Automatic //;
>> $title =~ s/^merge (of|with) /Merge ... /i;
>> if (length($title) > 50) {
>> $title =~ s/(http|rsync):\/\///;
>> }
>> if (length($title) > 50) {
>> $title =~ s/(master|www|rsync)\.//;
>> }
>> if (length($title) > 50) {
>> $title =~ s/kernel.org:?//;
>> }
>> if (length($title) > 50) {
>> $title =~ s/\/pub\/scm//;
>> }
>> }
>> $co{'title_short'} = chop_str($title, 50, 5);
>
> That's probably true, although some parts (like the protocol) seemed general
> enough to me to be worth considering for general use case, and some of the
> really specific parts seems so specific that they shouldn't really matter if
> used outside kernel.org.
>>
>> Of course, http:// and rsync:// are way outdated (https://, ssh://
>> and git:// are probably reasonable). Equally outdated is to merge
>> branches from master.kernel.org, www.kernel.org, or rsync.kernel.org
>> (many merges are recorded as pulling from git://git.kernel.org/ or
>> https://git.kernel.org/ these days).
>>
>> Even worse, I somehow thought that kernel.org no longer uses gitweb
>> but uses something else. So I suspect that nobody sheds tears if we
>> remove the whole block unconditionally. In fact, it would make the
>> world a better place.
>
> Well, I'm obviously totally fine with getting rid of it. FWIW, even the
> protocol part wouldn't change anything for the instance hosted by postgres.
>
> I'm attaching an updated v2 patch that removes all that logic and just keeps
> the title_short chopped at 50 chars.
Please "inline" your patches, see "Sending your patches" in
Documentation/SubmittingPatches (I.e. send it with git-send-email, or
similar). I see this as:
> [2. text/plain; v2-0001-gitweb-Remove-title_short-shortening-heuristics.patch]...
In my MUA. But manually extracting it and quoting it:
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 1835487ab2..e66eb3d9ba 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3560,23 +3560,6 @@ sub parse_commit_text {
> $title =~ s/^ //;
> if ($title ne "") {
> $co{'title'} = chop_str($title, 80, 5);
> - # remove leading stuff of merges to make the interesting part visible
> - if (length($title) > 50) {
> - $title =~ s/^Automatic //;
> - $title =~ s/^merge (of|with) /Merge ... /i;
> - if (length($title) > 50) {
> - $title =~ s/(http|rsync):\/\///;
> - }
> - if (length($title) > 50) {
> - $title =~ s/(master|www|rsync)\.//;
> - }
> - if (length($title) > 50) {
> - $title =~ s/kernel.org:?//;
> - }
> - if (length($title) > 50) {
> - $title =~ s/\/pub\/scm//;
> - }
> - }
> $co{'title_short'} = chop_str($title, 50, 5);
> last;
This looks good to me, The one thing I'd add is that we're just running:
git rev-list --parents --header --max-count=1 HEAD
And parsing that, but if we're truncating things perhaps we should just
run "git log" or "git show" with the "%<(<N>[,trunc|ltrunc|mtrunc])"
syntax or similar.
That's obviously a follow-up, but if anyone's interested in deleting
even more code here...
