fix for #348: use onEnterSupport to better handle indenting around doc block comments

src/vs/editor/common/commands/shiftCommand.ts

@@ -9,6 +9,7 @@ import {Range} from 'vs/editor/common/core/range';
 import {Selection} from 'vs/editor/common/core/selection';
 import EditorCommon = require('vs/editor/common/editorCommon');
 import {CursorMoveHelper} from 'vs/editor/common/controller/cursorMoveHelper';
+import {getRawEnterActionAtPosition} from 'vs/editor/common/modes/supports/onEnter';
 
 export interface IShiftCommandOpts {
 	isUnshift: boolean;
@@ -50,19 +51,20 @@ export class ShiftCommand implements EditorCommon.ICommand {
 	}
 
 	public getEditOperations(model: EditorCommon.ITokenizedModel, builder: EditorCommon.IEditOperationBuilder): void {
-		var startLine = this._selection.startLineNumber,
-			endLine = this._selection.endLineNumber;
+		let startLine = this._selection.startLineNumber,
+			endLine = this._selection.endLineNumber,
+			_SPACE = ' '.charCodeAt(0);
 
 		if (this._selection.endColumn === 1 && startLine !== endLine) {
 			endLine = endLine - 1;
 		}
 
-		var lineNumber:number,
+		let lineNumber:number,
 			tabSize = this._opts.tabSize,
 			oneIndent = this._opts.oneIndent;
 
 		// indents[i] represents i * oneIndent
-		var indents: string[] = ['', oneIndent];
+		let indents: string[] = ['', oneIndent];
 
 		// if indenting or outdenting on a whitespace only line
 		if (this._selection.isEmpty()) {
@@ -71,15 +73,16 @@ export class ShiftCommand implements EditorCommon.ICommand {
 			}
 		}
 
-		for (lineNumber = startLine; lineNumber <= endLine; lineNumber++) {
-			var lineText = model.getLineContent(lineNumber);
-			var indentationEndIndex = Strings.firstNonWhitespaceIndex(lineText);
+		// keep track of previous line's "miss-alignment"
+		let previousLineExtraSpaces = 0, extraSpaces = 0;
+		for (lineNumber = startLine; lineNumber <= endLine; lineNumber++, previousLineExtraSpaces = extraSpaces) {
+			extraSpaces = 0;
+			let lineText = model.getLineContent(lineNumber);
+			let indentationEndIndex = Strings.firstNonWhitespaceIndex(lineText);
 
-			if (this._opts.isUnshift) {
-				if (lineText.length === 0 || indentationEndIndex === 0) {
-					// empty line or line with no leading whitespace => nothing to do
-					continue;
-				}
+			if (this._opts.isUnshift && (lineText.length === 0 || indentationEndIndex === 0)) {
+				// empty line or line with no leading whitespace => nothing to do
+				continue;
 			}
 
 			if (indentationEndIndex === -1) {
@@ -87,7 +90,45 @@ export class ShiftCommand implements EditorCommon.ICommand {
 				indentationEndIndex = lineText.length;
 			}
 
-			var desiredIndentCount: number;
+			if (lineNumber > 1) {
+				let contentStartVisibleColumn = CursorMoveHelper.visibleColumnFromColumn2(lineText, indentationEndIndex + 1, tabSize);
+				if (contentStartVisibleColumn % tabSize !== 0) {
+					// The current line is "miss-aligned", so let's see if this is expected...
+					// This can only happen when it has trailing commas in the indent
+					let enterAction = getRawEnterActionAtPosition(model, lineNumber - 1, model.getLineMaxColumn(lineNumber - 1));
+					if (enterAction) {
+						extraSpaces = previousLineExtraSpaces;
+						if (enterAction.appendText) {
+							for (let j = 0, lenJ = enterAction.appendText.length; j < lenJ && extraSpaces < tabSize; j++) {
+								if (enterAction.appendText.charCodeAt(j) === _SPACE) {
+									extraSpaces++;
+								} else {
+									break;
+								}
+							}
+						}
+						if (enterAction.removeText) {
+							extraSpaces = Math.max(0, extraSpaces - enterAction.removeText);
+						}
+
+						// Act as if `prefixSpaces` is not part of the indentation
+						for (let j = 0; j < extraSpaces; j++) {
+							if (indentationEndIndex === 0 || lineText.charCodeAt(indentationEndIndex - 1) !== _SPACE) {
+								break;
+							}
+							indentationEndIndex--;
+						}
+					}
+				}
+			}
+
+
+			if (this._opts.isUnshift && indentationEndIndex === 0) {
+				// line with no leading whitespace => nothing to do
+				continue;
+			}
+
+			let desiredIndentCount: number;
 			if (this._opts.isUnshift) {
 				desiredIndentCount = ShiftCommand.unshiftIndentCount(lineText, indentationEndIndex + 1, tabSize);
 			} else {
@@ -95,7 +136,7 @@ export class ShiftCommand implements EditorCommon.ICommand {
 			}
 
 			// Fill `indents`, as needed
-			for (var j = indents.length; j <= desiredIndentCount; j++) {
+			for (let j = indents.length; j <= desiredIndentCount; j++) {
 				indents[j] = indents[j-1] + oneIndent;
 			}
 

src/vs/editor/common/controller/oneCursor.ts

@@ -15,6 +15,7 @@ import {IEnterAction,IndentAction,IElectricAction} from 'vs/editor/common/modes'
 import {CursorMoveHelper, ICursorMoveHelperModel, IMoveResult} from 'vs/editor/common/controller/cursorMoveHelper';
 import EditorCommon = require('vs/editor/common/editorCommon');
 import Errors = require('vs/base/common/errors');
+import {getEnterActionAtPosition} from 'vs/editor/common/modes/supports/onEnter';
 
 export interface IPostOperationRunnable {
 	(ctx: IOneCursorOperationContext): void;
@@ -950,64 +951,6 @@ export class OneCursorOp {
 		return this._enter(cursor, true, ctx);
 	}
 
-	private static _getEnterActionAtPosition(model:EditorCommon.IModel, lineNumber:number, column:number): { enterAction: IEnterAction; indentation: string; } {
-		var lineText = model.getLineContent(lineNumber);
-		var lineContext = model.getLineContext(lineNumber);
-		var enterAction:IEnterAction;
-
-		if (model.getMode().onEnterSupport) {
-			try {
-				enterAction = model.getMode().onEnterSupport.onEnter(model, new Position(lineNumber, column));
-			} catch (e) {
-				Errors.onUnexpectedError(e);
-			}
-		}
-
-		if (!enterAction) {
-			if (model.getMode().electricCharacterSupport) {
-				try {
-					enterAction = model.getMode().electricCharacterSupport.onEnter(lineContext, column - 1);
-				} catch(e) {
-					Errors.onUnexpectedError(e);
-				}
-			}
-		} else {
-			// console.log('USING NEW INDENTATION LOGIC!');
-		}
-
-		var indentation = Strings.getLeadingWhitespace(lineText);
-		if (indentation.length > column - 1) {
-			indentation = indentation.substring(0, column - 1);
-		}
-
-		if (!enterAction) {
-			enterAction = {
-				indentAction: IndentAction.None,
-				appendText: '',
-			};
-		} else {
-			if(!enterAction.appendText) {
-				if (
-					(enterAction.indentAction === IndentAction.Indent) ||
-					(enterAction.indentAction === IndentAction.IndentOutdent)
-				) {
-					enterAction.appendText = '\t';
-				} else {
-					enterAction.appendText = '';
-				}
-			}
-		}
-
-		if (enterAction.removeText) {
-			indentation = indentation.substring(0, indentation.length - 1);
-		}
-
-		return {
-			enterAction: enterAction,
-			indentation: indentation
-		};
-	}
-
 	private static _enter(cursor:OneCursor, keepPosition: boolean, ctx: IOneCursorOperationContext, position?: EditorCommon.IEditorPosition, range?: EditorCommon.IEditorRange): boolean {
 		if (typeof position === 'undefined') {
 			position = cursor.getPosition();
@@ -1017,7 +960,7 @@ export class OneCursorOp {
 		}
 		ctx.shouldPushStackElementBefore = true;
 
-		var r = this._getEnterActionAtPosition(cursor.model, position.lineNumber, position.column);
+		var r = getEnterActionAtPosition(cursor.model, position.lineNumber, position.column);
 		var enterAction = r.enterAction;
 		var indentation = r.indentation;
 
@@ -1308,7 +1251,7 @@ export class OneCursorOp {
 			return '\t';
 		}
 
-		var r = this._getEnterActionAtPosition(cursor.model, lastLineNumber, cursor.model.getLineMaxColumn(lastLineNumber));
+		var r = getEnterActionAtPosition(cursor.model, lastLineNumber, cursor.model.getLineMaxColumn(lastLineNumber));
 
 		var indentation: string;
 		if (r.enterAction.indentAction === IndentAction.Outdent) {

src/vs/editor/common/modes/supports/onEnter.ts

@@ -9,6 +9,7 @@ import {IEnterAction, IndentAction, IOnEnterSupport, ILineContext, IMode} from '
 import EditorCommon = require('vs/editor/common/editorCommon');
 import Errors = require('vs/base/common/errors');
 import Strings = require('vs/base/common/strings');
+import {Position} from 'vs/editor/common/core/position';
 
 export interface IBracketPair {
 	open: string;
@@ -179,3 +180,66 @@ export class OnEnterSupport implements IOnEnterSupport {
 		}
 	}
 }
+
+export function getRawEnterActionAtPosition(model:EditorCommon.ITokenizedModel, lineNumber:number, column:number): IEnterAction {
+	let enterAction:IEnterAction;
+
+	if (model.getMode().onEnterSupport) {
+		try {
+			enterAction = model.getMode().onEnterSupport.onEnter(model, new Position(lineNumber, column));
+		} catch (e) {
+			Errors.onUnexpectedError(e);
+		}
+	}
+
+	if (!enterAction) {
+		if (model.getMode().electricCharacterSupport) {
+			let lineContext = model.getLineContext(lineNumber);
+			try {
+				enterAction = model.getMode().electricCharacterSupport.onEnter(lineContext, column - 1);
+			} catch(e) {
+				Errors.onUnexpectedError(e);
+			}
+		}
+	} else {
+		// console.log('USING NEW INDENTATION LOGIC!');
+	}
+
+	return enterAction;
+}
+
+export function getEnterActionAtPosition(model:EditorCommon.ITokenizedModel, lineNumber:number, column:number): { enterAction: IEnterAction; indentation: string; } {
+	let lineText = model.getLineContent(lineNumber);
+	let indentation = Strings.getLeadingWhitespace(lineText);
+	if (indentation.length > column - 1) {
+		indentation = indentation.substring(0, column - 1);
+	}
+
+	let enterAction = getRawEnterActionAtPosition(model, lineNumber, column);
+	if (!enterAction) {
+		enterAction = {
+			indentAction: IndentAction.None,
+			appendText: '',
+		};
+	} else {
+		if(!enterAction.appendText) {
+			if (
+				(enterAction.indentAction === IndentAction.Indent) ||
+				(enterAction.indentAction === IndentAction.IndentOutdent)
+			) {
+				enterAction.appendText = '\t';
+			} else {
+				enterAction.appendText = '';
+			}
+		}
+	}
+
+	if (enterAction.removeText) {
+		indentation = indentation.substring(0, indentation.length - 1);
+	}
+
+	return {
+		enterAction: enterAction,
+		indentation: indentation
+	};
+}

src/vs/editor/test/common/commands/shiftCommand.test.ts

@@ -11,6 +11,8 @@ import EditorCommon = require('vs/editor/common/editorCommon');
 import {withEditorModel} from 'vs/editor/test/common/editorTestUtils';
 import {Selection} from 'vs/editor/common/core/selection';
 import {Cursor} from 'vs/editor/common/controller/cursor';
+import * as Modes from 'vs/editor/common/modes';
+import {OnEnterSupport} from 'vs/editor/common/modes/supports/onEnter';
 
 function testShiftCommand(lines: string[], selection: Selection, expectedLines: string[], expectedSelection: Selection): void {
 	TU.testCommand(lines, null, selection, (sel) => new ShiftCommand(sel, {
@@ -28,6 +30,69 @@ function testUnshiftCommand(lines: string[], selection: Selection, expectedLines
 	}), expectedLines, expectedSelection);
 }
 
+class DocBlockCommentMode implements Modes.IMode {
+
+	public onEnterSupport: Modes.IOnEnterSupport;
+
+	constructor() {
+		this.onEnterSupport = new OnEnterSupport(this.getId(), {
+			brackets: [
+				{ open: '(', close: ')' },
+				{ open: '{', close: '}' },
+				{ open: '[', close: ']' }
+			],
+			regExpRules: [
+				{
+					// e.g. /** | */
+					beforeText: /^\s*\/\*\*(?!\/)([^\*]|\*(?!\/))*$/,
+					afterText: /^\s*\*\/$/,
+					action: { indentAction: Modes.IndentAction.IndentOutdent, appendText: ' * ' }
+				},
+				{
+					// e.g. /** ...|
+					beforeText: /^\s*\/\*\*(?!\/)([^\*]|\*(?!\/))*$/,
+					action: { indentAction: Modes.IndentAction.None, appendText: ' * ' }
+				},
+				{
+					// e.g.  * ...|
+					beforeText: /^(\t|(\ \ ))*\ \*\ ([^\*]|\*(?!\/))*$/,
+					action: { indentAction: Modes.IndentAction.None, appendText: '* ' }
+				},
+				{
+					// e.g.  */|
+					beforeText: /^(\t|(\ \ ))*\ \*\/\s*$/,
+					action: { indentAction: Modes.IndentAction.None, removeText: 1 }
+				}
+			]
+		});
+	}
+
+	public getId(): string {
+		return 'docBlockCommentMode';
+	}
+
+	public toSimplifiedMode(): Modes.IMode {
+		return this;
+	}
+
+}
+
+function testShiftCommandInDocBlockCommentMode(lines: string[], selection: Selection, expectedLines: string[], expectedSelection: Selection): void {
+	TU.testCommand(lines, new DocBlockCommentMode(), selection, (sel) => new ShiftCommand(sel, {
+		isUnshift: false,
+		tabSize: 4,
+		oneIndent: '\t'
+	}), expectedLines, expectedSelection);
+}
+
+function testUnshiftCommandInDocBlockCommentMode(lines: string[], selection: Selection, expectedLines: string[], expectedSelection: Selection): void {
+	TU.testCommand(lines, new DocBlockCommentMode(), selection, (sel) => new ShiftCommand(sel, {
+		isUnshift: true,
+		tabSize: 4,
+		oneIndent: '\t'
+	}), expectedLines, expectedSelection);
+}
+
 suite('Editor Commands - ShiftCommand', () => {
 
 	// --------- shift
@@ -427,6 +492,65 @@ suite('Editor Commands - ShiftCommand', () => {
 		);
 	});
 
+	test('issue #348: indenting around doc block comments', () => {
+		testShiftCommandInDocBlockCommentMode(
+			[
+				'',
+				'/**',
+				' * a doc comment',
+				' */',
+				'function hello() {}'
+			],
+			new Selection(1,1,5,20),
+			[
+				'\t',
+				'\t/**',
+				'\t * a doc comment',
+				'\t */',
+				'\tfunction hello() {}'
+			],
+			new Selection(1,2,5,21)
+		);
+
+		testUnshiftCommandInDocBlockCommentMode(
+			[
+				'',
+				'/**',
+				' * a doc comment',
+				' */',
+				'function hello() {}'
+			],
+			new Selection(1,1,5,20),
+			[
+				'',
+				'/**',
+				' * a doc comment',
+				' */',
+				'function hello() {}'
+			],
+			new Selection(1,1,5,20)
+		);
+
+		testUnshiftCommandInDocBlockCommentMode(
+			[
+				'\t',
+				'\t/**',
+				'\t * a doc comment',
+				'\t */',
+				'\tfunction hello() {}'
+			],
+			new Selection(1,1,5,21),
+			[
+				'',
+				'/**',
+				' * a doc comment',
+				' */',
+				'function hello() {}'
+			],
+			new Selection(1,1,5,20)
+		);
+	});
+
 	test('bug #16815:Shift+Tab doesn\'t go back to tabstop', () => {
 
 		var repeatStr = (str:string, cnt:number): string => {