StanAngeloff / php.vim

An up-to-date Vim syntax for PHP (7.x supported)
477 stars 69 forks source link

Fold enhancements #22

Closed swekaj closed 10 years ago

swekaj commented 10 years ago

I've added some improvements to the folding.

For the simple folding method (folding all pairs of { }) I just added ( ) and [ ] as folded pairs so arrays can be folded.

The advanced folding (php_folding==1) got a lot more improvements:

There are a couple of bugs I couldn't figure out how to fix:

  1. Closing } highlighting. I could not figure out how to get the closing } in try/catch/finally and if/elseif/else to highlight as a delimiter when they were followed by the next block's keyword on the same line, e.g.
    if ($expr) {
        // true
    } else {
        // false
    }
  1. I couldn't figure out how to get successive case fall-throughs to fold together, only the last case statement will be in the fold, leaving the previous ones unfolded. e.g.
    switch ($expr) {
        case 0: // fall-through, not folded.
        case 1: // fall-through, not folded.
        case 2: // do something, folded.
            // code
            break;
    }
StanAngeloff commented 10 years ago

I've merged this locally and gave it a try. Highlighting the closing } incorrectly is a bummer. I've also lost highlighting of the if, else, etc. keywords.

I have been thinking of taking a rather radical approach and eliminate all folding support from the syntax. There is an excellent plug-in at rayburgemeestre/phpfolding.vim. Does it do what you are trying to accomplish here? Perhaps the way to go forward is to 'borrow' the indentation code from that plug-in?

swekaj commented 10 years ago

I've merged this locally and gave it a try. Highlighting the closing } incorrectly is a bummer. I've also lost highlighting of the if, else, etc. keywords

Can you show an example of where you've lost the highlighting? I thought I had written the matches and regions such that they'd highlight everything in the proper context (i.e., a random if won't be matched, but one followed by condition will). I had to remove the keyword definitions when folding since the keywords were taking priority over the regions and preventing the regions from matching.

I have been thinking of taking a rather radical approach and eliminate all folding support from the syntax. There is an excellent plug-in at rayburgemeestre/phpfolding.vim. Does it do what you are trying to accomplish here? Perhaps the way to go forward is to 'borrow' the indentation code from that plug-in?

That plug-in only folds functions and class properties. I was looking to fold most control structures, arrays (which I just realized I forgot to add support for), and HEREDOCs in addition to functions.

I can understand why you'd want to leave folding out of the syntax file. After having done some work on it, it is limited in ways that I don't think a fold function would be.

If, though, you're fine with adding this then I can update the HEREDOCs with what you mentioned in the line-comment, though I may have some questions about that.

StanAngeloff commented 10 years ago

Here are a couple of screenshots of a method after the merge (colour scheme vim-zend55):

php_folding=0 0

php_folding=1 1

php_folding=2 2

Before the merge only the opening { and closing } brackets were discoloured. After the merge the if has lost highlighting and is no longer phpKeyword but rather phpRegion.

If we can find a way to highlight if as a phpKeyword inside the fold region when php_folding=1 then I'd be very happy to merge this awesome pull request. I'm happy to look at applying the HiFold workaround to reduce the duplication.


BTW, I am in GMT+2. That may make it difficult to move the pull faster if you are in a different timezone.

swekaj commented 10 years ago

BTW, I am in GMT+2. That may make it difficult to move the pull faster if you are in a different timezone.

I guessed as much, I'm in GMT-7

I'll work on fixing the issue and adding folding for arrays today/tonight.

swekaj commented 10 years ago

I fixed the issue you discovered. I wasn't including the proper syntax groups in the contains= lists.

I have also used the method you described here to reduce the code duplication in folding the HEREDOCs.

I think ultimately I will have to write a folding function to properly fold arrays (both array() and [ ]). I could get some to fold, but couldn't find a good way to get everything fold and still be functional.

Unless you've noticed anything else or I've missed something, I think this is ready to be merged.

StanAngeloff commented 10 years ago

Merged, thanks for the awesome work!