From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [RFC PATCH] gitweb: improve title shortening heuristics
Date: Mon, 25 Jul 2022 07:54:34 +0200 [thread overview]
Message-ID: <220725.86czdtrayy.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <20220725020534.x33hso22ab2dwyo7@jrouhaud>
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...
next prev parent reply other threads:[~2022-07-25 5:57 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-24 6:12 [RFC PATCH] gitweb: improve title shortening heuristics Julien Rouhaud
2022-07-25 1:30 ` Junio C Hamano
2022-07-25 2:12 ` Julien Rouhaud
2022-07-25 5:54 ` Ævar Arnfjörð Bjarmason [this message]
2022-07-26 13:59 ` Julien Rouhaud
2022-07-27 6:31 ` Junio C Hamano
2022-07-28 1:30 ` Julien Rouhaud
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=220725.86czdtrayy.gmgdl@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=rjuju123@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).