vermiculus / sx.el

Stack Exchange for Emacs
http://stackapps.com/q/3950
709 stars 40 forks source link

Spin writing undo-buffer? #312

Closed rebcabin closed 8 years ago

rebcabin commented 8 years ago

Repro ... go to cstheory stackexchange page, arrow down to the fifth message, press RET. Emacs goes into hard loop. After several minutes, I get a message about undo on one of the sx buffers being too large. This message will repeat every few minutes with a different number of bytes. I can't remember the exact name of the buffer, and can't easily check it because it takes a long time for the message to appear (with fans blowing and other kinds of meltdown dramatics on the computer :)

Warning (undo): Buffer `*sx-<something>*' undo info was 47119441 bytes long.
 The undo info was discarded because it exceeded `undo-outer-limit'.

 This is normal if you executed a command that made a huge change
 to the buffer.  In that case, to prevent similar problems in the
 future, set `undo-outer-limit' to a value that is large enough to
 cover the maximum size of normal changes you expect a single
 command to make, but not so large that it might exceed the
 maximum memory allotted to Emacs.

 If you did not execute any such command, the situation is
 probably due to a bug and you should report it.

 You can disable the popping up of this buffer by adding the entry
 (undo discard-info) to the user option `warning-suppress-types'.

EDIT: I deleted the entire ~/.emacs.d/.sx directory and tried again. Sometimes you have to go down further than the fifth message, but I can still get this easily to spin and jam emacs.

The last message in the minibuffer is "Wrote /Users/foobar/.emacs.d/.sx/read-messages.el"

vermiculus commented 8 years ago

I've been able to track it to this line. I'm still working on this – it's a doosie :smile:

vermiculus commented 8 years ago

markdown-match-pre-blocks in sx-question-mode--skip-and-fontify-pre isn't putting point at the end of the code block so we never move past it.

screen shot 2016-01-16 at 5 06 40 pm
vermiculus commented 8 years ago

Could be related to #311

vermiculus commented 8 years ago

@jrblevin – any idea what might be going on here?

jrblevin commented 8 years ago

markdown-match-pre-blocks is just a call to markdown-match-propertized-text which should be moving past any identified pre blocks. See this line: https://github.com/jrblevin/markdown-mode/blob/master/markdown-mode.el#L2517

Also, I don't understand the screenshot above. The right side seems to be a snippet from sx, but what is on the left side?

Could you perhaps send a file as a test case (along with an initial point position and limit) where markdown-match-pre-blocks fails to move the point past the block?

Just a hunch, but markdown-mode now uses text properties during syntax analysis to mark pre blocks and certain other constructs. Then the related font-lock matching functions just look for those text properties. Since you're linking pretty deep within markdown-mode to run the matching functions directly, you might be neglecting to call the syntax-propertize functions first. Again, just a hunch based on recent development. I haven't really read the sx code.

vermiculus commented 8 years ago

The left side is a question buffer in the process of creation. It oddly stops fontifying in the middle of the block, which I thought odd.

I'll look into our use of markdown-mode's fanciness later this week unless @Malabarba beats me to it :smile: this is an interesting bug. It might happen only for sufficiently large code blocks…

Malabarba commented 8 years ago

Ok, I've added a clause to disable undo because undo makes no sense in question-mode. There's still an issue with code-blocks which I'm looking into now.

Malabarba commented 8 years ago

@jrblevin I think the problem is that we are calling markdown-match-pre-blocks on a buffer that is not in markdown-mode (and it has changed since markdown-mode was last active).

Steps to reproduce:

  1. Turn on markdown-mode and paste the following text:

    I give this new interface the benefit of the doubt
    
       git push -u origin B
  2. Place point at the start of the 3rd line, and run (markdown-match-pre-blocks (line-end-position)). Note that it works correctly.
  3. Switch to fundamental-mode, and paste the following text at the TOP of the buffer (before everything else):

    bogus
    bogus
    bogus
    bogus
    bogus
    bogus
    bogus
    bogus
    bogus
  4. Again, place point at the start of that line, and run (markdown-match-pre-blocks (line-end-position)). It will move the point backwards.

I understand you probably didn't expect people to be using your functions outside of markdown-mode, but it's pretty useful for us. :-)

jrblevin commented 8 years ago

Markdown mode now caches the bounds of code blocks in text properties (in this case, one called markdown-pre) during syntax analysis for efficiency. Then later, during font lock phase (where markdown-match-pre-blocks is intended to be called), it can simply read the cached bounds of any code blocks. These bounds are used by several other functions to identify when the point is inside a code block, so calculating the repeatedly becomes costly.

In the example, I think this is what is happening: When markdown-mode is active, it determines and stores the bounds of the pre block, in list form (53 77). So, the block starts at position 53 and ends at position 77. Moving past the block in step 2 means moving the point to position 78, just after the pre block. When you turn off markdown-mode in step 3, those text properties remain (maybe this is a bug--are there hooks that run when major modes are disabled?) but they don't get updated when you insert text in step 3 (as would happen if markdown-mode were still active). So, when you call markdown-match-pre-blocks in step 4, it sees the cached bounds and moves to point 78. Unknown to the function, some other text was inserted above and so point 78 is no longer in the pre block, but results in moving backwards.

As you have seen, I have written markdown-mode as a major mode, not a library for parsing Markdown files. So, when this function is called out of the usual progression, it does not behave as intended. I need to think more about how to balance the efficiency of the mode (not re-parsing each time certain functions are called) with the flexibility of allowing external dependencies.

I have never used sx so I don't know exactly how it works, but here are some thoughts on how to proceed for now. If the text inserted above is considered to be markdown-formatted as well, you could delay enabling of markdown-mode until all text is present, so that the buffer is parsed correctly, then call the functions you need. Another option is that you could dig a little deeper into markdown-mode and call the markdown-syntax-propertize function on the region after any buffer modifications have been made but before calling the -match- functions. This will set the text properties correctly for later parsing. They should remain correct as long as no text is inserted above.

Malabarba commented 8 years ago

Thanks @jrblevin. That's more or less what I was expecting. I know we've been abusing markdown-mode a bit, but it's the closest thing we have to a good markdown renderer. :-)

I think calling markdown-syntax-propertize will be all we need. I'll report back after trying it.

jrblevin commented 8 years ago

I'm glad to see that worked! Hopefully big structural changes to markdown-mode like this one will be infrequent.