Added spacing when toggling block comments in JavaScript

[michaelgmcd]

When toggling and untoggling block comments in JavaScript (I haven't tested other languages), extra spaces are added to the beginning and end of the block each time.

This did not happen in the previous version of VSCode.

• VSCode Version: 1.14.1
• OS Version: MacOS Sierra

Steps to Reproduce:

1. Toggle a block comment in a section of JavaScript.
2. Untoggle it and notice the added spacing.

Reproduces without extensions: Yes

[cleidigh]

@mjbvz
I'm experiencing this with CSS files as well.

If no one is working on this already I will investigate and do a PR.

[mjbvz]

Thanks. Confirmed that this is not js specific. The same issue also shows up in c files and other language types

[cleidigh]

@mjbvz
@alexandrudima
Does "Thanks" mean and go ahead and do a PR in this? ;-)

I think this was introduced June eighth with
97105c2
adding spaces to block comments, the reverse operation did not take the spaces into account.
Will fix with go-ahead...

public static _createAddBlockCommentOperations(r: Range, startToken: string, endToken: string): editorCommon.IIdentifiedSingleEditOperation[] {
	var res: editorCommon.IIdentifiedSingleEditOperation[] = [];

	if (!Range.isEmpty(r)) {
		// Insert block comment start
		res.push(EditOperation.insert(new Position(r.startLineNumber, r.startColumn), startToken + ' '));

		// Insert block comment end
		res.push(EditOperation.insert(new Position(r.endLineNumber, r.endColumn), ' ' + endToken));
	} else {
		// Insert both continuously
		res.push(EditOperation.replace(new Range(
			r.startLineNumber, r.startColumn,
			r.endLineNumber, r.endColumn
		), startToken + '  ' + endToken));
	}

	return res;
}

[mjbvz]

@cleidigh Sure. @rebornix should be able to help with the PR or answer questions about the code

[cleidigh]

@rebornix
One potential fix is very simple, add one to the delete ranges for removing the comments.

public static _createRemoveBlockCommentOperations(r: Range, startToken: string, endToken: string): editorCommon.IIdentifiedSingleEditOperation[] {
var res: editorCommon.IIdentifiedSingleEditOperation[] = [];

	if (!Range.isEmpty(r)) {
		// Remove block comment start
		
		res.push(EditOperation.delete(new Range(
			r.startLineNumber, r.startColumn - startToken.length,
			r.startLineNumber, r.startColumn + 1
		)));

		// Remove block comment end
		res.push(EditOperation.delete(new Range(
			r.endLineNumber, r.endColumn - 1,
			r.endLineNumber, r.endColumn + endToken.length
		)));
	} else {

Personally I don't think it's a good idea to do these mathematics, you would need fix ups
everywhere you are doing the calculation on token length. The commit adding the spaces
is essentially hardcoded, in other words the Space is not part of the actual token.
This means everywhere you use the length of the token you have to do the manual adjustment,
that doesn't seem like a good idea.

Shouldn't the space be part of the tokens? Let me know how you want to handle it and I will do a PR
Cheers

[cleidigh]

@rebornix
Well I see the single-line comment adds the space separately and adjusts the math wherever it needs it.
I'm guessing this is the way you'd prefer to do it.

[cleidigh]

Fixed with PR #30818
All Languages