GNU bug report logs - #69555
30.0.50; shr - Preserve indentation when shr-fill-text is nil

Previous Next

Package: emacs;

Reported by: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>

Date: Mon, 4 Mar 2024 22:04:01 UTC

Severity: normal

Found in version 30.0.50

Done: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 69555 in the body.
You can then email your comments to 69555 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to rahguzar <at> zohomail.eu, bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Mon, 04 Mar 2024 22:04:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Kévin Le Gouguec <kevin.legouguec <at> gmail.com>:
New bug report received and forwarded. Copy sent to rahguzar <at> zohomail.eu, bug-gnu-emacs <at> gnu.org. (Mon, 04 Mar 2024 22:04:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; shr - Preserve indentation when shr-fill-text is nil
Date: Mon, 04 Mar 2024 23:02:27 +0100
[Message part 1 (text/plain, inline)]
Hello,

I am a grateful and happy user of the new shr-fill-text option; one nit
I have with its current implementation is that, since setting it to nil
essentially short-circuits shr-fill-lines, we skip shr-fill-line, which
actually handles both filling *and* indentation.

This is very noticeable when reading HTML emails with Gnus: citations in
blockquotes become indistinguishable from the actual reply.  See
eww-current.png for a demonstration with EWW: 

* left is shr-fill-text t (default),
* right is shr-fill-text nil and visual-line-fringe-indicators
  '(left-curly-arrow right-curly-arrow).

The attached patch makes it so that shr applies indentation before it
consult the shr-fill-text option and skips filling.  See eww-patch.png
for a demonstration: 

* left is shr-fill-text nil and visual-line-fringe-indicators
  '(left-curly-arrow right-curly-arrow),
* right is the same, plus visual-wrap-prefix-mode,
* not pictured (unchanged): shr-fill-text t (default).

This patch also adds a rendering testcase, and teaches the rendering
test to re-run testcases while setting extra user options to make sure
they do not impact rendering.

Curious to hear your thoughts.  Will post an updated patch once I have a
bug number to replace the "bug#TODO" placeholders.

Thanks!


[0001-Keep-indenting-text-when-shr-fill-text-is-nil-bug-TO.patch (text/x-patch, attachment)]
[eww-current.png (image/png, attachment)]
[eww-patch.png (image/png, attachment)]
[Message part 5 (text/plain, inline)]

In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.39, cairo version 1.18.0) of 2024-01-06 built on amdahl30
Repository revision: 657275529e31226bbc6c92eb7f7af887474a0bb8
Repository branch: master
Windowing system distributor 'SUSE LINUX', version 11.0.12302004
System Description: openSUSE Tumbleweed

Configured using:
 'configure --prefix=/home/peniblec/apps/emacs --with-cairo
 --with-sqlite3 --with-xinput2'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Mon, 04 Mar 2024 22:11:02 GMT) Full text and rfc822 format available.

Message #8 received at 69555 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: 69555 <at> debbugs.gnu.org
Cc: Rahguzar <rahguzar <at> zohomail.eu>
Subject: Re: bug#69555: 30.0.50; shr - Preserve indentation when
 shr-fill-text is nil
Date: Mon, 04 Mar 2024 23:08:20 +0100
[Message part 1 (text/plain, inline)]
Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

>                                 Will post an updated patch once I have a
> bug number to replace the "bug#TODO" placeholders.

See attached.

[0001-Keep-indenting-text-when-shr-fill-text-is-nil-bug-69.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Tue, 05 Mar 2024 12:10:02 GMT) Full text and rfc822 format available.

Message #11 received at 69555 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 69555 <at> debbugs.gnu.org, rahguzar <at> zohomail.eu
Subject: Re: bug#69555: 30.0.50;
 shr - Preserve indentation when shr-fill-text is nil
Date: Tue, 05 Mar 2024 14:09:01 +0200
> Cc: Rahguzar <rahguzar <at> zohomail.eu>
> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Date: Mon, 04 Mar 2024 23:08:20 +0100
> 
> Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
> 
> >                                 Will post an updated patch once I have a
> > bug number to replace the "bug#TODO" placeholders.
> 
> See attached.

The change in behavior is unconditional, AFAIU.  Should we have a
separate knob for that?  Perhaps someone out there doesn't want text
to be indented when shr-fill-text is nil?

Also, please time the code on some substantially large body of text,
with and without shr-fill-text, and compare that with the current
version.  I think performance is an important aspect of any change in
this area.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Tue, 05 Mar 2024 19:25:01 GMT) Full text and rfc822 format available.

