"J.D. Smith" <[email protected]> writes:

* Window Configurations [was: Another Sticky Bar Cursor]

>>>> Start with the usual incantation ("make repro", o-h-e-m (I've set the
>>>> default to true already in Org's sources!), open test file, o-i-m, move
>>>> into emphasis and ensure cursor is bar).  Then:
>>>>
>>>> + C-x t 2
>>>> + C-x t o
>>>> + M-x org-inside-mode RET
>>>> + C-x t o
>>>> -> Cursor stays bar.
>>>
>>> I can't reproduce this.  The cursor is removed for either; maybe it's a
>>> recent change.  I don't use inbuilt tabs but the native ones in my
>>> build, which are just frames.
>>
>> Ok, here is some more information on that.  I cannot reproduce that for
>> Emacs 30.2, either, only for Emacs 31+.  The reason is this one hook
>> *non*-addition for Emacs 31+ in org-inside:
>>
>>   ;; In v31+, buffer-local window-buffer-change-functions fire for a
>>   ;; buffer appearing AND disappearing from a window.
>>   (when (< emacs-major-version 31)
>>     (add-hook 'window-buffer-change-functions #'org-inside--frame-changed))
>
> Interesting.  I don't know how tabs are treated w.r.t. window change
> functions, but IMO they "should" fire buffer-locally for both entering
> and exiting the buffer (on tab change), so this global hammer should not
> be needed.  I'd say this could be considered a bug/omission in v31.

I tried to get my point through by delicate hints here and there, but
obviously have failed to do so.  So here rather bluntly, hoping not to
lecture you more than needed:

- Tabs are special window configurations.  Changing a tab just changes
  the frame's window configuration.

- From the documentation of window configurations, I cannot deduce that
  changing one should run `window-buffer-change-functions'.  Changing a
  window configuration places a previously stored complete set of
  windows and their buffers on the glass, but, at least in my
  perception, the buffers inside these windows do not change.

  IOW and IMO, changing a buffer in a window happens on a different
  scope than changing a window configuration in a frame.

>   *** 'window-buffer-change-functions' is run for removed buffers too.
>   The buffer-local version of 'window-buffer-change-functions' may be run
>   twice now: Once for the buffer removed from the respective window and
>   once for the buffer now shown in that window.
>
> Are you interested in opening a report?

- So the only bug I see here is that changing a window configuration
  does actually run `window-buffer-change-functions'.  And I won't file
  that, no.

>> I'll leave it to you whether and how you'd like to fix that.  Probably
>> employ `window-configuration-change-hook' in some way or other?
>
> The goal is to remove the global hook.

- Note that I referenced `window-**configuration**-change-hook' in my
  mail, a global hook which org-inside does not use.


* Window Configurations and Window Parameters

And another interesting fact that I have mentioned only indirectly, by
referencing the documentation:

- Window configurations (and the related window states) by default do
  *not* store window parameters.  See `window-persistent-parameters'.

- So changing window configurations can nuke your window parameters, and
  org-inside to some extent relies on these.  I have not been able to
  reproduce an org-inside bug out of that fact, mainly because -
  contrary to the documentation - window parameters seem to be pretty
  persistent in window configurations.

So my overall impression here is that org-inside could care a bit more
about window configurations, most notably through
`window-persistent-parameters' and/or
`window-configuration-change-hook'.  By and large, the interaction of
org-inside with window configurations and window states seems to work
OOTB, but at least I have not always been able to understand why.


* More "Natural" Cursor Movement?

> To give you a clear sense of how I use this, I (nearly) NEVER actually
> see the markers.  I don't want to see them.  I have key bindings that
> emphasize the current symbol.  And I can use org-inside to precisely
> edit around the borders of hidden-markers without every revealing them.
> In my usage, they are just a detail.

I haven't been talking about unhiding markers in my previous mail, so
I'm not sure why you do.  Everything I wrote previously I related only
to always-hidden markers.

>> If you remember the beginning of this thread, you might recall that I
>> have been whining because of the cursor "jumping" at emphasis markers.
>> I took your explanation as given back then, but have reconsidered this
>> since then, after I understood org-inside better.  That jumping happends
>> because of cursor adjustments around the markers.  But there is really
>> no need for that.  Optionally and without too much effort (prototype
>> available) one could have cursor movement like described below, which
>> feels much more natural at least to me.
>
> I recall your misconstrual of where point is with block vs. bar cursors
> was the real issue here (which may happen to others; we still need to
> decide on suitable defaults).

Not only.  More disconcerting I find the fact that the sequence C-f C-b
(or C-b C-f) does not always bring me back to the same visible state as
previously.

> I had considered this option and the advantages you listed, which I
> agree with for the most part.  I even tried using it for a while.  But I
> dismissed it.  The invariant that one C-f = one cursor motion on screen
> is pretty sacred.

For me, obviously, the fact that C-f C-b leaves the cursor in the same
visible state on the screen as previously is more sacred.  But let's
agree to disagree here.  I guess I've pestered you long enough with my
point of view that you will remember it if others will do so, too.


* Replacing Overlay Modifications Hooks and Cursor Sensors

Not sure whether you are interested in that, but the following replaces
overlay modifications hooks and cursor sensors by a text-property-only
solution.  Pros: Feels simpler.  Reliably destroys the overlay when
needed (because for me the cursor sensors *do* fire when their text
properties are removed).  Cons: Does not work OOTB on Emacs 31+.  Plus I
haven't tested it as thoroughly as you (and I) did, so it might behave
differently from your approach in some cases.

@@ -94,16 +94,6 @@ for the entity you are inside."
     (org-element-lineage ctx '( link bold code italic verbatim
                                 underline strike-through) t)))

-(defun org-inside--overlay-modification (ov after-p &rest _r)
-  "Detect modifications of the entity covered by `org-inside' overlay OV.
-AFTER-P is t if called after the modification occurs.  If the org entity
-covered by OV no longer exists, we set the appearance for outside."
-  (when after-p
-    (save-excursion
-      (goto-char (overlay-start ov))
-      (unless (org-inside--elem-at-point)
-        (org-inside--set-appearance (selected-window))))))
-
 (defun org-inside--overlay (win face unhide)
   "Return an appropriately styled overlay for window WIN.
 FACE and UNHIDE are the text face and invisibility status; see
@@ -112,8 +102,6 @@ FACE and UNHIDE are the text face and invisibility status; 
see
     (unless (and ov (overlayp ov) (buffer-live-p (overlay-buffer ov)))
       (setq ov (make-overlay 1 1 (window-buffer win) t))
       (overlay-put ov 'window win)
-      (overlay-put ov 'cursor-sensor-functions '(org-inside--sensor))
-      (overlay-put ov 'modification-hooks '(org-inside--overlay-modification))

       ;; For auto-unhiding, we set the invisible property to something
       ;; guaranteed not to be on the `buffer-invisibility-spec'.
@@ -139,10 +127,7 @@ consult this window parameter to restore the cursor type."
            (showing-p (overlay-get ov 'invisible))) ; non-nil = unhidden!
       ;; We move the overlay when returning to the run-loop to avoid
       ;; the cursor-sensor race for point adjustment, since our
-      ;; overlay unhides text which point adjustment can skip.  As
-      ;; well, since both text and overlay implement
-      ;; cursor-sensor-functions, this avoids a similar race as to
-      ;; which applies.
+      ;; overlay unhides text which point adjustment can skip.
       (run-at-time 0 nil (lambda (buf)
                            (with-current-buffer buf
                              (if inside-p (move-overlay ov beg end)
@@ -185,6 +170,10 @@ type."
    ((eq type 'left)          ; called from the overlay's cursor-sensor
     (org-inside--set-appearance win))))

+(defun org-inside--sensor-exit-only (win _pos type)
+  (when (eq type 'left)
+    (org-inside--set-appearance win)))
+
 (defsubst org-inside--restore-cursor (win)
   "Restore old cursor in WIN (if any).
 If the current window cursor type is nil (i.e. the cursor is hidden), no
@@ -232,7 +221,7 @@ If POS is nil, use point."
        (org-inside--restore-cursor win)))
    nil frame))

-(defun org-inside--add-properties (type _beg _end visible-beg visible-end)
+(defun org-inside--add-properties (type beg end visible-beg visible-end)
   "Add text properties to invisible text for org-inside functionality.
 TYPE is the type of text being hidden.  BEG, END, VISIBLE-BEG,
 VISIBLE-END are the buffer positions of the affected text and its
@@ -246,6 +235,7 @@ visible portion."
   ;; inserted after it.
   (when (< emacs-major-version 31)      ;; TODO: use static-if when available
     (setq visible-end (min (1+ visible-end) (point-max)))
+    (setq end (min (1+ end) (point-max)))
     (add-text-properties (1- visible-end) visible-end
                          '(rear-nonsticky (cursor-sensor-functions))))
   ;; for proper point adjustment
@@ -253,8 +243,14 @@ visible portion."
     (org-rear-nonsticky-at visible-beg)
     (when (< emacs-major-version 31)
       (org-rear-nonsticky-at visible-end)))
+  (unless (get-text-property beg 'cursor-sensor-functions)
+    (put-text-property beg visible-beg
+                      'cursor-sensor-functions 
'(org-inside--sensor-exit-only)))
   (put-text-property visible-beg visible-end
-                    'cursor-sensor-functions '(org-inside--sensor)))
+                    'cursor-sensor-functions '(org-inside--sensor))
+  (unless (get-text-property beg 'cursor-sensor-functions)
+    (put-text-property visible-end end
+                      'cursor-sensor-functions 
'(org-inside--sensor-exit-only))))

 (defun org-inside--setup ()
   "Setup buffer for `org-inside'."


* The End

Let's call it a day here, at least from my side.  Thanks for your work
and for the discussion.

But as soon as the branch gets merged I'll switch from my solution to
yours for routine work - which may bring more bug reports.  I'll be
back!

Reply via email to