-
Notifications
You must be signed in to change notification settings - Fork 338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
improvement: thresholds for folding regions #7013
Conversation
- don't merge region with parent when if parent too small without it - allow to configure folding region threshold using server properties - make threshold work consistently
@@ -41,13 +42,13 @@ String hello(String str) >>region>>{ | |||
return "asssd"; | |||
}<<region<< | |||
|
|||
void openingCommentInStr() >>region>>{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we lose a region here? That should be 3 lines with all the {}
Same below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I counted only the lines that actually get folded, so:
def a = {
1
}
is one
def a =
1
is also one. Think it's a bit more intuitive than if these had two and one respectively. I can offset by one, so both count as 2, would that be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, let's try this solution. I don't really have preference and it looks sensible. Thanks for explaining!
val t = | ||
1 + 2 | ||
|
||
t match |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should have a region for match as weel
@@ -7,14 +7,14 @@ class A >>region>>{ | |||
}<<region<< | |||
|
|||
def noSpacing =>>region>> | |||
for>>region>>{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should still have this region as well, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thanks! This seems great ❤️ |
resolves: #6938