Message #14 received at 69555 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 69555 <at> debbugs.gnu.org, rahguzar <at> zohomail.eu
Subject: Re: bug#69555: 30.0.50; shr - Preserve indentation when
 shr-fill-text is nil
Date: Tue, 05 Mar 2024 20:22:52 +0100
Eli Zaretskii <eliz <at> gnu.org> writes:

>> Cc: Rahguzar <rahguzar <at> zohomail.eu>
>> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
>> Date: Mon, 04 Mar 2024 23:08:20 +0100
>> 
>> Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
>> 
>> >                                 Will post an updated patch once I have a
>> > bug number to replace the "bug#TODO" placeholders.
>> 
>> See attached.
>
> The change in behavior is unconditional, AFAIU.  Should we have a
> separate knob for that?  Perhaps someone out there doesn't want text
> to be indented when shr-fill-text is nil?

Perhaps; I could look into adding that knob.  FWIW nothing in the
original feature request (bug#66676) gives me the impression that this
new option should affect indentation; the stated motivation was:

  > 1) Using `visual-line-mode` for line wrapping. I think this is more
  > natural for html and makes resizing windows work more nicely.

That's why I interpret the indentation change as a side-effect; curious
to hear Rahguzar's thoughts though.

Another reason I'm not overly fond of adding another knob is figuring
out what should happen if a user keeps shr-fill-text set to t, but sets
that hypothetical indentation option to nil.  Should we support that?
Or should shr-fill-text become a tri-state?  (nil, 'indent-only, t)

I'm open to biting the bullet, but I'd also like to make sure we don't
sophisticate shr.el further than we want to.

> Also, please time the code on some substantially large body of text,
> with and without shr-fill-text, and compare that with the current
> version.  I think performance is an important aspect of any change in
> this area.

Can do; would "(elisp) Profiling" be the starting point?  (For all my
list lurking, I confess my eyes have always glossed over discussions
involving benchmarks)

(Also wondering if we have any "standard" or preferred HTML documents or
websites to throw at shr.el for benchmarking purposes; if not, I guess
I'll peruse <https://en.wikipedia.org/wiki/Special:LongPages> 🤔)


Thanks for the review!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Tue, 05 Mar 2024 19:49:02 GMT) Full text and rfc822 format available.

Message #17 received at 69555 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 69555 <at> debbugs.gnu.org, rahguzar <at> zohomail.eu
Subject: Re: bug#69555: 30.0.50; shr - Preserve indentation when
 shr-fill-text is nil
Date: Tue, 05 Mar 2024 21:47:09 +0200
> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: 69555 <at> debbugs.gnu.org,  rahguzar <at> zohomail.eu
> Date: Tue, 05 Mar 2024 20:22:52 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > Also, please time the code on some substantially large body of text,
> > with and without shr-fill-text, and compare that with the current
> > version.  I think performance is an important aspect of any change in
> > this area.
> 
> Can do; would "(elisp) Profiling" be the starting point?

I think benchmark-run is a better starting point.

> (Also wondering if we have any "standard" or preferred HTML documents or
> websites to throw at shr.el for benchmarking purposes; if not, I guess
> I'll peruse <https://en.wikipedia.org/wiki/Special:LongPages> 🤔)

Actually, something with a lot of text in large paragraphs, sometimes
indented, would be better.  Those Wikipedia pages are basically long
lists, but there's not much opportunity there to perform filling and
indentation of large amounts of text, which is what is sought here.
Here's one possible candidate:

  https://debbugs.gnu.org/Developer.html





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Tue, 05 Mar 2024 23:19:02 GMT) Full text and rfc822 format available.

Message #20 received at 69555 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 69555 <at> debbugs.gnu.org, rahguzar <at> zohomail.eu
Subject: Re: bug#69555: 30.0.50; shr - Preserve indentation when
 shr-fill-text is nil
Date: Wed, 06 Mar 2024 00:16:53 +0100
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
>> Cc: 69555 <at> debbugs.gnu.org,  rahguzar <at> zohomail.eu
>> Date: Tue, 05 Mar 2024 20:22:52 +0100
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> > Also, please time the code on some substantially large body of text,
>> > with and without shr-fill-text, and compare that with the current
>> > version.  I think performance is an important aspect of any change in
>> > this area.
>> 
>> Can do; would "(elisp) Profiling" be the starting point?
>
> I think benchmark-run is a better starting point.
>
>> (Also wondering if we have any "standard" or preferred HTML documents or
>> websites to throw at shr.el for benchmarking purposes; if not, I guess
>> I'll peruse <https://en.wikipedia.org/wiki/Special:LongPages> 🤔)
>
> Actually, something with a lot of text in large paragraphs, sometimes
> indented, would be better.  Those Wikipedia pages are basically long
> lists, but there's not much opportunity there to perform filling and
> indentation of large amounts of text, which is what is sought here.
> Here's one possible candidate:
>
>   https://debbugs.gnu.org/Developer.html

Thanks for the pointers.  Attaching the over-engineered scripts I built
from that, which with 1000 REPETITIONS yield:

2024-03-05; 33976ecf244; 30.0.50; master          shr-fill-text=nil  ( 3.013 23  0.313)
2024-03-04; b06916cb218; 30.0.50; shr-blockquote  shr-fill-text=nil  ( 3.121 24  0.328)

2024-03-05; 33976ecf244; 30.0.50; master          shr-fill-text=t    (32.331 65  0.904)
2024-03-04; b06916cb218; 30.0.50; shr-blockquote  shr-fill-text=t    (32.045 65  0.902)

I can bump up REPETITIONS if that would help; sending the scripts &
results as-is before hitting the hay since I figure they might have some
glaring methodology issues, or there is more information I might not
have thought of reporting.

If not, the tentative conclusion would be "shr-fill-text nil gets 4%
slower; shr-fill-text t is none the worse for wear; nil still runs
circles around t"?

[bench.el (text/x-emacs-lisp, attachment)]
[bench.sh (application/x-shellscript, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Wed, 06 Mar 2024 07:32:02 GMT) Full text and rfc822 format available.

Message #23 received at 69555 <at> debbugs.gnu.org (full text, mbox):

From: Rahguzar <rahguzar <at> zohomail.eu>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 69555 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#69555: 30.0.50; shr - Preserve indentation when
 shr-fill-text is nil
Date: Wed, 06 Mar 2024 08:27:50 +0100
Hi Kévin,

Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> Perhaps; I could look into adding that knob.  FWIW nothing in the
> original feature request (bug#66676) gives me the impression that this
> new option should affect indentation; the stated motivation was:
>
>   > 1) Using `visual-line-mode` for line wrapping. I think this is more
>   > natural for html and makes resizing windows work more nicely.
>
> That's why I interpret the indentation change as a side-effect; curious
> to hear Rahguzar's thoughts though.

You are right that losing indentation was not intentional and an
oversight on my part. Thanks for the fix. I will test it soon.

Rahguzar




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Wed, 06 Mar 2024 11:55:01 GMT) Full text and rfc822 format available.

Message #26 received at 69555 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 69555 <at> debbugs.gnu.org, rahguzar <at> zohomail.eu
Subject: Re: bug#69555: 30.0.50; shr - Preserve indentation when
 shr-fill-text is nil
Date: Wed, 06 Mar 2024 13:53:22 +0200
> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: 69555 <at> debbugs.gnu.org,  rahguzar <at> zohomail.eu
> Date: Wed, 06 Mar 2024 00:16:53 +0100
> 
> If not, the tentative conclusion would be "shr-fill-text nil gets 4%
> slower; shr-fill-text t is none the worse for wear; nil still runs
> circles around t"?

Thanks, I think 4% slowdown is negligible in this case.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Wed, 06 Mar 2024 21:20:01 GMT) Full text and rfc822 format available.

Message #29 received at 69555 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 69555 <at> debbugs.gnu.org, rahguzar <at> zohomail.eu
Subject: Re: bug#69555: 30.0.50; shr - Preserve indentation when
 shr-fill-text is nil
Date: Wed, 06 Mar 2024 22:18:19 +0100
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
>> Cc: 69555 <at> debbugs.gnu.org,  rahguzar <at> zohomail.eu
>> Date: Wed, 06 Mar 2024 00:16:53 +0100
>> 
>> If not, the tentative conclusion would be "shr-fill-text nil gets 4%
>> slower; shr-fill-text t is none the worse for wear; nil still runs
>> circles around t"?
>
> Thanks, I think 4% slowdown is negligible in this case.

ACK.

Re. indentation being done unconditionally…

Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>> The change in behavior is unconditional, AFAIU.  Should we have a
>> separate knob for that?  Perhaps someone out there doesn't want text
>> to be indented when shr-fill-text is nil?
>
> Perhaps; I could look into adding that knob.  FWIW nothing in the
> original feature request (bug#66676) gives me the impression that this
> new option should affect indentation; the stated motivation was:
>
>   > 1) Using `visual-line-mode` for line wrapping. I think this is more
>   > natural for html and makes resizing windows work more nicely.
>
> That's why I interpret the indentation change as a side-effect; curious
> to hear Rahguzar's thoughts though.

… considering Rahguzar's reply (thanks for that, and for the offer to
try the patch 🙏)…

