From mboxrd@z Thu Jan  1 00:00:00 1970
Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail
From: Eli Zaretskii <eliz@gnu.org>
Newsgroups: gmane.emacs.bugs
Subject: bug#66676: 29.1; Should some aspects of shr rendering be configurable
Date: Sat, 18 Nov 2023 10:59:23 +0200
Message-ID: <831qcno2s4.fsf@gnu.org>
References: <87v8azxek4.fsf@zohomail.eu> <83o7groufg.fsf@gnu.org>
 <87r0lnx7kx.fsf@zohomail.eu> <87zg06r7fk.fsf@zohomail.eu>
 <83lebe547t.fsf@gnu.org> <87wmuxoha2.fsf@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214";
	logging-data="25601"; mail-complaints-to="usenet@ciao.gmane.io"
Cc: rahguzar@zohomail.eu, larsi@gnus.org, 66676@debbugs.gnu.org
To: =?UTF-8?Q?K=C3=A9vin?= Le Gouguec <kevin.legouguec@gmail.com>
Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Nov 18 10:00:19 2023
Return-path: <bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org>
Envelope-to: geb-bug-gnu-emacs@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 <bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org>)
	id 1r4HBb-0006WQ-Ab
	for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 18 Nov 2023 10:00:19 +0100
Original-Received: from localhost ([::1] helo=lists1p.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.90_1)
	(envelope-from <bug-gnu-emacs-bounces@gnu.org>)
	id 1r4HBM-0001j8-Gy; Sat, 18 Nov 2023 04:00:04 -0500
Original-Received: from eggs.gnu.org ([2001:470:142:3::10])
 by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <Debian-debbugs@debbugs.gnu.org>)
 id 1r4HBK-0001hE-Hx
 for bug-gnu-emacs@gnu.org; Sat, 18 Nov 2023 04:00:02 -0500
Original-Received: from debbugs.gnu.org ([2001:470:142:5::43])
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128)
 (Exim 4.90_1) (envelope-from <Debian-debbugs@debbugs.gnu.org>)
 id 1r4HBJ-0007YI-Kb
 for bug-gnu-emacs@gnu.org; Sat, 18 Nov 2023 04:00:01 -0500
Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2)
 (envelope-from <Debian-debbugs@debbugs.gnu.org>) id 1r4HBK-00070D-A7
 for bug-gnu-emacs@gnu.org; Sat, 18 Nov 2023 04:00:02 -0500
X-Loop: help-debbugs@gnu.org
Resent-From: Eli Zaretskii <eliz@gnu.org>
Original-Sender: "Debbugs-submit" <debbugs-submit-bounces@debbugs.gnu.org>
Resent-CC: bug-gnu-emacs@gnu.org
Resent-Date: Sat, 18 Nov 2023 09:00:02 +0000
Resent-Message-ID: <handler.66676.B66676.170029797626854@debbugs.gnu.org>
Resent-Sender: help-debbugs@gnu.org
X-GNU-PR-Message: followup 66676
X-GNU-PR-Package: emacs
Original-Received: via spool by 66676-submit@debbugs.gnu.org id=B66676.170029797626854
 (code B ref 66676); Sat, 18 Nov 2023 09:00:02 +0000
Original-Received: (at 66676) by debbugs.gnu.org; 18 Nov 2023 08:59:36 +0000
Original-Received: from localhost ([127.0.0.1]:47787 helo=debbugs.gnu.org)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <debbugs-submit-bounces@debbugs.gnu.org>)
 id 1r4HAu-0006z3-4Y
 for submit@debbugs.gnu.org; Sat, 18 Nov 2023 03:59:36 -0500
Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:49222)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <eliz@gnu.org>) id 1r4HAq-0006yp-SW
 for 66676@debbugs.gnu.org; Sat, 18 Nov 2023 03:59:34 -0500
Original-Received: from fencepost.gnu.org ([2001:470:142:3::e])
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <eliz@gnu.org>)
 id 1r4HAk-0007Un-KQ; Sat, 18 Nov 2023 03:59:26 -0500
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org;
 s=fencepost-gnu-org; h=MIME-version:References:Subject:In-Reply-To:To:From:
 Date; bh=N0m+6LmnSV9MtVGdgotMwQhV9NSvwnMpEi5zUv8u/Vg=; b=NPtJ+bqt6apxpe8lVM3Y
 ZNuK63ez8Aid+p29XYqtY7I0Eyp3S8PKany7g0UaIDr34sSI/tJyLdW2qoeUH6z738xxBIwKFxsQw
 jAUlAemsqxQDhCrsyzIOm+hXQPQY/XsnthGSotxuf9NRKSbVFCiBaEkcoZ6tpn/LiPW6lgs+Eq2FI
 QeyEaVt3nvWTFhIcapoLDdXrvQOprrgoWukZOAoduT06eIIOdGkWyg9FBgzgeJeN6iOe5dueGppTF
 kFkTljKsULDIh/a4nXXwDncataXqc6nwS+iwaw8ozmGQbdRTWGcoxxuE39lm4iphhBrpJKnotB6gA
 Y6J283ViGpjBnQ==;
In-Reply-To: <87wmuxoha2.fsf@gmail.com> (message from =?UTF-8?Q?K=C3=A9vin?=
 Le Gouguec on Sat, 04 Nov 2023 13:05:25 +0100)
