[folding] should not fold whitespace after function

[Tyriar]

Consider the Python script:

def a:
  pass



def b:
  pass

Currently all whitespace following the pass in a is collapsed with it:

This will make it the code harder to read in some cases as there is no space between the functions are the author intended, making a look more like part of b.

[aeschli]

To implement this we need to know more about the underlying language. Does a definition end with the last non-whitespace line, or with a line of the same indent. The current implementation is tuned for the JavaScript case:

function alwaysNull() {
  return 0;

}

[Tyriar]

This is something that can wait until language-awareness comes about. The def ends on the last non-whitespace line that is of the same indent level (or lower):

def a:
  something


  something_else # a ends here


def b: # since there was a deintent here

def a:
  something


  def b:
    something_else # a ends here


def b: # since there was a deintent here

For both these cases I would expect:

def a: [...]


def b: # since there was a deintent here

[Tyriar]

As @inergy points out on #4968, adding a newline at the end of the folded section will immediately add the new line to the folded block.

[NiklasRosenstein]

Is there a fix in sight? This *The current folding behaviour is super annoying 😅

[NiklasRosenstein]

Any update on this? How can people work with this folding behaviour? I switched to Atom not long after my last comment but switched back to VSCode recently because Atom is so slow. The folding behaviour is literally the only thing I was switching to Atom in the first place.

If anyone has a hint on where the behaviour can be changed, I might just look into submitting a patch myself. It's just hard to find your way around without any idea how VSCode is structured etc.

[NiklasRosenstein]

Bump

[ZombieProtectionAgency]

Is the post-fold whitespace folding a different issue than the collapsing of trailing whitespace?

[NiklasRosenstein]

What's the difference of whitespace that follows a folding and trailing whitespace? (except if you mean whitespace trailing in a line not trailing a block like empty newlines?)

[ZombieProtectionAgency]

When you add new lines after an already folded block the folded block will gobble them up as you add them

[NiklasRosenstein]

I don't know if that is a separate issue, but it is certainly linked. The block fold shouldn't consume any newlines after the code block anyway, so if that's fixed, it wouldn't gobble them up anymore.

[DollarAkshay]

IS this bug still not fixed ?

[NiklasRosenstein]

Nope. And its still super annoying. I'd give it a shot if I had an idea where to start, but no one seems to be interested in that either.

[Satak]

+1 for fixing this.

[feliperyan]

If someone can point me in the right direction I will work on this - it's really disrupting my workflow with Python

[NiklasRosenstein]

NiklasRosenstein 15 Oct 2016 -- If anyone has a hint on where the behaviour can be changed, I might just look into submitting a patch myself. It's just hard to find your way around without any idea how VSCode is structured etc.

NiklasRosenstein 19 Dec 2016 -- I'd give it a shot if I had an idea where to start, but no one seems to be interested in that either.

I don't think that's ever going to happen, @feliperyan

[Tyriar]

I believe this is blocked on #3422 which will involve a fair bit of work. Just keep adding 👍's to the issues and they will eventually bubble to the top of the sorted list which is one of the ways we prioritize.

[spitis]

Ugly, but better than nothing, workaround:

