From: Eli Zaretskii <eliz@gnu.org>
To: "Kévin Le Gouguec" <kevin.legouguec@gmail.com>
Cc: rahguzar@zohomail.eu, larsi@gnus.org, 66676@debbugs.gnu.org
Subject: bug#66676: 29.1; Should some aspects of shr rendering be configurable
Date: Sat, 18 Nov 2023 10:59:23 +0200 [thread overview]
Message-ID: <831qcno2s4.fsf@gnu.org> (raw)
In-Reply-To: <87wmuxoha2.fsf@gmail.com> (message from Kévin Le Gouguec on Sat, 04 Nov 2023 13:05:25 +0100)
> 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.
next prev parent reply other threads:[~2023-11-18 8:59 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-22 7:29 bug#66676: 29.1; Should some aspects of shr rendering be configurable Rahguzar via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-10-22 9:52 ` Eli Zaretskii
2023-10-22 10:26 ` Rahguzar via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-10-25 16:18 ` Rahguzar via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-11-04 8:10 ` Eli Zaretskii
2023-11-04 9:06 ` Rahguzar via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-11-04 12:05 ` Kévin Le Gouguec
2023-11-18 8:59 ` Eli Zaretskii [this message]
2023-11-19 12:07 ` Rahguzar via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-11-22 20:14 ` Rahguzar via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-11-25 11:01 ` Eli Zaretskii
2023-11-18 13:08 ` Rahguzar via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-11-18 18:09 ` Kévin Le Gouguec
2023-11-19 11:12 ` Rahguzar via Bug reports for GNU Emacs, the Swiss army knife of text editors
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=831qcno2s4.fsf@gnu.org \
--to=eliz@gnu.org \
--cc=66676@debbugs.gnu.org \
--cc=kevin.legouguec@gmail.com \
--cc=larsi@gnus.org \
--cc=rahguzar@zohomail.eu \
/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 external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.