> Another reason I'm not overly fond of adding another knob is figuring
> out what should happen if a user keeps shr-fill-text set to t, but sets
> that hypothetical indentation option to nil.  Should we support that?
> Or should shr-fill-text become a tri-state?  (nil, 'indent-only, t)
>
> I'm open to biting the bullet, but I'd also like to make sure we don't
> sophisticate shr.el further than we want to.

… do we think there is a case for an option to disable indentation then?
Or should we leave that for a future feature request?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Thu, 07 Mar 2024 06:40:01 GMT) Full text and rfc822 format available.

Message #32 received at 69555 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 69555 <at> debbugs.gnu.org, rahguzar <at> zohomail.eu
Subject: Re: bug#69555: 30.0.50; shr - Preserve indentation when
 shr-fill-text is nil
Date: Thu, 07 Mar 2024 08:39:10 +0200
> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: 69555 <at> debbugs.gnu.org,  rahguzar <at> zohomail.eu
> Date: Wed, 06 Mar 2024 22:18:19 +0100
> 
> … do we think there is a case for an option to disable indentation then?

I guess not.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Wed, 13 Mar 2024 19:57:02 GMT) Full text and rfc822 format available.

Message #35 received at 69555 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Rahguzar <rahguzar <at> zohomail.eu>
Cc: 69555 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#69555: 30.0.50; shr - Preserve indentation when
 shr-fill-text is nil
