"J.D. Smith" <[email protected]> writes: > Fixed/added all of these.
Hmm. Could you also push the changes to savannah? I still see ae0e2cdb2 as the latest commit. >>> + (get-text-property (org-element-begin el) 'invisible)) >> >> Why not `invisible-p'? > > Because we are interested in whether the text at the entity's beginning > /could/ be invisible, not whether it actually is. Overlay properties & > buffer-invisibility-spec affect /actual/ invisibility. 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. >>> + (let ((beg (org-element-begin elem)) >>> + (end (- (org-element-end elem) (org-element-post-blank elem)))) >> >> This is not accurate when the post-blank lines contain tabs/spaces. > > I don't understand why or in what scenarios org-element "absorbs" > trailing white-space. Please suggest a workaround. You code will actually work for objects. All whitespace after the end of object is inside its post-blank, counting characters. The problem is with paragraph-like elements - post-blank counts *lines* there. I'd put a comment that ELEM must be an object. Then, the code is 100% correct. > ... I understand there > will be a new element `noblank-end' or so, but we can't use that? Yeah, I plan to add a property marking position before blank to the parser. However, I have been struggling to find time for any substantial contributions like this one for a long time. Patches and bug reports occupy all my free time nowadays. >>> + (when (get-text-property beg 'invisible) >>> + (setq beg (next-single-property-change beg 'invisible nil end) >>> + end (previous-single-property-change end 'invisible nil beg)) >>> + (and (> end beg) (cons beg end))))) >> >> Considering that you are sometimes putting 'invisible 'totally-visible, >> invisible-p would be more reliable. > > We do that only on the overlay. We are trying to find out where we are > inside the: > > IIIIIIVVVVVIII > > entity, where I=invisible, V=visible (real text properties only). We > want to know this independent of any other buffer setting which affects > visibility (as above). `invisible-p' is "too smart". But what happens if (get-text-property beg 'invisible) is non-nil but we also have invisibility spec making it visible? I feel that the logic in the code is off in such situation. >>> + (unless (minibuffer-window-active-p win) >> >> Why is it necessary? > > If I recall, sometimes the sensor fires when you are entering the > minibuffer, which is never what we want. It is worth an explanatory comment. If I understand you correctly, you do not want cursor/visibility changes when user, say, types M-x while "inside" something. >>> + ;; 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? >>> +;; Starting in v31, buffer-local change functions are run in >>> +;; windows a buffer left as well those it entered. >>> +(unless (>= emacs-major-version 31) >>> + (add-hook 'window-buffer-change-functions #'org-inside--frame-changed)) >> >> Why is this not a part of org-inside-mode setup? >> Side effects when merely loading a library should be avoided. > > I assumed with our autoload in place, no one would ever load the library > except because they want to enable it. 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. > 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"? >> 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. -- 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>
