masukomi / vim-markdown-folding

Fold markdown documents by section.
248 stars 43 forks source link

Unsolicited folds in fenced code blocks #1

Closed taketwo closed 11 years ago

taketwo commented 11 years ago

Hi, thanks for the great plugin! I have noticed that it does not treat fenced code blocks properly though. Consider the following document:

# Minimal C++ program

Here is a minimal C++ program which prints "Hello world!":

```cpp
#include <iostream>
void main()
{
  std::cout << "Hello world!";
}

The plugin treats `#include` directive as a section header and produces two folds:

1 # Minimal C++ program [4 lines]------------------------------------------ 6 # include [5 lines]-------------------------------------------

nelstrom commented 11 years ago

Thanks for reporting this. I'll look into it.

nelstrom commented 11 years ago

Please check out the fenced-code-blocks branch to try out a 1st draft solution. It has unit tests, but it could do with some user testing too. I'm concerned that it might have a negative impact on performance, so please report back if you find it to be in any way laggy.

taketwo commented 11 years ago

Thank you for the prompt response and fix! I have tried it with some of my documents and it worked as expected. The documents were not very large, about 200 lines at most, and I did not mention any lags. However, when I created a new document with 1000 lines and more than 50 code blocks, opening it (and folding with zi) took some small yet noticeable amount of time. With 3000 lines my CPU was at 100% for a while. I should mention that the current version from master branch opens the file instantly. To conclude, for my use-case the fix seems to be sufficient. For someone who works with really large files it will probably bring an unacceptable performance degradation.

nelstrom commented 11 years ago

I've figured out how to profile the performance of the folding script. Here's the bottleneck; the searchpairpos() function is computationally expensive. In my naive first attempt at a solution for this issue, I was calling searchpairpos() on every line of the file. This patch produces a big performance boost. Testing with a 1500 line .md file, it went from 5 seconds to 1 second.

Still room for improvement.

nelstrom commented 11 years ago

This patch further improves performance. Testing with the same 1500 line .md file, it's gone from 1.2 seconds to 0.2 seconds. That's more like it!

Instead of using searchpairpos(), I'm simply checking if the current line belongs to the syntax group 'markdownFold'. That introduces a dependency on vim-markdown, but I think that's acceptable.

taketwo commented 11 years ago

I think the dependency on vim-markdown is perfectly reasonable. There is a caveat in using the syntax group set by the plugin though. It has an option g:markdown_fenced_languages which allows to do proper syntax highlighting inside fenced blocks. In my case I have let g:markdown_fenced_languages = ['cpp', 'python'], and the simple example which I have posted in the beginning of this thread is not folded properly.

nelstrom commented 11 years ago

Ok, I think I've covered that now. Try the latest version on the fenced-code-blocks branch.

taketwo commented 11 years ago

That solved the issue!

Here is a new one:

# Minimal C++ program

Here is a minimal C++ program which prints "Hello world!":

```cpp
/*
Main function
=============
*/
#include <iostream>
void main()
{
  std::cout << "Hello world!";
}


This example is contrived, but you can imagine what will happen in a language like Haskell, where comment lines start with  two dashes.
nelstrom commented 11 years ago

Ok, f5ee87 should fix that.

taketwo commented 11 years ago

Works like a charm, thank you for the effort!

nelstrom commented 11 years ago

That's in the master branch now. Thanks for your help with this.