Add an empty comment (#) in between functions you want to fold, and they will act as whitespace upon folding.

[NiklasRosenstein]

I'd rather have the ugly folding rather than ugly code.

…

On 26 February 2017 at 16:41, spitis ***@***.***> wrote: Ugly, but better than nothing, workaround: Add an empty comment (#) in between functions you want to fold, and they will act as whitespace upon folding. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3353 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABQeJhMa4jNjAfPciyCutDoW1HDzK4ZJks5rgZ0mgaJpZM4Hgzak> .

-- Niklas Rosenstein https://niklasrosenstein.com

[acls]

The fix seems rather simple: don't fold newlines. Newlines at the end of a js function. eg:

function alwaysNull() {
  return 0;

}

is a formatting issue that shouldn't be hidden by the editor.

[Tyriar]

@acls it's not simple though as maybe people, including myself, sometimes break up chunks of code within functions. To give a real example: https://github.com/sourcelair/xterm.js/blob/2221f70ff05ba2af42ee0d26bed2f75dafe2d116/src/Linkifier.ts#L136

  private _addLinkMatcherToList(matcher: LinkMatcher): void {
    if (this._linkMatchers.length === 0) {
      this._linkMatchers.push(matcher);
      return;
    }

    for (let i = this._linkMatchers.length - 1; i >= 0; i--) {
      if (matcher.priority <= this._linkMatchers[i].priority) {
        this._linkMatchers.splice(i + 1, 0, matcher);
        return;
      }
    }

    this._linkMatchers.splice(0, 0, matcher);
  }

This is perfectly valid code but would fold like this:

  private _addLinkMatcherToList(matcher: LinkMatcher): void { ...

    for (let i = this._linkMatchers.length - 1; i >= 0; i--) {
      if (matcher.priority <= this._linkMatchers[i].priority) {
        this._linkMatchers.splice(i + 1, 0, matcher);
        return;
      }
    }

    this._linkMatchers.splice(0, 0, matcher);
  }

Going forward with that solution would be a big regression. We need language-aware folding support to be implemented #3422.

[acls]

Oops, I meant, don't fold trailing newlines. So you would find the end of the fold and then remove any empty lines.

[Tyriar]

@aeschli could we do this by ignoring empty or whitespace-only lines when considering where where the fold should end? For example:

  private _addLinkMatcherToList(matcher: LinkMatcher): void { 
    if (this._linkMatchers.length === 0) {
      this._linkMatchers.push(matcher);
      return;
    }

    // ^ Skip above line as it's empty
    for (let i = this._linkMatchers.length - 1; i >= 0; i--) {
      if (matcher.priority <= this._linkMatchers[i].priority) {
        this._linkMatchers.splice(i + 1, 0, matcher);
        return;
      }
    }

    // ^ Skip above line as it's empty
    this._linkMatchers.splice(0, 0, matcher);
  } // ^ Fold found

For python you could do this as well, for the following block:

def foo:
  a = 1

  pass



def bar:
  pass

We can determine the right folding level by simply ignoring the empty lines so we're essentially checking this:

def foo:
  a = 1
  pass
def bar: # ^ Fold found
  pass

[acls]

I made PR #26258 last night, shortly after my last comment, that I think fixes this issue.

[aeschli]

I don't think there's a strategy that pleases both the C-style and the Python style language camps.

I doubt the C-style users will accept the argument that the remaining new line is a formatting issue.
I think we need to specify the language style in the language configuration. Also, see #25365 (comment)

[acls]

I'm in the C-style camp and if the rest of my campers don't like it, then wrap the one line change in a config setting. The vscode source is in the C-style camp and it has a meager 6 instances where there is an empty line before the closing bracket.

I think language specific folding is needed for some things, but the default handling of trailing whitespace isn't one of them.

Since vscode is similar to Sublime and Atom, I'll add that both of them handle folding the same as in #26258.

[acls]

Don't get me wrong, I think language aware folding is needed and should be able to collapse this:

function alwaysNull() {
  return 0;

}

to this:

function alwaysNull() { ...
}

or even this:

function alwaysNull() { ... }

But the default indention based should collapse it to:

function alwaysNull() { ...

}

The reason being is that Python and other indention based languages are just that, indention based. And it's obviously a problem in those languages to be collapsing the whitespace.

[rkeithhill]

+1 for collapsing to a single line - when the opening { is on the first line:

function alwaysNull() { ... }

[flamders]

Hi guys, as I understand a patch exist that fixes the worst problem #26258

I run on Ubuntu 16.04 (snap install). Can you perhaps give me a pointer for applying that patch? Thx

[acls]

I don't know about the snap install, but I do know the easiest way to apply the patch is to push the Merge pull request button. Can someone please just push it.

All sarcasm aside can someone merge #26258 or give a good reason as to why it hasn't been merge. The only reason I've heard so far is that c-style camp wouldn't accept it as a formatting issue. I recently found clang-format, a tool for formatting c-style languages.

It formats this c code:

int main(int argc, char **argv) {
  printf("hello ");
  printf("world\n");


}

into this:

int main(int argc, char **argv) {
  printf("hello ");
  printf("world\n");
}

And if formats this javascript:

function foo() {
  var result = 0;
  return result;


}

into this

function foo() {
  var result = 0;
  return result;
}

clang-format doesn't have a configuration option to keep empty lines before a closing brace. So my initial assertion is true it is a formatting issue.

[regexus]

I also would like to know why this issue considered as belonging to language-aware folding. Not ignore empty lines at the end of the block is just a bug that current folding behavior has and it should be fixed before the big language-aware folding thing is done.
The patch from acls works as expected, without any problems. Please merge it. It will make python programmers happy and will probably not affect well formatted c-like code at all. Please look at the comparison images in my comment on the pull request page #26258 for more info.

[Shamtam]

+1 to merging the fix in #26258 into the master branch... I've resorted to building from repository and using the OSS version, but would much prefer to be back on the Insider's edition build to get the regular updates without having to re-build. This is pretty much the only thing keeping me from reverting back to a regular release channel.

[aeschli]

I pushed a new language configuration setting to distinguish C-style block language from offSide style languages:

	"folding": {
		"offSide": true
	}

I added the section to corresponding languages and adapted the indentation based folding to use the new setting.