[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#41810: [PATCH][ELPA] adaptive-wrap: Fontify wrap-prefix
From: |
Basil L. Contovounesios |
Subject: |
bug#41810: [PATCH][ELPA] adaptive-wrap: Fontify wrap-prefix |
Date: |
Sun, 21 Jun 2020 19:32:17 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:
> OK, here is a patch that I think should be good to push, tested against
> Emacs 28 and 26.3.
Thanks, just some minor comments from me.
> +(defun adaptive-wrap--prefix-face (fcp beg end)
> + (or (get-text-property 0 'face fcp)
> + ;; If the last character is a newline and has a face that
> + ;; extends beyond EOL, assume that this face spans the whole
> + ;; line and apply it to the prefix to preserve the "block"
> + ;; visual effect.
> + ;; NB: the face might not actually span the whole line: see for
> + ;; example removed lines in diff-mode, where the first character
> + ;; has the diff-indicator-removed face, while the rest of the
> + ;; line has the diff-removed face.
> + (when (= (char-before end) ?\n)
> + (let ((eol-face (get-text-property (1- end) 'face)))
Is it guaranteed that (< (point-min) end (1+ (point-max)))?
Otherwise = and get-text-property will barf.
> + (when (and eol-face (adaptive-wrap--face-extends eol-face))
> + eol-face)))))
Nit: Can't the when+and be replaced with a single and?
> +(defun adaptive-wrap--prefix (fcp)
> + (let ((fcp-len (string-width fcp)))
> + (cond
> + ((= 0 adaptive-wrap-extra-indent)
> + fcp)
> + ((< 0 adaptive-wrap-extra-indent)
> + (concat
> + fcp
> + (make-string adaptive-wrap-extra-indent
> + (if (< 0 fcp-len)
> + (string-to-char (substring fcp -1))
> + ?\ ))))
Please change this to ?\s regardless of whether the second patch is
installed.
> + ((< 0 (+ adaptive-wrap-extra-indent fcp-len))
> + (substring fcp
> + 0
> + (+ adaptive-wrap-extra-indent fcp-len)))
> + (t
> + ""))))
> Open questions:
>
> - The (or … (when … (let … (when (and …))))) chain looks clumsy but I
> don't really know how to improve it off the top of my head. Maybe a
> when-let or two would help? That'd mean requiring Emacs 25.1 though.
Apart from the redundant when, I think it's fine. If you really want
to shave off some forms you can write e.g.
(defun adaptive-wrap--prefix-face (fcp beg end)
(or (get-text-property 0 'face fcp)
(let ((face (and (= (char-before end) ?\n)
(get-text-property (1- end) 'face))))
(and face (adaptive-wrap--face-extends face) face))))
or
(defun adaptive-wrap--prefix-face (fcp beg end)
(cond ((get-text-property 0 'face fcp))
((= (char-before end) ?\n)
(let ((face ...))
(and face (adaptive-wrap--face-extends face) face)))))
Thanks,
--
Basil