live preview async timer

[cosmicexplorer]

This takes the *-live-preview-* functionality and turns it all async with an idle timer, as you suggested. Instead of using the markdown-do-sync-or-async macro as in the previous attempt, I just rewrote the code that invokes markdown into a single small function.

This requires lexical-binding to be on to use sentinels, so it has #62 merged into it. lexical-binding is supported since emacs 24.1, but if that's an issue, I can probably refactor those out into buffer-local defvars.

I closed #56 because this is really far better than that.

This pull request makes the preview process a lot faster and is more convenient than having to explicitly save on each preview; however, the defcustom markdown-live-preview-do-sync, when turned on, maintains the previous functionality.

[cosmicexplorer]

That's really weird; the CI is complaining that emacs wasn't compiled with libxml2? That was definitely working previously. Not sure what's up.

[jrblevin]

Maybe this is a problem with Emacs version manager? There is a seemingly-related open issue... This is what I use on Travis for installing Emacs.

CC: @syohex

[cosmicexplorer]

Thanks for checking this out; it is weird that libxml2 isn't there (although it was in other builds; maybe that build was on a different server, and it's arbitrary whether or not the build server has libxml2 compiled in? Cause it definitely didn't have this issue before (the previous test failures were for different issues).

To mitigate this, one thing we could do is, in addition to (require 'eww nil t), test (fboundp 'libxml-parse-html-region). That's what eww does when it raises its error. Just skipping them if there's no libxml2 wouldn't really fix the issue, since if we end up getting a server which doesn't have libxml2 compiled in, it would just skip running the tests, which is not what we want at all (imo it's worse than just having them fail). However, we could at least throw up an informative error message? Not sure what's best to do here.

[jrblevin]

That is very strange. I thought the error was due to some new functionality, not things that were already there before.

Independent of whether the CI server is happy with it, the tests pass locally on my end. I'm still playing around with everything. Very cool so far--thanks! I'll also think about how to best handle the error.

[cosmicexplorer]

Absolutely! Tell me if there any more types of tests you'd like me to add. I'll have less time than in the past few weeks because I'm back at school, but I should have plenty to fix things up.

[syohex]

• https://github.com/cosmicexplorer/markdown-mode/blob/feat/live-preview-async-timer/tests/markdown-test.el#L3293
• https://github.com/cosmicexplorer/markdown-mode/blob/feat/live-preview-async-timer/tests/markdown-test.el#L3327

I suppose above lines should also check (fboundp 'libxml-parse-html-region) for testing eww tests only on Emacs where has libxml2 features.

[cosmicexplorer]

Oops, I pushed that one too fast. It doesn't pass on 24.3, which I'm working on. 24.4 and 24.5 still don't have libxml2, for some reason. Sorry about that, fixing now.

[cosmicexplorer]

The test failures were due to not checking for (fboundp 'libxml-parse-html-region), so I've added those checks as well, which happens to solve test failures on 24.4 and 24.5; however, this doesn't mean we've solved the EVM problem, since it just skips the libxml check (and as you can see in the CI log, the build failed when EVM was just installing on 24.5...that concerns me a bit especially since I'm not familiar with why that would happen). The tests pass on my local builds of 24.3/4/5 (and 25.1), but we can definitely sit on this until we figure out the EVM problem.

What I just added in 6702e69 is thoroughly described in the commit message, but it essentially ensures that an eww buffer window, if it exists, is selected when eww-open-file renders. eww uses the width of the selected window to determine line breaks and other things, so it's important that it does that with the dimensions of the window actually displaying its content. Previously, if your markdown buffer window and the eww buffer window were different sizes, eww would either render too thin (hardly noticeable) or too thick (looks very bad).

[syohex]

I suppose tests on Emacs 24.5 are failed by network issue. So they may be passed by rerunning tests. You can rerun tests by some commit and push(For example, squashing some commits).

[cosmicexplorer]

Ok, I've also just ensured that the first eww (or whatever) window displayed has the correct display width, in addition to any subsequent exports. I added testing for that case, and squashed the commits. The CI test on 24.3 failed during the async exports test; this doesn't happen on my local machine, so it's likely a nondeterministic result of not choosing a long enough delay for the async testing. I think markdown-test/test-window-usage-live-preview looks extremely hairy already, so when I refactor that to make a little bit more sense I'll take a look at those async tests and see if I can ensure they'll complete without race conditions. I'll also squash a few more commits.

To sum up:

1. We have async export now using an idle timer, in addition to synchronous export.
2. For both sync and async export, the eww (or other) buffer is rendered into the same window it is finally displayed into, which means the rendered output isn't too wide or too thin.

[jrblevin]

FYI I forced the failing test to run again and it passed.

I've had an issue with asynchronous live preview that I haven't been able to reliably reproduce. Sometimes the Markdown process seems to hang and the buffer stops updating. Many times it's after the initial preview, with no subsequent updates. When this happens, I check the output html file and it is empty (0 bytes), while the Markdown process is still running.

Another thing: currently it doesn't handle the markdown-command-needs-filename setting. Perhaps you could reuse the code from the existing routines that handle Markdown processing (which handle both cases)?

Edit: Regarding the EVM issue, I prefer to have the CI tests pass even when those tests don't run (as you have done). The alternative--having them always fail (due to the libxml2 issue)--is uninformative. At least this way if a patch introduces a regression in tests that do run, we'll know. I view the CI server as a compliment to running the tests locally as well, not a substitute for it.

[cosmicexplorer]

Ok, I'll try to reproduce that, and I'll check out markdown-command-needs-filename. Thanks for clarifying the use of the CI.

[syohex]

Could you separate this PR(lexical binding, live preview feature etc) after #95(Some part of this change conflict with #95) ?

[cosmicexplorer]

#62 is cleanly merged into this, which has the lexical-binding stuff done independently. Is that what you're looking for?

[syohex]

Is that what you're looking for ?

Yes. I missed that closed PR.

This PR implements multiple features, enabling lexical-binding and live preview. It is difficult to review big PR.

[cosmicexplorer]

Ok, I'll update #62 and reopen it when I address the other comments made in this thread.

[syohex]

Thanks