Jens Schmidt <[email protected]> writes:

> On 2026-05-17  13:40, J.D. Smith wrote:
>
>> Replied separately. I /think/ I'm caught up on your review and
>> suggestions; let me know if I missed something.  Biggest thing now is to
>> text v30 vs. v31 (which I haven't yet done), for functionality:
>> 
>> - Enter/exit at correct locations for both (modulo the small difference
>>   I pointed out w.r.t. the end).
>> 
>> - Editing text near boundaries (just inside, just outside) does what's
>>   expected for both.
>> 
>> - Changing buffer in windows works as expected for both.
>> 
>> Thanks for the thorough tests and good suggestions!
>
> Thanks for your work, tests continue.  Sorry for not focusing on what
> you would like to have tested, but I just still keep finding other
> issues.

:)

> General remarks:

> Here is a patch, advantages listed below:

<snip>

> [ Not sure whether I got all the "<" vs "<=" right - I just hacked
>   that together rather quickly. ]
>
> - It makes `o-i-t-v' suitable for addition to some abnormal hooks,
>   which I in particular would be keen to have.

I don't fully understand your patch.  The overlay is only on some text
if point is inside that text, so it seems the additional checks are
superfluous.

> - It avoids touching markers outside of a narrowed region.  Not
>   sure whether you consider this an issue.

Confused again.  When you narrow to a region, point is automatically
moved within that region, so the overlay is always strictly inside the
visible part of the buffer.

> - It fixes a bug like this:
>
>   + "make repro"
>   + M-: (setq org-hide-emphasis-markers t) RET
>   + M-: (setq org-inside-appearance nil) RET
>   + open reference Org mode document
>   + M-x org-inside-mode RET
>   + M->
>   + M-x org-inside-toggle-hidden RET (*outside* of the emph text)
>   + C-b a couple of times
>
>   => This shows the emph markers when moving into the emph text,
>   since the call to `o-i-t-h' has "primed" the overlay already to
>   be visible, IIUC.

Thanks, this was a subtle bug: you should not have been able to "prime"
the unhiding of the overlay.  For whatever reason I recall being able to
stash an overlay and keep it out of the buffer by moving to 0,0, but
that isn't actually correct.  I've switched to properly deleting and
moving (deleting would be better called "detaching"), now fixed.

> That window-vs-buffer thingy will surely be good for more surprises.
> Here is one:
>
>   + make repro
>   + (setq org-hide-emphasis-markers t)
>   + open reference Org mode document
>   + M-x org-inside-mode RET
>   + Move into emph text and let cursor turn bar
>   + C-x 2
>   + C-x o
>   + M-x org-inside-mode RET
>   + C-x o
>
>   => Cursor stays bar in upper window.

> I guess whenever the visibility or appearance changes for whatever
> reason, you'd need to walk all windows.

Good point.  I now loop over windows displaying the buffer where it is
being disabled/enabled.  The overlay definitely needs to be
window-local, since you might be displaying the same buffer in multiple
windows, with different inside-ness status.

> BTW, when walking windows (regardless for above issue or for
> `org-inside--reset-all'), probably consider walking with
>
>   (walk-windows ... nil t)
>
> Otherwise, e.g., *customize*-in-one-frame-org-mode-buffer-in-other
> scenarios still can lead to dangling bar shaped cursors when
> customizing o-i-a.  These operations should be rather infrequent,
> so more walking shouldn't be a problem.

Fair point; done. 

> The custom spec has issues.  Well, at least one, and only in an
> exotic (?) Emacs session where cus-start.el has not been loaded
> yet and `(get 'cursor-type 'custom-type)' can equal nil:
>
>   + "make repro"
>   + M-x load-library org-inside RET
>   + M-x customize-variable org-inside-appearance RET
<snip>
>
> [ It can be that I have been seeing pink elephants again here,
>   caused by test fatigue.  At some time I couldn't reproduce the
>   stack trace shown above, but then it happened again ... ]

I think this is fair.  No reason not to require cus-start on load, since
our file is autoloaded anyway.

> I've noticed trailing whitespace in org-inside.el, not sure
> whether you or anybody else bothers.

Fixed.

> The story will probably continue ...

Thanks for the close look.  When you get a chance to compare behavior in
v30 vs v31 I'll be all ears.

Reply via email to