[Fix #77] Show beacon after folded org-mode headers
I don't consider this "done" yet, but am perhaps letting perfect be the
enemy of the good. The beacon does /show/ correctly, and I drop a few
spaces to make up for the ellipsis. After-string still shouldn't wrap around
thanks to that and beacon--visual-current-column measuring from the
beginning of the line.
Rough edges:
- The ellipsis is one long char, distorting the gradient; think I just gotta live with this one, but it wouldn't be /as/ bad if I didn't drop those 3-4 spaces here too.
- The face that
org-modeuses for it's headers doesn't:extendto the overlay, so these blinks are much shorter than others. This is a broader situation regarding proportional fonts, perhaps beyond the scope of this issue. Also makes it hard to experimentally check that the ellipsis spacing is right, I think we only need to drop two- not that it matters with these super slim spaces. - These blinks don't fade out smoothly, but vanish all at once.
Regarding nitpick 3:
- When
beacon-blinkis invoked from the char preceeding the ellipsis,beacon-decis called only once; on an overlay frompointto(+ (point) 1)(just ahead of the invisible chars). -
beacon--vanishis then called with an overlay still present inbeacon--ovs, located immediately after the invisible chars (eg. from1544-1544, notn-n+1).
I'm ignorant of exactly how overlays work, or how you're cleaning them up so beautifully, but this looks like the most addressable shortcoming; maybe you've got some insights on how the 'good path' works and where to look to see what's going on.
Invisible text props aren't just an org-mode thing so I'm thinking of a more general solution, maybe not just at the ends of lines, maybe handling other long-chars, and maybe keeping the org funcions if you're cool with loading them. There might be some hard problems with proportional fonts, and I couldn't find many ways to inspect the current line's assorted faces. There might be a package out there for that.
I've got a Proof-of-Concept solution that's behaviorally clean, but awfully slow, and unnecessarily extensive. I'll be back at some point once I trim it into a smaller patch, which should help with preformance too-- I get the impression that speed is something people really pay attention to for this package in particular, so I'll be bringing the numbers with me when I do.
Thanks for the PR @AutumnalAntlers I think the beacon vanishing issue could be caused by the fact that line 251 moves the point, and beacon expect like that. (The other issues indeed don't seem like something we should fix here.)
Thanks for taking the time to review my patch!
Is the # at the end of ... intentional?
It was based on the default value when customizing org-ellipsis, but I ought to dismiss it because, even without digging to why it's there, we only intend to account for the space taken up on screen, not in buffer. Fixed, but it's still a little jarring to be dropping those characters from the middle..
Maybe with a save-excursion....
Oops, that sounds right; I see that beacon--shine already wraps beacon--after-string-overlay in a save-excursion, but maybe you're specifically suggesting returning from the excursion before the fade out. I'll address that later on.
Some time later: (Edit: This comment and the next were a bit of a mess, but I appreciate your time and want it to read easily, so I've cleaned it up.)
I've either in-lined or obsolesced the org functions. Instead of factoring in the ellipsis explicitly, the code in my org-mode branch now calls the posn utilities to "do-the-right-thing" (including a smooth gradient, and correct truncation at EOL in variable-pitch buffers). I'm not happy with the performance of this command; it's about 3x faster to sum the width of each glyph from beginning-of-visual-line to point (on my machine), and the code is rough, but I'd like to clean that method up instead.
The issue with the fade-out still exists, just haven't looked into it yet. Thanks for the lead. I have a hunch that it might be due to the fact that the entire collapsed section is in the middle of the beacon, and I might just need to handle invisible characters more intelligently.
I've bench-marked the posn function in comparison to:
- A blink of the beacon
- The existing
beacon--visual-current-columnfunction - My pixel-width counting implementation
As implied by the direct comparison, the posn function should be considered an alternative implementation of beacon--visual-current-column for now, not a part of beacon--after-string-overlay-colors as it is in the commits that I've actually pushed.
;; 'prettify-symbols-mode' uses compositions, which break
;; beacon--visual-current column and pixel-width-based-visual-column,
;; but this is a problem for another day. For what it's worth, I don't
;; think it's worth relying on the 'posn' variant in order to address this,
;; and it can patched into any method as another issue maybe-someday.
(prettify-symbols-mode -1)
(defun posn-based-visual-column ()
(floor (/ (float (car (posn-x-y (posn-at-point)))) (window-font-width))))
(defalias 'pixel-width-based-visual-column
;; Implementation not shown.
#'beacon--visual-column-at-x-coordinate)
(defun random-point ()
(seq-random-elt (number-sequence (window-start) (window-end))))
(let ((x 100000))
;; Show implementations are equivalent
(dotimes (_ x)
(save-excursion
(goto-char (random-point))
(cl-assert
(equal (length (seq-uniq (list
(beacon--visual-current-column)
(posn-based-visual-column)
(pixel-width-based-visual-column))))
1))))
;; Relative benchmarks
(list
(benchmark-run-compiled x
(save-excursion (goto-char (random-point)) (beacon-blink)))
(benchmark-run-compiled x
(save-excursion (goto-char (random-point)) (beacon--visual-current-column)))
(benchmark-run-compiled x
(save-excursion (goto-char (random-point)) (posn-based-visual-column)))
(benchmark-run-compiled x
(save-excursion (goto-char (random-point)) (pixel-width-based-visual-column)))))
| Benchmark (100,000 iterations) | Wall-Clock Time (s) | GC Runs | Time in GC (s) |
|---|---|---|---|
| beacon-blink [master] | 56.73930948 | 2062 | 41.38249635 |
| beacon--visual-current-column [master] | 39.21924088 | 1094 | 22.53072220 |
| posn-based-visual-column [org-mode] | 233.17117498 | 3767 | 76.67917287 |
| pixel-width-based-visual-column [impl. not shown] | 85.76274830 | 2514 | 49.79807926 |
I have the idea that either alternative could be used down the line (once per blink, to maintain performance) to standardize the width and gradient of the beacon in variable-pitch buffers , giving a consistent appearance on lines which mix different font families, ligatures, and heights (if not composition's :p), so the performance cost has more pros than just truncating correctly in the context of this issue.
The counting method still doesn't handle compositions or the ellipsis correctly, but I've learned more about them. The ellipsis is rendered by Emacs out of slot 4 of the active display-table (which I can work with, just not sure hiw outline-mode is applying each headings face to it yet), and I might be able to account for continuations by learning more about glyphs and char-tables. I'd really like to be able to account for these without 'relying on' or 'otherwise losing the performance advantage over' posn-at-point, and will have compositions and overlays on my mind.
Edit 6/18: Doesn't feel worth pinging over, but I have dug further into this, and may yet be back. Compositions and selective-display would both be handled if I had a function of the form (glyph-width char &optional face buffer window) (or just (glyph-width glyph)) that was still fast enough to outpreform posn-at-point, and I have a hard time letting go of little things I've set my mind to, but eventually I'll realize that the difference isn't worth the complexity or that I ought to come back to it after closing this. Either way in due time.
Edit 6/28: Ooo, I think I got a lead!
Edit 6/29: Involves window-text-pixel-area, which.. maybe I can get away with calling it once, like to handle selective-display-ellipsis (ie. this actual issue), but for more ambitous use, I can't assume the number of eg. compositions in region. Ain't that how it goes.
Edit 6/30: Emacs promises so much freedom, fasciliated by so much transparency, and then selective-display-ellipses exists. It still endears me, but from my lisp immersed perspective, the C source and the depths of the glyph system are murky waters. I think to myself:
Would it still be quicker (than calling posn-at-point) to create an invisable frame, insert each char of a composition into it, apply the appropriate faces, and then use font-get-glyph (which can't be used in child frames?) or window-text-pixel-area (which is only 3x faster on my machine) to add up the width of each char?".
Crazy talk, but not if it checked out; just wouldn't pass code review. Thankfully, I've got compositions figured out, but that could still apply to the selective display ellispsis.
~~The issue I'm at is the reason I bumped a -1 to a -2 here, though I wouldn't have known it at the time, and that's not the exact code I'm looking at. If you go into org-mode, fold a heading, and put your cursor just past the ellipsis ((kbd "A") in evil), then the posn returned by posn-at-point or window-text-pixel-area appears to be one window-font-width short. I'll get this figured out in due time too, first by checking that it scales with the face applied to the ellipsis, and then by setting the ellipsis to a single period to see what happens.~~
insert each char of a composition into it, apply the appropriate faces, and then use font-get-glyph
Edit 7/3: I might have been on to something there! Instead of a new frame or window, one can propertize a string and pass that directly to font-get-glyph, solving compositions and selective-display.
Still ambitiously fiddling with the things I've mentioned above; this week, I found out about string-split-glyph (IIRC) and discovered that calling posn-at-x-y is way less expensive than posn-at-point. ~In fact, it might be faster to call window-text-pixel-area with X-LIMIT and Y-LIMIT set to point, fudge those numbers a bit (another rabbit hole of corner cases), and then use them to call posn-at-x-y, than it would be to call posn-at-point in any context where one might need to. Might be able to take that upstream if it works! But I digress.~
(Nah, that's just how long it takes to get x-y @ point via eg. window-absolute-pixel-position, but still a win for what I'm up to. Mostly hampered by the fact that both functions are slower at the bottom of a window than at the top. Wish I could access a useful C function directly.).
The blocker on this issue (barring any code review, much digression / iteration / uncharacteristically poor VCS use, maybe because it's so small and I'm so carried away?) is no longer speed, or correctness, but that the selective-display-ellipsis seems to inherit it's face from the preceding character, so it's not possible to shine a beacon correctly with point /on/ the ellipsis, only on the char before or at the the EOL after. I'll look into outline.el, org-*.el, and the C Source if I need to, to figure out where this behavior is defined and whether or not I can address it. At worst, it's not any different from the current beacon behavior, but I haven't ran out of steam quite yet.
P.S.: The longer I keep discovering new functions the more tempted I am to just read entire source files, and the more convinced I become that maybe, for all the fuzzy finding packages, Emacs does have a discovery issue after all. Eg. I wish I could search for functions by their named parameter lists (in an ideal world, by those parameter's types, but IK that's asking too much). Also learned about Edebug, disassemble, and which kinds of loops are the fastest. The byte-compiler makes me miss Scheme.
Edit: A lot of the complexity behind posn-at-point is to handle RTL display. I've settled on window-pixel-absolute-posn and the measuring of glyph widths to pull tricks instead, but have assumed LTR in the process; should be a quick fix.
Holy shit, I got it! I can style the selective-display-ellipsis! Only took two months :p It only works in GUI frames ('ll figure something out for TUI frames but it's going to be second-rate), and it's a little bit.. hack-ish, but that's the only way it can be done.
I forked beacon.el in all sorts of other ways that I'd love to get feedback on (not a lot of folks in my own life would appreciate or care), but it's a pretty wide departure from upstream at this point and well outside the scope a pull request, so if you'd just casually take a look it would make my ~day~ week. Got some polishing left, but I'll make a GH branch for it by mid-August, so around then. I've developed a couple questions too, so if I don't stumble into any answers on my own before then, I'll have those.
As for this PR, I work two jobs today, but will find time to pull out the bits related specifically to drawing at/over selective-display and truncating correctly at EOL in the presence of variable-pitch fonts (exclusively truncation, anything else is out-of-scope) and hard-reset the org-mode branch that's tied to this PR tonight.
Other notes:
- I was miffed with the preformance of
posn-at-point, especially at the bottom of the screen, but that's premature optimization and, after a lot of fiddling, is neither avoidable nor impactful in most contexts. Was mostly a concern for other features, not this special case. - If the newline at the end of a line is styled with a diffrent font, could that change the width after the
after-stringoverlay? When would that happen? I can account for it. - I don't think I ever circled back to deal with how
beacon--dechandles this case, I feel confident now I can add a quick conditional for it now-- I know what was going on.
Alright, so, beacon--dec came out pretty.. rough.
I know what was going on.
Thought it would be as simple as shuffling point around, but no plan survives first contact.
Learning how beacon--dec worked was interesting: I'd never seen timers before, finally saw how the beacon--colors property was used down the line, and love the way that point combs down a line tossing everything over it's shoulder. I've got few benchmarks I'd like to try (as I'm wont to do), but I'll be back to refactor this unprompted eventually-- can't think of any less cumbersome function names, and in the meantime I wouldn't ask that you look too closely. I haven't structured anything cleanly, or added any comments, outside of the work I'd already done when writing those utility functions. Ideally some of the logic in beacon--dec could be refactored out, I feel like I'm missing a layer.
So it's still a.. functional proof-of-concept-- but function it does! Frankly it's just amazing I had time to get anything fixed up at all, I knew that wouldn't be the case for a while now. I'd add a GIF if I knew how, and had any more time, and heck, maybe later this week, it's something I'd like to learn to do anyway, maybe there are emacs-specific workflows out there.
^ See commit message c:
Mid-August I said, never give a date! That fork of mine has diverged far enough that this fix for beacon--dec doesn't actually apply to it, so it'll need it's own reworking of --vanish (within arbitrary performance constraints :p) before I can turn my attention to some pretty and digestible GIFs. Perfectly happy to mush this PR into whatever shape you'd like in the meantime, provided it bears promise now that the method is functional and readable.
just an idea
What I'm saying is that I still haven't written doc-strings or strong leading comments these functions, nor for those that style the selective-display ellipsis :p Also on the TODO is checking how this behaves in a TUI frame, lest I forget.
Edit: could have sworn I removed the forward-char calls from --toss-after-string, those don't need to be there
Edit (12AM, 2 Months Later): After working on Other Stuff for a bit, I stayed up and reimplemented beacon--dec for my pretty GIFs-- just got to come back 'round later this week:
- bench the changes
- remove/address straggling todo comments
Won't try too hard to rewrite anything again because, gosh, I just want to make those GIFs to show off and then project over! But I have very little control over what I get carried away with, and kinda did rough things up, it was... "more complicated than it seemed"™. Fading out `after-string' fade-out was no longer as simple as deleting the first char; Honestly, I don't know how my solution works, certainly won't in the morning, but it removes roughly a handful of spaces per-iteration and, most importantly, looks right, so it'll do. Tests still pass! Oh, and I wrote tests, before my hiatus; they're just awful code, but, you know, exist to help make the actual code better and they've done their job superbly, so, maybe I could learn something from that~
Edit: disappears back into the shadows Said tests need re-worked, and I've added a handful that uncover assorted issues with extremely large or small fonts and some Unicode character / font combinations. Ofc, I want to fix those...
Edit: 10PM, frickin december I was real miffed to find out that the ellipsis was drawn by the display engine; you know what else is? Line numbers! They're not in the margin, and not in the fringe, but inserted directly into the glyph matrix :/ I guess I have tests, but, how do I really know what else might be? I still fiddle with this, and am /always/ re-writing it "again" with every corner-case and concern known by-heart and in-mind. Still no perfect rendition.
Status as of Christmas: I swear this is the last time I'll rewrite it. Hit 80% feature parity last week, a bit more efficient than last time, way happier with the code layout, and going to rewrite the tests with Buttercup as I port over the corner cases that comprise that last 20%. Bothers me that edebug doesn't work very well with the macros I've written, but I'll need to balance what it takes to fix that with efficiency and readability. May have engaged in some LOC golfing, am weighing whether or not to inflate LOC for more thorough tests (ie. by isolating input/output from logic) but leaning against it. The result will be an incomplete compromise inspired by that train of thought, benefiting from the caution and adapting existing abstractions that are almost it, without going all-in at the cost of workload, LOC, or efficiency. This has been fun!
2023/03/26: Didn't really work on this in March but I feel obliged to reach the arbitrary standard I set for sharing the code. Learned all about native-comp, ditched all in-lining (ie. byte-compiled performance), frankly frustrated even with native-comp (port to C after sharing? :p), still a few corner cases that I need robust solutions for, you know. Really happy with my bespoke test framework (no more Buttercup bc reasons), maybe I'll factor that out into it's own thing, but am also still porting old tests into it (and enumerating new edge-cases ;-;). Then there's updating doc-strings, the fact that this is on the back burner, etc. I'mma say... soon™; what matters is that I'm learning (might port to C for real, gotta learn the language!).
2023/05/28:
Status as of Christmas: I swear this is the last time I'll rewrite it
lol, April had some progress, not much in May ~because~ but I'm still doing my thing-- have decided the test framework needs reworked into a human-readable composition-based design inspired by Jane Street and Judge. Meanwhile, rat's nests have grown within my beautiful re-write, which never quite reached feature-parity with the last. I intend to cycle back to this, indulge in bike-shedding the test framework, clean the nests, and reconcile the iterations as I head into June, nearly a year into my on-and-off experiments. Robustness is still questionable, but at this point I blame Emacs itself for many of the corner cases (unknown unknowns!), and will have a better idea of where my approach / test-coverage stands in about three weeks of mostly working on other stuff. I think I might have ADHD?
Post-June: Well that didn't happen; other stuff came up. learned C tho, that could come in handy Late-Aug: ...those human read-able test-results are rubbing me the wrong way. I think I'm going to make an X11-only test suite that uses eg. scrot to ensure that, not only is the data structure we create correct, but it actually works. And the images will be hella human readable! /Then/ I'll make sure all the tests actually pass and publish it to GH. And then maybe re-write it in C. Gosh I love learning projects.
Oh my gosh, there's precedent for image-based tests /and/ for patching emacs in the process :o (I may attempt to export frames as images)