GNU bug report logs -
#66676
29.1; Should some aspects of shr rendering be configurable
Previous Next
Reported by: Rahguzar <rahguzar <at> zohomail.eu>
Date: Sun, 22 Oct 2023 08:12:02 UTC
Severity: wishlist
Found in version 29.1
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
Full log
Message #43 received at 66676 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi Eli,
Please disregard the patches in my last email. A last minute change
introduced an pretty bad error into them. Basically I had noticed that
the first time a page is fetched the spacing between inline images is
not write. So I tried some changes and I don't know how but they seemed
to work so I sent them. However they caused an error when starting a new
Emacs session. I have not found the correct fix and done it in such a
way that if the option for maximum size of inline images is not set the
old code path is followed, so existing behavior will not be changed by
default. I have used this fix for couple of days and it works as
expected.
I have attached updated set of patches with this email.
Thank you,
Rahguzar
Rahguzar <rahguzar <at> zohomail.eu> writes:
> Hi Eli,
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>> Thanks. Rahguzar, any followup to these comments?
>
> I replied to the comments. Please let me know what is the preferred way
> of preferred way of proceeding in that case.
>
>> 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.
>
> Thanks! I have reworded all log messages. Hopefully they confirm to the
> standards better now.
>
>>> +(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.
>
> Fixed.
>
>>> +(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.
>
> Fixed.
>
>>> +(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"
>
> Fixed.
>
>>> +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?
>
> I meant 'trailing space'. Fixed now.
>
> Sorry for the typos and thanks for catching them.
>
>>> * 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.
>
> Fixed.
>
>> Thanks.
>
> I have attached the updated patches.
>
> Thank you,
> Rahguzar
>
> [2. text/x-patch; 0001-Make-some-aspects-of-shr-rendering-customizable.patch]...
>
> [3. text/x-patch; 0002-Allow-displaying-images-inline.patch]...
>
> [4. text/x-patch; 0003-Outline-support-for-shr-rendered-documents.patch]...
>
> [5. text/x-patch; 0004-Optionally-turn-on-visual-line-mode-outline-support.patch]...
>
> [6. text/x-patch; 0005-Don-t-insert-subscript-on-a-newline.patch]...
[0001-Make-some-aspects-of-shr-rendering-customizable.patch (text/x-patch, attachment)]
[0002-Allow-displaying-images-inline.patch (text/x-patch, attachment)]
[0003-Outline-support-for-shr-rendered-documents.patch (text/x-patch, attachment)]
[0004-Optionally-turn-on-visual-line-mode-outline-support.patch (text/x-patch, attachment)]
[0005-Don-t-insert-subscript-on-a-newline.patch (text/x-patch, attachment)]
This bug report was last modified 1 year and 259 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.