Re: [PATCH] Highlight ANSI sequences in the whole buffer (was [PATCH] ANSI color on example blocks and fixed width elements)

[Ihor Radchenko <yantar92@posteo.net>]

Nathaniel Nicandro <nathanielnicandro@gmail.com> writes:

>> Nathaniel, may I know if you are still working on this?
>
> Hello Ihor,
>
> Yes I'm still working on this.  Attached is an updated patch with some
> tests this time.  It's still a work in progress.  Below are responses to
> your previous comments about my last update and some comments about this
> current patch.

Thanks for the update! And for the resilience working on this difficult
patch :)

>> This is very fragile.
>> I believe that hooking into `org-fold-check-before-invisible-edit'
>> would lead to simpler implementation.
>
> Thank you for the feedback.  I indeed was able to come up with a
> more simpler solution by hooking into that function.
>
> To integrate with `org-fold-check-before-invisible-edit' I had to
> introduce two variables, `org-fold-visibility-detail' which is set to
> the argument of `org-fold-show-set-visibility' when that function is
> called and `org-ansi-fontify-begin' to determine the start of the
> fontification region to see if it's close to the beginning of an
> invisible sequence that should be turned visible.
>
> Let me know if this is an OK approach.

I do not like the idea of introducing extra global state variables.
What about let-binding `org-ansi-hide-sequences' in

(let (org-hide-emphasis-markers
      ...
              (region (or (org-find-text-property-region (point) 'org-emphasis)
                          ...)))
           ...
          (when region
            (org-with-point-at (car region)
              (forward-line 0)
              (let (font-lock-extend-region-functions)
                (font-lock-fontify-region (max (point-min) (1- (car region))) (cdr region))))))

> I ran into an issue when trying to hook into
> `org-fold-check-before-invisible-edit' in that when it revealed a
> sequence at the end of a line, there would be an extra fontification
> cycle that would occur after the reveal which would cause the sequence
> to be re-hidden again.  To counteract this I had to use
> `buffer-chars-modified-tick' in the way I do.  I couldn't figure out
> why redisplay was causing that extra fontification cycle when there
> were no modifications to the buffer.

Redisplay is a bit tricky in `org-fold-show-set-visibility' (see "FIXME"
comment).

Let's leave it be for now, until the patch gets closer to its final
state. Just add a "FIXME:" comment somewhere near the relevant code, so
that we do not forget about this issue.

> With this patch, editing around sequences should be more stable and
> non-surprising.  Basically if a sequence is invisible around point and
> you edit it, the sequence remains visible.  It is only after the first
> edit outside of a sequence that should make the sequence invisible.
> Whenever a sequence is being edited, it should always be visible and
> not turn invisible while in the middle of editing it, e.g. due to an
> invalid sequence turning valid.

We actually discussed a similar problem with links recently:
https://list.orgmode.org/orgmode/20240428093320.120843ae@enoush2o/
The general conclusion we came to was that whatever smart behavior we
may came up with will confuse users. Probably, the most predictable
approach would be either doing the same thing as what
https://github.com/awth13/org-appear or some variant of it that also
does not rely on fiddling with redisplay and/or fontification. Even what
we do in `org-fold-show-set-visibility' is sometimes confusing to the
users.

TL;DR: Do not worry about this problem - it should be solved in more
general context, for the whole Org.

> Some comments about the patch, as it currently stands, follow.
>
> - I've introduced two text properties `org-ansi' and
>   `org-ansi-context'.
>
>   The first is placed on the regions that actually contain ANSI
>   sequences and holds information about the sequence that is useful to
>   keep around to detect when a sequence has been modified or deleted
>   between fontification cycles, as well as information about whether
>   or not a sequence should be revealed due to modifications or because
>   of visibility changes.

Let's drop the part with modifications/visibility changes. It should not
be a job for fontification function, so let's not complicate things (as
I mentioned above). I believe that 'org-ansi may no longer be needed
once we drop this.

> ...
> - The logic to use in `org-fontify-ansi-sequences' and how to maintain
>   the highlighting across edits in the buffer are my main focus at
>   this point.  I think I've basically figured out the gist of the
>   logic, just need to clean it up.  What I have not really considered
>   that much is how to maintain/remove the highlighting across edits,
>   e.g. when there is something like
>   
>   <ANSI>line1
>   line2
>   line3
>   line4
>   
>   all lines being highlighted by the sequence, and the paragraph is
>   split at line3 so it becomes
>   
>   <ANSI>line1
>   line2
>   
>   line3
>   line4
>   
>   the highlighting is removed from line3 but not line4.  And there are
>   other situations where editing the buffer does not result in the
>   maintenance of the highlighting across the affected elements.  I
>   think I had it working in more situations when I had also placed the
>   `font-lock-multiline' property on the highlighted regions, but I tried
>   to simplify things by just using the `org-ansi-context' property
>   which may be able to handle these kinds of situations also somehow,
>   by detecting these kinds of edits and extending the region to
>   account for them.

I believe that `font-lock-extend-region-functions' should be able to
tackle this situation. Is there any problem with it?

(The rest of logic sounds reasonable, although I did not yet dive into
the code)

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>