Date: Wed, 13 Mar 2024 20:55:09 +0100
Hey Rahguzar,

Rahguzar <rahguzar <at> zohomail.eu> writes:

> Hi Kévin,
>
> Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
>
>> Perhaps; I could look into adding that knob.  FWIW nothing in the
>> original feature request (bug#66676) gives me the impression that this
>> new option should affect indentation; the stated motivation was:
>>
>>   > 1) Using `visual-line-mode` for line wrapping. I think this is more
>>   > natural for html and makes resizing windows work more nicely.
>>
>> That's why I interpret the indentation change as a side-effect; curious
>> to hear Rahguzar's thoughts though.
>
> You are right that losing indentation was not intentional and an
> oversight on my part. Thanks for the fix. I will test it soon.

No rush or anything; it just occurs to me that if you're busy with
Things™, please do not hesitate to delegate the testing: if you have
specific use-cases or webpages in mind that you'd want to check the
patch against, don't hesitate to tell me about them, I can take a look
and report how things pan out.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Wed, 13 Mar 2024 20:32:02 GMT) Full text and rfc822 format available.

Message #38 received at 69555 <at> debbugs.gnu.org (full text, mbox):

From: Rahguzar <rahguzar <at> zohomail.eu>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 69555 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#69555: 30.0.50; shr - Preserve indentation when
 shr-fill-text is nil
Date: Wed, 13 Mar 2024 21:28:04 +0100
Hi Kévin,

Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:

> No rush or anything; it just occurs to me that if you're busy with
> Things™, please do not hesitate to delegate the testing: if you have
> specific use-cases or webpages in mind that you'd want to check the
> patch against, don't hesitate to tell me about them, I can take a look
> and report how things pan out.

Sorry, this just slipped out of my mind. But I have looked at the patch
and also tested it and it is a definite improvement. I hadn't realized
why some html emails looked a bit jumbled up so restoring the
indentation fixes them. Thanks a lot for it.

Rahguzar




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Wed, 13 Mar 2024 21:44:01 GMT) Full text and rfc822 format available.

Message #41 received at 69555 <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Rahguzar <rahguzar <at> zohomail.eu>
Cc: 69555 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#69555: 30.0.50; shr - Preserve indentation when
 shr-fill-text is nil
Date: Wed, 13 Mar 2024 22:41:32 +0100
Rahguzar <rahguzar <at> zohomail.eu> writes:

