Subject: 28.0.50; [ELPA] adaptive-wrap: Fontify wrap-prefix

From: Kévin Le Gouguec <[email-masked]>
Date: Thu, 11 Jun 2020 18:16:40 +0200

Hello,

I really enjoy the adaptive-wrap package, I find that it usually makes
long lines significantly more legible than plain visual-line-mode.

In some situations though, I think the wrap prefix could be improved.

1. When the fill prefix does not end with a space and extra-indent > 0
======================================================================

When extra indent is requested, the code uses the last character of
(fill-context-prefix beg end) as the padding character.

E.g. in *scratch*, from emacs -Q -rv:

#+begin_src elisp
(progn
  (package-initialize)
  (setq adaptive-wrap-extra-indent 2)
  (visual-line-mode)
  (adaptive-wrap-prefix-mode))
;;Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. 
#+end_src

See first screenshot.

The comment's continuation line starts with ";;;;".  I see two problems
with this:

1. The padding characters are not propertized, so we get two fontified
   semicolons, then two unfontified semicolons.

2. Visually, this looks sort of cluttered.  I have searched Debbugs and
   emacs-devel for a rationale for using (substring fcp -1) instead of
   unconditionally using spaces, but I could not find any.

Should we simplify adaptive-wrap-fill-context-prefix to stop bothering
with (substring fcp -1) and simply use " " for extra-indent?  If not,
should we at least propertize the padding characters, applying whatever
properties we find on (substring fcp -1)?

I can produce patches for either approach; I'd like to know what we'd
prefer first.

2. When wrapping lines that have an :extend t face
==================================================

See second screenshot, taken from emacs -Q -rv, with:

#+begin_src elisp
(progn
  (package-initialize)
  (global-visual-line-mode)
  (setq visual-line-fringe-indicators '(left-curly-arrow right-curly-arrow))
  (add-hook 'visual-line-mode-hook 'adaptive-wrap-prefix-mode)
  (setq-default adaptive-wrap-extra-indent 2))
#+end_src

(Ignore the fact that continuation lines are indented much further for
removed lines than for added lines, that's because adaptive-fill-regexp
matches -'s but not +'s.)

One of the reasons to use :extend t faces, IMO, is to make it easier for
the eye to delineate horizontal chunks (e.g. "hunk headers", "added
lines", "removed lines" in diff-mode).  The unfontified prefix inserted
by adaptive-wrap makes the overall result visually confusing to me.

In these situations I'd like the wrap prefix to have the same background
color as the extended background: see third screenshot.

I can't think of a robust way to determine this color though.  In these
diff-mode examples, the string returned by fill-context-prefix has no
properties, so we can't use that as source of truth.

As demonstrated in the screenshot, we could look at the face at EOL and
slap that onto the wrap prefix if it has :extend t, but I wonder if
there could be situations where this heuristic would break down[1].

I'm not suggesting to apply the patch shown in the screenshots as-is[2];
I just cobbled it up to show what I'd like the wrap-prefix to look like.

Let me know if any of what I wrote was unclear or confusing.  I would be
more than happy to work on patches for both issues; I'd just like to
know what resolution people would prefer for the first issue, and what
traps I should watch out for with the second issue.

Thank you for your time.

PS: Congratulations on the Debian inclusion ;)

https://lists.debian.org/debian-devel/2020/06/msg00065.html
https://salsa.debian.org/emacsen-team/adaptive-wrap-el

[1] E.g.

    here is a line that is wrapped,  ⤸
⤹     its continuation lines indented⤸
⤹     with 2 extra-indent spaces.

If only the final chars "spaces.\n" have an :extend t face, and we
naively apply (plist-get (text-properties-at (1- end)) 'face) onto the
wrap prefix, I guess the overall result could look psychedelic:
unardorned first line, then colored wrap-prefix, then unadorned
continuation line, colored wrap-prefix again, unadorned continuation
line again, then the final word, with a background that extends beyond
EOL.

(Maybe the answer is simply that :extend t faces are generally applied
to the whole line, and we shouldn't worry about such hypothetical
situations…  Those sound like famous-last-words material though.)

(I tried to materialize this hypothetical bug, to no avail.)

[2] Though here it is if anyone wants to comment:

diff --git a/packages/adaptive-wrap/adaptive-wrap.el b/packages/adaptive-wrap/adaptive-wrap.el
index f8d89ac69..5a23e6d6b 100644
--- a/packages/adaptive-wrap/adaptive-wrap.el
+++ b/packages/adaptive-wrap/adaptive-wrap.el
@@ -81,8 +81,12 @@ extra indent = 2
      ((= 0 adaptive-wrap-extra-indent)
       fcp)
      ((< 0 adaptive-wrap-extra-indent)
-      (concat fcp
-              (make-string adaptive-wrap-extra-indent fill-char)))
+      (let ((face (plist-get (text-properties-at (1- end)) 'face))
+	    (prefix (concat fcp
+			    (make-string adaptive-wrap-extra-indent fill-char))))
+	(if (and face (face-extend-p face))
+	    (propertize prefix 'face face)
+	  prefix)))
      ((< 0 (+ adaptive-wrap-extra-indent fcp-len))
       (substring fcp
                  0

In GNU Emacs 28.0.50 (build 32, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.16.0)
 of 2020-05-30 built on my-little-tumbleweed
Repository revision: 780f674a82a90c4e3e32583059b498bfa57e4a06
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: openSUSE Tumbleweed

Configured using:
 'configure --with-xwidgets --with-cairo'

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

Important settings:
  value of $LC_CTYPE: en_US.UTF-8
  value of $LC_TIME: en_GB.UTF-8
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=local
  locale-coding-system: utf-8-unix