X-BeenThere: debbugs-submit@debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
X-BeenThere: bug-gnu-emacs@gnu.org
List-Id: "Bug reports for GNU Emacs,
 the Swiss army knife of text editors" <bug-gnu-emacs.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/bug-gnu-emacs>,
 <mailto:bug-gnu-emacs-request@gnu.org?subject=unsubscribe>
List-Archive: <https://lists.gnu.org/archive/html/bug-gnu-emacs>
List-Post: <mailto:bug-gnu-emacs@gnu.org>
List-Help: <mailto:bug-gnu-emacs-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/bug-gnu-emacs>,
 <mailto:bug-gnu-emacs-request@gnu.org?subject=subscribe>
Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org
Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org
Xref: news.gmane.io gmane.emacs.bugs:274537
Archived-At: <http://permalink.gmane.org/gmane.emacs.bugs/274537>

> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>,  rahguzar@zohomail.eu,
>   66676@debbugs.gnu.org
> Date: Sat, 04 Nov 2023 13:05:25 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Lars, anybody else?  Any comments to these changes?
> 
> As an occasional user of shr (mainly when viewing HTML parts in Gnus)
> who frequently messes with window width and face height, I'm very much
> interested in better visual-line-mode integration.  Also intrigued by
> outline-mode support.  So closely following this thread, and
> enthusiastically thanking Rahguzar for tackling this 👏
> 
> Don't know if I have anything insightful to say re. the patches.  Idly
> wondering if '(null shr-fill-text) ⇒ (visual-line-mode)' is the right
> control flow; e.g. maybe
> 
>   (a) shr-fill-text could be set to 'visual as a more explicit hint for
>   major modes, or
> 
>   (b) major modes should grow new user options: e.g. eww-wrap-method ∈
>   {fill, visual}; 'visual would tell modes to set shr-fill-text to nil &
>   enable visual-line-mode.
> 
> There are already plenty of user knobs at the shr level though, so maybe
> no need to overthink this.

Thanks.  Rahguzar, any followup to these comments?

Please also see my minor comments below:

> * lisp/net/shr.el
> (shr-fill-text): New custom variable
> (shr-sup-raise-factor): New custom variable
> (shr-sub-raise-factor): New custom variable
> (shr-image-ascent): New custom variable
> (shr-fill-lines): Only fill if shr-fill-text is non nil
> (shr-put-image): Use shr-image-ascent as value of :ascent
> (shr-rescale-image): Use shr-image-ascent
> (shr-make-placeholder-image): Use shr-image-ascent
> (shr-tag-sup): use shr-sup-raise-factor
> (shr-tag-sub): use shr-sub-raise-factor

This doesn't follow our conventions:

  . identical entries should be grouped if possible (see below)
  . descriptions of changes should be complete sentences: start with a
  capital letter and end with a period
  . symbols should be quoted 'like this'

In this case, here's how to format the above descriptions:

* lisp/net/shr.el (shr-fill-text, shr-sup-raise-factor)
(shr-sub-raise-factor, shr-image-ascent): New custom variables.
(shr-fill-lines): Only fill if 'shr-fill-text' is non-nil.
(shr-put-image): Use 'shr-image-ascent' as value of :ascent.
(shr-rescale-image, shr-make-placeholder-image): Use
'shr-image-ascent'.
(shr-tag-sup, shr-tag-sub): Use 'shr-sub-raise-factor'.

Similar changes are needed in your other log messages.

> +(defcustom shr-fill-text t
> +  "Non-nil means to fill the text according to the width of the window.
> +If nil text is not filled and `visual-line-mode' can be used to reflow text."
         ^                  ^
Two commas missing there.

> +(defcustom shr-sup-raise-factor 0.2
> +  "The value of raise property for superscripts.
> +Should be a number between 0 and 1."

This is better:

  Should be a non-negative float number between 0 and 1.

> +(defcustom shr-sub-raise-factor -0.2
> +  "The value of raise property for subscripts.
> +Should be a number between 0 and -1."

Likewise here (but "non-positive" instead of "non-negative").

> +(defcustom shr-max-inline-image-size nil
> +  "If non-nil determines when the images can be displayed inline.
> +If nil images are never displayed inline.

Commas missing after "nil" in both sentences.

> +HEIGHT can be also be an integer or a floating point number.  If it is an
> +integer and the pixel height of an image exceeds it, the image image is
> +displyed on a separate line.  If it is an floating point, the limit is
                                          ^^^^^^^^^^^^^^^^^
"a floating point number"

> +interpreted as multiples of the height of default font."
               ^^^^^^^^^^^^
"as a multiple"

> @@ -1103,19 +1135,25 @@ shr-put-image
>                                             (plist-get flags :width)
>                                             (plist-get flags :height)))))))
>          (when image
> +          ;; The trailing confuse can confuse shr-insert into not
> +          ;; putting any space after inline images.

"The trailing confuse can confuse" sounds strange, and is probably a
typo of sorts.  What did you mean to say there?

> * lisp/net/shr.el (shr-tag-sub): see above

This is not a proper change description.  Either repeat the heading or
include the file and function in the heading line (if they fit; they
don't fit in this case, I think).

> +  ;; possible in Emacs. So we remove the newline in that case.
                         ^^
Our convention is to leave 2 spaces between sentences, not one.

Thanks.