"J.D. Smith" <[email protected]> writes: >> Hmm. Could you also push the changes to savannah? >> I still see ae0e2cdb2 as the latest commit. > > Ooops, push failed before for some reason. Should be on 17e801515 now.
I can see the update now. Thanks! >> Once upon a time, for a short while, link brackets were hidden using >> overlays. get-text-property does not look into overlays. >> You need get-char-property. > > Wow, that was an odd choice. Using font lock? org-inside's own > overlay uses 'invisible. For this reason, it is safer to use > text-properties for this. It was an attempt to support isearch'ing inside hidden parts of links. >>>>> + ;; for proper point adjustment >>>>> + (when (memq type '(emphasis raise)) >>>>> + (org-rear-nonsticky-at visible-beg) >>>>> + (when (< emacs-major-version 31) >>>>> + (org-rear-nonsticky-at visible-end))) >>>> >>>> Why do you need to do it in org-inside? Maybe we can already apply >>>> rear-nonsticky as a part of fontification? >>> >>> I'm respecting this dire warning (org.el): >>> >>> ;; FIXME: This would break current behavior with point >>> ;; being adjusted before hidden emphasis marker when >>> ;; using M-b. A proper fix would require custom >>> ;; syntax function that will mark emphasis markers as >>> ;; word constituents where appropriate. >>> ;; https://orgmode.org/list/87edl41jf0.fsf@localhost >>> ;; (org-rear-nonsticky-at (match-end 3)) >>> >>> I can't say I have understood what problems this caused. I'd be happy >>> to have the upstream do so (though I can understand if it doesn't want >>> to get into the subtleties of cursor-sensor differences between v30 and >>> v31). >> >> That warning is about 'invisible specifically. >> But, AFAIU, you are dealing with cursor-sensor property. >> Why not limiting non-stickiness to that specific property? > > The commented-out position (match-end 3) is where we do now add > 'rear-nonsticky (in org-inside). If this final line of code in the > stanza above wasn't commented out, we wouldn't need that. But > presumably you put that big FIXME in there (in 724135dda) to prevent > someone from just commenting it back in :). If you think it's now OK to > do so, we can remove the equivalent from org-inside. That is only commented out for the purposes of 'invisible stickiness. But we can limit stickiness to 'cursor-sensor-functions by setting the value of 'rear-nonsticky to a list of properties. >> This is Elisp-wide convention that loading libraries should not cause >> side effects. To the point, that Elisp manual says >> >> • Simply loading a package should not change Emacs's editing >> behavior. Include a command or commands to enable and disable the >> feature, or to invoke it. >> >> This convention is mandatory for any file that includes custom >> definitions. If fixing such a file to follow this convention >> requires an incompatible change, go ahead and make the incompatible >> change; don't postpone it. > > I'm certainly aware, but like most conventions, with good reason they > can be broken. In any case it's already fixed. Just a bit inefficient > to try to add the global hook each time, but its doing so on v<31 only, > so tolerable. This one is particularly strongly-worded. To the point of suggesting breaking changes. Suggesting breaking changes is almost like cursing in Elisp world :) >>> Easy enough to put it in the >>> setup (done). Since the mode is buffer-local, you can't turn it off >>> when the mode turns off. This btw is why I gate it for <=v30. >> >> What do you mean by "can't turn it off"? > > I meant only that you cannot remove a global hook when the buffer-local > mode is disabled, since other buffers may still need it. So even if all > buffers have org-inside disabled, it will continue to do a (tiny bit) of > unnecessary work. Again, a v<31 corner case. Well. We can work around that. But your change is acceptable. >>>> Do we have to change where 'org-emphasis property is applied? AFAIR, >>>> some third-party packages depend on the current behavior. >>> >>> This is mostly just a convenience reorg to be able to pass vbeg/vend to >>> the new hook. >> >> You could pass adjusted values to the hook. I do not see why changing >> how the actual text properties are applied is necessary. >> >>> We did need to change from invisible=t to >>> invisible=org-raise so our overlay invisibility countermanding approach >>> worked. >> >> That should be fine. I do not know any code that would depend on >> invisible to be exactly t. In fact, it was not t for quite a while in >> one of the releases. > >>> ... And we needed to add rear-nonsticky too, for consistent point >>> movement. >> >> See above. > > My point was, because I needed to track visible region, and also change > the applied properties, it made sense for efficiency and compactness to > pre-compute them and use directly. The end result is it reads more > clearly, IMO. Feel free to rework if you feel it's important. I feel that we are talking past each other. I am talking about (unless (or comment-p emph-p link-p keyw-p) - (put-text-property (match-beginning 3) (match-end 0) - 'display + (put-text-property (match-beginning 2) vend 'org-emphasis t) + (add-text-properties (match-beginning 2) (match-end 2) props) + (when (and (eq (char-after vbeg) ?{) + (eq (char-before vend) ?})) + (add-text-properties vbeg (1+ vbeg) props) + (add-text-properties (1- vend) vend props) + (setq vbeg (1+ vbeg) vend (1- vend))) + (put-text-property vbeg vend 'display (if (equal (char-after (match-beginning 2)) ?^) (nth (if table-p 3 1) org-script-display) (nth (if table-p 2 0) org-script-display))) - (put-text-property (match-beginning 2) (match-end 3) - 'org-emphasis t) - (add-text-properties (match-beginning 2) (match-end 2) - (list 'invisible t)) - (when (and (eq (char-after (match-beginning 3)) ?{) - (eq (char-before (match-end 3)) ?})) - (add-text-properties (match-beginning 3) (1+ (match-beginning 3)) - (list 'invisible t)) - (add-text-properties (1- (match-end 3)) (match-end 3) - (list 'invisible t)))) + (run-hook-with-args 'org-hidden-text-functions 'raise + (match-beginning 0) (match-end 0) vbeg vend)) where you apply 'org-emphasis to (match-beginning 2) ... venv while it was (match-beginning 2) ... (match-end 3) in the past. -- Ihor Radchenko // yantar92, Org mode maintainer, 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>