> Hi Kévin,
>
> Kévin Le Gouguec <kevin.legouguec <at> gmail.com> writes:
>
>> No rush or anything; it just occurs to me that if you're busy with
>> Things™, please do not hesitate to delegate the testing: if you have
>> specific use-cases or webpages in mind that you'd want to check the
>> patch against, don't hesitate to tell me about them, I can take a look
>> and report how things pan out.
>
> Sorry, this just slipped out of my mind.

No worries 👌

>                                          But I have looked at the patch
> and also tested it and it is a definite improvement. I hadn't realized
> why some html emails looked a bit jumbled up so restoring the
> indentation fixes them.

Yay!

>                         Thanks a lot for it.

Thank _you_ for getting the ball rolling with shr-fill-text in the first
place!


Eli, am I clear to push & close?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Thu, 14 Mar 2024 05:06:02 GMT) Full text and rfc822 format available.

Message #44 received at 69555 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: rahguzar <at> zohomail.eu, 69555 <at> debbugs.gnu.org
Subject: Re: bug#69555: 30.0.50; shr - Preserve indentation when
 shr-fill-text is nil
Date: Thu, 14 Mar 2024 07:04:31 +0200
> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: 69555 <at> debbugs.gnu.org,  Eli Zaretskii <eliz <at> gnu.org>
> Date: Wed, 13 Mar 2024 22:41:32 +0100
> 
> Eli, am I clear to push & close?

Yes, but please fix the following nits before installing:

  . use only ASCII characters in the commit log message
  . make sure lines in the commit log message do not exceed 65 columns

Thanks.




Reply sent to Kévin Le Gouguec <kevin.legouguec <at> gmail.com>:
You have taken responsibility. (Fri, 15 Mar 2024 07:12:02 GMT) Full text and rfc822 format available.

Notification sent to Kévin Le Gouguec <kevin.legouguec <at> gmail.com>:
bug acknowledged by developer. (Fri, 15 Mar 2024 07:12:02 GMT) Full text and rfc822 format available.

Message #49 received at 69555-done <at> debbugs.gnu.org (full text, mbox):

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rahguzar <at> zohomail.eu, 69555-done <at> debbugs.gnu.org
Subject: Re: bug#69555: 30.0.50; shr - Preserve indentation when
 shr-fill-text is nil
Date: Fri, 15 Mar 2024 08:10:09 +0100
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
>> Cc: 69555 <at> debbugs.gnu.org,  Eli Zaretskii <eliz <at> gnu.org>
>> Date: Wed, 13 Mar 2024 22:41:32 +0100
>> 
>> Eli, am I clear to push & close?
>
> Yes, but please fix the following nits before installing:
>
>   . use only ASCII characters in the commit log message

(ACK; FWIW I did check the repo log for a precedent when adding those
ellipses.  Wondering if build-aux/git-hooks/commit-msg ought to report
this?  It does seem to explicitly check for non-UTF-8)

>   . make sure lines in the commit log message do not exceed 65 columns

(ACK; can't remember where I got my 70-odd filling came from, CONTRIBUTE
does explicitly mention "63".  Maybe I got confused with the concurrent
discussion on emacs-devel)

> Thanks.

Nits picked, patch pushed, closing.  Thank you both for the review!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69555; Package emacs. (Fri, 15 Mar 2024 08:50:02 GMT) Full text and rfc822 format available.

Message #52 received at 69555-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: rahguzar <at> zohomail.eu, 69555-done <at> debbugs.gnu.org
Subject: Re: bug#69555: 30.0.50; shr - Preserve indentation when
 shr-fill-text is nil
Date: Fri, 15 Mar 2024 10:48:53 +0200
> From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
> Cc: rahguzar <at> zohomail.eu,  69555-done <at> debbugs.gnu.org
> Date: Fri, 15 Mar 2024 08:10:09 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >   . use only ASCII characters in the commit log message
> 
> (ACK; FWIW I did check the repo log for a precedent when adding those
> ellipses.  Wondering if build-aux/git-hooks/commit-msg ought to report
> this?  It does seem to explicitly check for non-UTF-8)

I'm against making our commit hooks more strict, as they are already
quite a nuisance in some situations, for example, when merging from
the release branch.  It isn't a catastrophe when UTF-8 encoded
non-ASCII characters snick into our logs, so it's a soft preference,
not something we should enforce by rejecting commits.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Fri, 12 Apr 2024 11:24:26 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 126 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.