-
Notifications
You must be signed in to change notification settings - Fork 29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Live render partially working #198
Comments
You can pass $ INLYNE_LOG="warn,inlyne=trace" inlyne ratatui/BREAKING_CHANGES.md It's pretty hit or miss on what actually has logs, but the file watcher is pretty log-happy since it's notoriously annoying to work with I'll see if I can reproduce later today |
Thanks, I'll be sure to upload the logs and investigate on it. At a quick glance I don't see anything standing out except multiple file events for a single write. |
Thanks! I appreciate all the help investigating
That's expected for neovim. More context here #104 |
I feel your pain :/ Other editorsSo I tested other editors with inlyne 0.3.3:
In fact to reproduce with mousepad or vim I had to stress my all my CPUs by running a cargo compilation. But um ... with neovim stressing the CPUs affected it positively ... it rendered correctly more often. Sets of logs for each editor
Notice vim and mousepad both re-register the file watcher, while neovim and shell redirect don't. nvim
vim
mousepad
echo >
Additional infoMy nvim has no swapfile/backup file whereas my vim has Also edited the original post to add nvim version 0.9.4 |
Thanks for all the info! I'll dig in later tonight My hunch is that there's something funky going on with the logic for freezing the screen while the interpreter is rendering a new document. Regardless it's probably a good time to sprinkle more |
Off work now, and thanks for all the info. I think it should actually be a pretty easy fix. The piece that put it all together was
because I wasn't able to reproduce the issue, but I was using a swapfile. After disabling that and adding some extra logging it looks like we're reading the file before it has time to fully be written to disk. Here are the logs from just continuously saving the same file where it logs the length of the file it recieved
|
@Valentin271 You did all the hard work debugging everything. Do you also want to take a stab at fixing it? I don't want to steal your glory |
Oh so that's interesting. I guess with a swap file saving is as simple as moving/copying it, whereas without you need to write the whole file (which is probably buffered) which explains why it happens more often on bigger files. And that's why neovim had multiple This is also why the bug started appearing more consistently around 250 lines, ~300 is where it starts to have 3 It all makes sense now. EDIT: As such, bigger and bigger files will raise more and more
haha thanks! You found the bug in the end. Gladly, I'll take a look later today and try to submit a fix if I can figure it out along with the codebase. |
Great! Don't be afraid to reach out if you have questions. A high level overview of the relevant parts:
So, the thing that needs fixing is how we read the files to send them to the interpreter. I don't really care that much about the performance of reading the markdown files specifically since it'd be incredibly rare for them to be above 1 MiB even. Maybe checking the length after finishing a read to see if they match, or reading twice, or something |
Thank you this was definitely helpful! Ok so I have two sort of solution I wanted to discuss. First solutionJust wait a few milliseconds before reloading files. On my machine 1ms is enough up to ~100kb files, after that bug reappears. Second solutionUse notify_debouncer_mini to debounce the events. I think this is a very good solution but:
Its up to you to evaluate those drawbacks, let me know if you want a PR with that to evaluate and test it. I think this is the most robust solution of the twos. It would make sense for a file watcher to include a configurable debounce timing. For a 160k file, the second solution also reduces the number of call to interpret_md to 2 instead of 22. Granted a 160k file should almost never be. |
This may be a dealbreaker with the second solution for Some iteration of the first solution should probably be good enough. I think going with a shorter delay while doing multiple checks should help to reduce latency. Also ideally we get a robust enough solution that we don't have to expose any additional configuration options |
I just thought that the notify doc also listed this lib https://docs.rs/notify-debouncer-full/latest/notify_debouncer_full/index.html which actually transmit original notify event. Alright then I'll send a PR with the first solution and try to improve it. |
That list of features does look like it addresses all of my issues :D |
Yeah I thought so 😄 , shall I try to use it? |
Go for it! |
Description
Sometimes when saving a file, the live reload will not render the whole file (screen 3) or just a blank screen (screen 2).
The initial render - when starting the app - is always ok (screen 1).
Triggering the reload again will end up in one of the 3 mentioned cases.
I noticed saving with modifications will more often lead to a correct render than saving without.
It seems that the larger the file the easier it is to reproduce. While testing, I started to encounter the issue with ~250 lines, but it was very rare.
I'd like to help more but couldn't find any log of why that happens, neither stdout/stderr or files.
From an outside point of view it seems to me like there is some non-determinism going on which causes inlyne to sort of exit early.
Additional details
Screenshots
Screen 1, working correctly on open
Screen 2, blank render on live reload
Screen 3, partially rendered after live reload (notice the size of the scrollbar compared to screen 1)
I run ubuntu with i3wm and neovim v0.9.4 (if that's any relevant).
$ uname -a Linux xubuntu 5.15.0-91-generic #101-Ubuntu SMP Tue Nov 14 13:30:08 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Also happens on main, 0.3.2
I mainly tested against the ratatui breaking change file and the other md files in the repo.
Please tell me if I can be of any more help!
EDIT: on version 0.2.1, I don't have any blank screen, sometimes a partial render (but very rare). Hope that helps.
The text was updated successfully, but these errors were encountered: