folding improvements #1428

hexasoftware · 2017-09-06T10:15:22Z

Improved fold syntax to include comments above, currently only working on funcs

hexasoftware · 2017-09-06T11:33:45Z

Actually there is some issue with syntax coloring and I notice the default folds seems odd

*note: the left most lines showing the folds

On the left i have the foldcolumn and with normal behaviour seems to be wrong. Now I'm not sure if this is a vim-go issue or if i have any other plugin performing this

When the fold on varconst is disabled the folds seems proper

*EDIT:
I realized this was an issue related to other plugin 'rainbow'

arp242 · 2017-09-06T16:02:35Z

FYI, I have a related PR to fold package-level coments: #1377. I've been waiting for that to be merged to do more work on the folding.

arp242 · 2017-09-06T18:52:57Z

I'm afraid I don't quite understand what you mean with your last comment btw. The screenshots are kind of confusing since there is so much going on, and I'm not sure which colour stuff ought to be.

At any rate, I tested this PR. I noticed that it also folds the function or type declaration. Is this intentional? Especially with var/const/type blocks (var ( ... )) you lose hat line, so you no longer know if it's a var or const, and while the standard form of a comment is "[name] [rest-of-comment]", many projects don't follow that convention, so you don't know what's being documented.

But I would personally also consider it annoying with just functions and the correct comment. The function signature is kind of important :-)

It doesn't seem to work with block comments btw:

/* asd
* asd
*/
func main() {

I get E490: No fold found when using zc.

It also noticed it doesn't work nested stuff like so:

// asd
// zxczxc
type (
    // asdf
    // zxcv
    foo interface {
        ASD()
    }
)

hexasoftware · 2017-09-06T18:57:32Z

I didn't made support for block comments yet, since I wanted to check viability on this as you mentioned that we lose var/const/type signature and I initial wrote this for funcs and ended up including all the rest, nested comments would not work since the way is being folded but i think we can figure something out to improve the regex,

The screenshots above if you look at the left most columns, it shows the foldlevels, and when a const (\n) or var(\n) appear it basically makes a fold for all text

to show folds:

set foldcolumn=2

And seems that fatih merged couple minutes ago, I will try to solve the conflicts and merge into mine

arp242 · 2017-09-06T18:59:55Z

Thanks @hexasoftware; my (now merged) PR added package_comment to the list in g:go_fold_enable; it's probably clearer if you rename your variable to something like declaration_comment or some such? Just to make it clearer what applies where.

Let me know if you need help. It's a good PR to add 👍

hexasoftware · 2017-09-06T19:11:45Z

I agree should be renamed, I've seen some references to same blocks as 'textobjs', refering to a func including its doc, but 'textobjs' wouldn't be intuitive

hexasoftware · 2017-09-06T19:28:59Z

@Carpetsmoker what do you think, should I create these per types? as in func_comment, type_comment varconst_comment? or for all? right now anything with a comment in a text next to it would fold

arp242 · 2017-09-06T22:28:35Z

what do you think, should I create these per types? as in func_comment, type_comment varconst_comment? or for all?

I think both approaches are fine.

I added the package comment separately because personally I don't want to fold function comments (as they're often useful), but do want to fold package comments (as they're often an introduction, and not useful to me).

So the question is, are there people who want to fold type comments but not function comments? Maybe? Maybe not? I don't know what the answer is to that :-)

Just go with what works well for you and/or is easiest to implement. It can always be changed later :-)

right now anything with a comment in a text next to it would fold

Yeah, that's probably not great :-) You can see in my PR on how to fix that: https://github.com/fatih/vim-go/pull/1377/files#diff-6bbebb95ce25cd40d0072544e2dad91eR418 – this will fold comments only if it's followed by the text package .

Another way to do it would be to use nextgroup; I'm not sure why I didn't use that there.

it will fold all grouped single comments and block comments

hexasoftware · 2017-09-06T23:56:35Z

My original goal was indeed to to fold both comment and func as a single fold, as I've seen in other languages some pretty folds with only descriptions but I wasn't able to solve the highlight problems as a 'func' contained keyword shows differently than a goDeclaration.

for this PR I just added folding for any kind of repeated comment either /.../ or repeated //.. which I've seen requested in other situations.

I would like the fold to include declarations in the future(in other option), I will check better on that when I have the time, as is not a 'conventional' fold

arp242 · 2017-09-07T04:39:12Z

for this PR I just added folding for any kind of repeated comment either /.../ or repeated //.. which I've seen requested in other situations.

That sounds like a reasonable place to start with.

I would like the fold to include declarations in the future(in other option)

I attempted to do that with this, but it doesn't quite work in all situations (open the test file).

arp242 · 2017-09-07T04:46:27Z

syntax/go.vim

+	" changed this to comments instead of declaration_comment
+  syn region    goComment           start="/\*" end="\*/" contains=@goCommentGroup,@Spell fold
+  syn match     goComment           "\v(^\s*//.*\n)+" contains=goGenerate,@goCommentGroup,@Spell fold
+endif


Looks good and works well in my testing, but could you move this next to the regular goComment lines on line 168? I think that makes more sense and would make it easier to maintain later on.

Would also be nice if you could remove the extra blank lines you added :-)

arp242 · 2017-09-07T04:46:50Z

syntax/go.vim

@@ -240,6 +246,13 @@ else
                        \ contains=ALLBUT,goParen,goBlock,goFunction,goTypeName,goReceiverType,goReceiverVar
 endif

+if s:fold_comment
+	" changed this to comments instead of declaration_comment


I think this comment can be removed?

arp242 · 2017-09-07T04:47:29Z

syntax/go.vim

@@ -452,4 +465,5 @@ syn sync minlines=500

 let b:current_syntax = "go"

-" vim: sw=2 ts=2 et


I think we want to keep the modeline; you you add this back please?

Oh, sry I guess I removed modeline within my tests down there and didn't realize, I was also trying to create a FoldText() function so in the cases we fold the declarations the text would contain the subject (func/type/const/var),

I made the suggested changes

hexasoftware · 2017-09-07T19:21:12Z

I attempted to do that with this, but it doesn't quite work in all situations (open the test file).

I tried, and this is meant to fold only the comments above the functions? the multiline comments would serve that purpose as well, unless indeed users don't want to fold all but only descriptions of certain types

the very first screenshots I posted was kind of my intention of fold funcs and was working except for the highlight (and probably other) issues

Disable folding of comments by default; it's probably a bit too much for most people.

hexasoftware · 2017-09-09T03:21:08Z

Keep in mind by leaving s:fold_comment = 0 I think there is no way for user to turn on?

arp242 · 2017-09-09T04:24:38Z

Ugh. Looks like I forgot to test that feature after I added it @hexasoftware >_< Good catch 👍 I fixed it now.

stdiopt added 2 commits September 6, 2017 06:52

added fold func to include comment

f3f3c07

there was some highlight issues

726e530

stdiopt added 2 commits September 6, 2017 20:46

Merge branch 'master' of http://github.com/fatih/vim-go

07206dc

reverted declaration_comment to just comment

894fea8

it will fold all grouped single comments and block comments

arp242 reviewed Sep 7, 2017

View reviewed changes

formated and moved comment fold to proper place

5bcafad

Disable by default; add docs.

18cd08e

Disable folding of comments by default; it's probably a bit too much for most people.

arp242 merged commit 9cce36b into fatih:master Sep 8, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

folding improvements #1428

folding improvements #1428

hexasoftware commented Sep 6, 2017

hexasoftware commented Sep 6, 2017 •

edited

Loading

arp242 commented Sep 6, 2017

arp242 commented Sep 6, 2017 •

edited

Loading

hexasoftware commented Sep 6, 2017 •

edited

Loading

arp242 commented Sep 6, 2017

hexasoftware commented Sep 6, 2017

hexasoftware commented Sep 6, 2017

arp242 commented Sep 6, 2017

hexasoftware commented Sep 6, 2017

arp242 commented Sep 7, 2017 •

edited

Loading

arp242 Sep 7, 2017

arp242 Sep 7, 2017

arp242 Sep 7, 2017

hexasoftware Sep 7, 2017

hexasoftware commented Sep 7, 2017

hexasoftware commented Sep 9, 2017

arp242 commented Sep 9, 2017

		@@ -452,4 +465,5 @@ syn sync minlines=500

		let b:current_syntax = "go"

		" vim: sw=2 ts=2 et

folding improvements #1428

folding improvements #1428

Conversation

hexasoftware commented Sep 6, 2017

hexasoftware commented Sep 6, 2017 • edited Loading

arp242 commented Sep 6, 2017

arp242 commented Sep 6, 2017 • edited Loading

hexasoftware commented Sep 6, 2017 • edited Loading

arp242 commented Sep 6, 2017

hexasoftware commented Sep 6, 2017

hexasoftware commented Sep 6, 2017

arp242 commented Sep 6, 2017

hexasoftware commented Sep 6, 2017

arp242 commented Sep 7, 2017 • edited Loading

arp242 Sep 7, 2017

Choose a reason for hiding this comment

arp242 Sep 7, 2017

Choose a reason for hiding this comment

arp242 Sep 7, 2017

Choose a reason for hiding this comment

hexasoftware Sep 7, 2017

Choose a reason for hiding this comment

hexasoftware commented Sep 7, 2017

hexasoftware commented Sep 9, 2017

arp242 commented Sep 9, 2017

hexasoftware commented Sep 6, 2017 •

edited

Loading

arp242 commented Sep 6, 2017 •

edited

Loading

hexasoftware commented Sep 6, 2017 •

edited

Loading

arp242 commented Sep 7, 2017 •

edited

Loading