From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [PATCH v2] Font lock long Git commit summary lines Date: Mon, 05 Sep 2022 15:07:49 +0300 Message-ID: <8335d6koru.fsf@gnu.org> References: <20220903170308.301805-1-spwhitton@spwhitton.name> <871qsq2096.fsf@melete.silentflame.com> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="9135"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Sean Whitton Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Mon Sep 05 14:38:22 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oVBMq-0002A9-6Z for ged-emacs-devel@m.gmane-mx.org; Mon, 05 Sep 2022 14:38:20 +0200 Original-Received: from localhost ([::1]:58578 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oVBMo-0003wS-QI for ged-emacs-devel@m.gmane-mx.org; Mon, 05 Sep 2022 08:38:18 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:39208) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oVAtg-00012e-8W for emacs-devel@gnu.org; Mon, 05 Sep 2022 08:08:12 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]:54566) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oVAtf-000831-Ul; Mon, 05 Sep 2022 08:08:11 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=2cvhtcnVGhefDoYnzNWbLwDELymxsOAly/u5sCNA9IE=; b=g5k0k3gWDhnu 7bM/hSpdClkIE6+cxFFdhZf04ivy2p9t1WMSi2X4yEP2dvuh58DVqBqCCXyilx56TbXqDTp5n1xSB 7UIi+ecPIzBzsiJZtWsKAMAbkrohGvip/t2Q6zZ8q77udZJCpJND2Dl6Bu1CgqwiumtAVI9UofzRh FD8rRZQRWkwC7Cr9HyJr5hlNVOD5iEsR/86Wqmx6dS96IupQ4Co3ELSkjBed6r9bu0oLWoiHMriZx 4dTJxmAd9LKnX+JntjzxnXbpoJ7Wi20qzgXvVCx5/6Dzaspn99zVHiiqPICGfh1tLo75JwTVEenpx EG0PNNGNgDzIMuh+vPxiGA==; Original-Received: from [87.69.77.57] (port=4775 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oVAtf-0003Qc-Bl; Mon, 05 Sep 2022 08:08:11 -0400 In-Reply-To: <871qsq2096.fsf@melete.silentflame.com> (message from Sean Whitton on Sun, 04 Sep 2022 16:22:29 -0700) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:294743 Archived-At: > From: Sean Whitton > Date: Sun, 04 Sep 2022 16:22:29 -0700 > > Here is v2 of my patch. Thank you for the comments. Thanks. May I comment a bit more? > +(defcustom vc-git-log-edit-summary-target nil > + "Target length for Git commit summary lines. Given the description below, "Target length" is not the best wording here. Perhaps "Preferred maximum length" is better? And consequently, I think a better name for the variable is vc-git-log-edit-summary-warning-len > +By setting this to an integer around 50, you can improve the > +compatibility of your commit messages with Git commands that > +print the summary line in width-constrained contexts. However, > +many commit summaries will need to exceed this length. I'd lose the last sentence here: it confuses the whole issue. When you say "Preferred" or "Target", and describe what will happen beyond that, you already in effect tell the reader that the limitation is not a hard one, but just a guideline. > +(defface vc-git-log-edit-summary-warning > + '((t :inherit warning)) > + "Face for Git commit summary lines beyond the target length.") This should mention vc-git-log-edit-summary-target. Imagine a user who looks at the face's doc string and wonders which "target length" does the above allude to. > +(defcustom vc-git-log-edit-summary-max 68 I'd call this vc-git-log-edit-summary-max-len > + "Maximum length for Git commit summary lines. > +If non-nil, characters in summary lines beyond this length are ^^^^^^^^^^ "If a number, ..." Saying "if non-nil, ... beyond this length" is problematic, because not all non-nil values are numbers. > +(defface vc-git-log-edit-summary-error > + '((t :inherit error)) > + "Face for Git commit summary lines beyond the maximum length.") Reference the variable in the doc string. > +(defun vc-git--log-edit-summary-check (limit) > + (and (re-search-forward "^Summary: " limit t) > + (when-let ((regex > + (cond ((and vc-git-log-edit-summary-max > + vc-git-log-edit-summary-target) Should this test using numberp? > + (vc-git-log-edit-summary-max > + (format ".\\{,%d\\}\\(?2:.*\\)" > + vc-git-log-edit-summary-max)) > + (vc-git-log-edit-summary-target > + (format ".\\{,%d\\}\\(.*\\)" > + vc-git-log-edit-summary-target))))) Likewise here.