GNU bug report logs - #41810
28.0.50; [ELPA] adaptive-wrap: Fontify wrap-prefix

Previous Next

Full log

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 41810 <at> debbugs.gnu.org, Stephen Berman <stephen.berman <at> gmx.net>
Subject: Re: bug#41810: 28.0.50; [ELPA] adaptive-wrap: Fontify wrap-prefix
Date: Sat, 13 Jun 2020 00:48:09 +0200

[Message part 1 (text/plain, inline)]

(Hit reply instead of followup; apologies for the duplicate, Stefan)

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> I think I narrowed it down to this condition in fill-context-prefix:
>>
>> <snip>
>>
>> In the *scratch* buffer, the condition holds true, so first-line-prefix
>> is returned, text properties and all: that's why the first two
>> semicolons are fontified.
>>
>> In a diff buffer,
>>
>> 1. for removed lines, the condition is false, so we make a new,
>>    unfontified string, without the diff-removed face,
>
> We should be able to make this work by trying to preserve
> `first-line-prefix`s text properties somehow.

I wonder if we shouldn't go the other direction?  As in, why should
fill-context-prefix bother returning text properties?  From a quick
glance at other users of this function in the Emacs source tree, AFAICT
most of them actually insert (something based on) the return value, so
fontification is updated after insertion; these users do not care about
the returned text properties.

So a conclusion could be that adaptive-wrap-f-c-p should make no
assumptions about what text properties f-c-p returns, and determine the
face… some other way (see below).

> I guess we could try and fix it in `adaptive-wrap-fill-context-prefix`
> by trying to preserve any face that covers the whole line (including the
> final newline).

Mmm… I think that won't DTRT in some cases.  In diff buffers, typically:

- the first character in a removed line has the diff-indicator-removed
  face, which some themes[1] might customize to have no background,

- the rest of the line has the diff-removed face.

> I'm glad I'm not the one who'll write the code ;-)

I certainly wouldn't mind anyone beating me to it ;D

Here's a proof-of-concept patch (which only handles the positive
extra-indent case) and some before/after screenshots.  Again, not
suggesting to apply this patch as-is; this is just to show in which
direction I'm thinking of going:

1. if (substring fcp -1) has text properties, grab that,
2. else grab some text properties from the current line,
3. slap whatever we grabbed on the whole wrap-prefix.

Step 2 is not very well-defined yet, even though the "current
implementation" works well enough for the sales-pitch screenshots.

The rationale behind propertizing the whole prefix in step 3, as
mentioned a few paragraphs above, is to stop relying on
fill-context-prefix returning text properties.

[poc.patch (text/x-patch, inline)]

diff --git a/packages/adaptive-wrap/adaptive-wrap.el b/packages/adaptive-wrap/adaptive-wrap.el
index f8d89ac69..0d9ed8b94 100644
--- a/packages/adaptive-wrap/adaptive-wrap.el
+++ b/packages/adaptive-wrap/adaptive-wrap.el
@@ -59,6 +59,11 @@ extra indent = 2
   :group 'visual-line)
 (make-variable-buffer-local 'adaptive-wrap-extra-indent)
 
+(defun adaptive-wrap--prefix-properties (fcp beg)
+  (or (and (> (string-width fcp) 0)
+           (text-properties-at 0 (substring fcp -1)))
+      (text-properties-at beg)))
+
 (defun adaptive-wrap-fill-context-prefix (beg end)
   "Like `fill-context-prefix', but with length adjusted by `adaptive-wrap-extra-indent'."
   (let* ((fcp
@@ -81,8 +86,9 @@ extra indent = 2
      ((= 0 adaptive-wrap-extra-indent)
       fcp)
      ((< 0 adaptive-wrap-extra-indent)
-      (concat fcp
-              (make-string adaptive-wrap-extra-indent fill-char)))
+      (apply #'propertize
+             (concat fcp (make-string adaptive-wrap-extra-indent fill-char))
+             (adaptive-wrap--prefix-properties fcp beg)))
      ((< 0 (+ adaptive-wrap-extra-indent fcp-len))
       (substring fcp
                  0

[Message part 3 (text/plain, inline)]

The screenshots show improvements for the diff buffer (all leading
whitespace fontified) and for the comments in the scratch buffer (all
semicolons fontified).

[before.png (image/png, attachment)]

[after.png (image/png, attachment)]

[Message part 6 (text/plain, inline)]

Thank you for your time.  I'll try to followup with a more complete
patch Soonish™ (though I'm not sure what heuristic I'll use for step 2
yet).


[1] https://gitlab.com/peniblec/eighters-theme/-/blob/55710346/eighters-theme.el#L53

Previous Next

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