Subject: Re: bug#59141: 28.1.90; Face :extend when all the line but trailing \n is invisible

From: Kévin Le Gouguec <[email-masked]>
Date: Mon, 14 Nov 2022 23:22:11 +0100

Juri Linkov <[email-masked]> writes:

>> My point with this comparison is to show that an outline-like UI with
>> :extended backgrounds is obviously possible; in my previous messages, I
>> tried to highlight the relevant code in magit-section that handles
>> delimiting section headings vs content and setting the overlays.
>>
>> I did that mainly FTR, so that Someone™ with motivation and time can see
>> if outline.el could grow a user option to support a similar way to
>> display outlines, thus solving the problem of :extended backgrounds.
>
> I haven't looked at the magit-section source code.  I once tried
> to copy the syntax highlighting code from diff-mode to magit-diff,
> but magit code is such a mess that I abandoned the attempt.
>
> But from your screenshots it's clear what is needed to do
> to achieve the same in outline(-minor)-mode:

I don't think I found anything particularly messy about magit-section,
but piecing together the logic responsible for delimiting section
headings and content certainly took me enough time that I felt it was
worth summarizing.

Glad the screenshots helped.

> 3. Your screenshot shows that magit doesn't use an ellipsis.
> And indeed, ellipses get in the way.  But we need to find a way
> to disable them without breaking this feature.  The line
>
>   (overlay-put o 'invisible 'outline)
>
> either should be replaced with
>
>   (overlay-put o 'invisible t)
>
> or the ellipsis glyph to be disabled with something like:
>
>   (or standard-display-table (setq standard-display-table (make-display-table)))
>   (set-char-table-extra-slot standard-display-table 4 (vector))

As I mentioned, magit supports replacing its fringe bitmaps with
ellipses.  See the magit-section-visibility-indicator user option, and
the functions that act on it[1].  tl;dr the option can be set to

* (SYMBOL1 . SYMBOL2): each symbol describes a fringe bitmap to be used
  for expandable (1) or collapsible (2) sections,

* (STRING . BOOLEAN): STRING is appended at the end of collapsed
  sections.

IIUC magit-section sets these ellipses via 'after-string overlays on the
last character before a heading's newline, rather than adjusting display
tables.

Thanks for weighing in on how outline.el could be adapted, Juri.  Can't
promise I'll be able to act on your advice and turn it into a patch
anytime soon, but finally hashing out the implementation details between
these two packages has made me more hopeful outline.el can be taught
more tricks without resorting to ugly hacks.

[1]

  (defcustom magit-section-visibility-indicator
    (if (window-system)
        '(magit-fringe-bitmap> . magit-fringe-bitmapv)
      (cons (if (char-displayable-p ?…) "…" "...")
            t))
    "Whether and how to indicate that a section can be expanded/collapsed.

  If nil, then don't show any indicators.
  Otherwise the value has to have one of these two forms:

  \(EXPANDABLE-BITMAP . COLLAPSIBLE-BITMAP)

    Both values have to be variables whose values are fringe
    bitmaps.  In this case every section that can be expanded or
    collapsed gets an indicator in the left fringe.

    To provide extra padding around the indicator, set
    `left-fringe-width' in `magit-mode-hook'.

  \(STRING . BOOLEAN)

    In this case STRING (usually an ellipsis) is shown at the end
    of the heading of every collapsed section.  Expanded sections
    get no indicator.  The cdr controls whether the appearance of
    these ellipsis take section highlighting into account.  Doing
    so might potentially have an impact on performance, while not
    doing so is kinda ugly."
    :package-version '(magit-section . "3.0.0")
    :group 'magit-section
    :type '(choice (const :tag "No indicators" nil)
                   (cons  :tag "Use +- fringe indicators"
                          (const magit-fringe-bitmap+)
                          (const magit-fringe-bitmap-))
                   (cons  :tag "Use >v fringe indicators"
                          (const magit-fringe-bitmap>)
                          (const magit-fringe-bitmapv))
                   (cons  :tag "Use bold >v fringe indicators)"
                          (const magit-fringe-bitmap-bold>)
                          (const magit-fringe-bitmap-boldv))
                   (cons  :tag "Use custom fringe indicators"
                          (variable :tag "Expandable bitmap variable")
                          (variable :tag "Collapsible bitmap variable"))
                   (cons  :tag "Use ellipses at end of headings"
                          (string :tag "Ellipsis" "…")
                          (choice :tag "Use face kludge"
                                  (const :tag "Yes (potentially slow)" t)
                                  (const :tag "No (kinda ugly)" nil)))))

  (defun magit-section-maybe-update-visibility-indicator (section)
    (when (and magit-section-visibility-indicator
               (magit-section-content-p section))
      (let* ((beg (oref section start))
             (eoh (save-excursion
                    (goto-char beg)
                    (line-end-position))))
        (cond
         ((symbolp (car-safe magit-section-visibility-indicator))
          (let ((ov (magit--overlay-at beg 'magit-vis-indicator 'fringe)))
            (unless ov
              (setq ov (make-overlay beg eoh nil t))
              (overlay-put ov 'evaporate t)
              (overlay-put ov 'magit-vis-indicator 'fringe))
            (overlay-put
             ov 'before-string
             (propertize "fringe" 'display
                         (list 'left-fringe
                               (if (oref section hidden)
                                   (car magit-section-visibility-indicator)
                                 (cdr magit-section-visibility-indicator))
                               'fringe)))))
         ((stringp (car-safe magit-section-visibility-indicator))
          (let ((ov (magit--overlay-at (1- eoh) 'magit-vis-indicator 'eoh)))
            (cond ((oref section hidden)
                   (unless ov
                     (setq ov (make-overlay (1- eoh) eoh))
                     (overlay-put ov 'evaporate t)
                     (overlay-put ov 'magit-vis-indicator 'eoh))
                   (overlay-put ov 'after-string
                                (car magit-section-visibility-indicator)))
                  (ov
                   (delete-overlay ov)))))))))

  (defvar-local magit--ellipses-sections nil)

  (defun magit-section-maybe-paint-visibility-ellipses ()
    ;; This is needed because we hide the body instead of "the body
    ;; except the final newline and additionally the newline before
    ;; the body"; otherwise we could use `buffer-invisibility-spec'.
    (when (stringp (car-safe magit-section-visibility-indicator))
      (let* ((sections (append magit--ellipses-sections
                               (setq magit--ellipses-sections
                                     (or (magit-region-sections)
                                         (list (magit-current-section))))))
             (beg (--map (oref it start) sections))
             (end (--map (oref it end)   sections)))
        (when (region-active-p)
          ;; This ensures that the region face is removed from ellipses
          ;; when the region becomes inactive, but fails to ensure that
          ;; all ellipses within the active region use the region face,
          ;; because the respective overlay has not yet been updated at
          ;; this time.  The magit-selection face is always applied.
          (push (region-beginning) beg)
          (push (region-end)       end))
        (setq beg (apply #'min beg))
        (setq end (apply #'max end))
        (dolist (ov (overlays-in beg end))
          (when (eq (overlay-get ov 'magit-vis-indicator) 'eoh)
            (overlay-put
             ov 'after-string
             (propertize
              (car magit-section-visibility-indicator) 'font-lock-face
              (let ((pos (overlay-start ov)))
                (delq nil (nconc (--map (overlay-get it 'font-lock-face)
                                        (overlays-at pos))
                                 (list (get-char-property
                                        pos 'font-lock-face))))))))))))

  (defun magit-section-maybe-remove-visibility-indicator (section)
    (when (and magit-section-visibility-indicator
               (= (oref section content)
                  (oref section end)))
      (dolist (o (overlays-in (oref section start)
                              (save-excursion
                                (goto-char (oref section start))
                                (1+ (line-end-position)))))
        (when (overlay-get o 'magit-vis-indicator)
          (delete-overlay o)))))