Re: [RFC PATCH] gitweb: improve title shortening heuristics

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

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@gmail.com> 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...