put commenting ranges affordance into line decorations area.

src/vs/editor/common/config/editorOptions.ts

@@ -1543,7 +1543,7 @@ function _stringSet<T>(value: T, defaultValue: T, allowedValues: T[]): T {
 	return value;
 }
 
-function _clampedInt(value: any, defaultValue: number, minimum: number, maximum: number): number {
+export function _clampedInt(value: any, defaultValue: number, minimum: number, maximum: number): number {
 	let r: number;
 	if (typeof value === 'undefined') {
 		r = defaultValue;

src/vs/workbench/parts/comments/electron-browser/commentGlyphWidget.ts

@@ -4,37 +4,38 @@
  *--------------------------------------------------------------------------------------------*/
 'use strict';
 
-import { ICodeEditor, IContentWidget, IContentWidgetPosition, ContentWidgetPositionPreference } from 'vs/editor/browser/editorBrowser';
+import { ICodeEditor, IContentWidgetPosition, ContentWidgetPositionPreference } from 'vs/editor/browser/editorBrowser';
+import { ModelDecorationOptions } from 'vs/editor/common/model/textModel';
 
-export class CommentGlyphWidget implements IContentWidget {
-	private _id: string;
+export class CommentGlyphWidget {
 	private _lineNumber: number;
-	private _domNode: HTMLDivElement;
 	private _editor: ICodeEditor;
+	private commentsDecorations: string[] = [];
+	private _commentsOptions: ModelDecorationOptions;
 
-	constructor(id: string, editor: ICodeEditor, lineNumber: number, disabled: boolean, onClick: () => void) {
-		this._id = id;
-		this._domNode = document.createElement('div');
-		this._domNode.className = disabled ? 'comment-hint commenting-disabled' : 'comment-hint';
-		this._domNode.addEventListener('click', onClick);
 
+	constructor(editor: ICodeEditor, lineNumber: number, commentsOptions: ModelDecorationOptions, onClick: () => void) {
+		this._commentsOptions = commentsOptions;
 		this._lineNumber = lineNumber;
-
 		this._editor = editor;
-		this._editor.addContentWidget(this);
-
+		this.update();
 	}
 
-	getDomNode(): HTMLElement {
-		return this._domNode;
-	}
+	update() {
+		let commentsDecorations = [{
+			range: {
+				startLineNumber: this._lineNumber, startColumn: 1,
+				endLineNumber: this._lineNumber, endColumn: 1
+			},
+			options: this._commentsOptions
+		}];
 
-	getId(): string {
-		return this._id;
+		this.commentsDecorations = this._editor.getModel().deltaDecorations(this.commentsDecorations, commentsDecorations);
 	}
 
 	setLineNumber(lineNumber: number): void {
 		this._lineNumber = lineNumber;
+		this.update();
 	}
 
 	getPosition(): IContentWidgetPosition {
@@ -46,4 +47,10 @@ export class CommentGlyphWidget implements IContentWidget {
 			preference: [ContentWidgetPositionPreference.EXACT]
 		};
 	}
+
+	dispose() {
+		if (this.commentsDecorations) {
+			this._editor.deltaDecorations(this.commentsDecorations, []);
+		}
+	}
 }
\ No newline at end of file

src/vs/workbench/parts/comments/electron-browser/commentThreadWidget.ts

@@ -34,6 +34,8 @@ import { Range, IRange } from 'vs/editor/common/core/range';
 import { IPosition } from 'vs/editor/common/core/position';
 import { IOpenerService } from 'vs/platform/opener/common/opener';
 import { MarkdownRenderer } from 'vs/editor/contrib/markdown/markdownRenderer';
+import { IMarginData } from 'vs/editor/browser/controller/mouseTarget';
+import { ModelDecorationOptions } from 'vs/editor/common/model/textModel';
 
 export const COMMENTEDITOR_DECORATION_KEY = 'commenteditordecoration';
 const EXPAND_ACTION_CLASS = 'expand-review-action octicon octicon-chevron-down';
@@ -287,11 +289,10 @@ export class ReviewZoneWidget extends ZoneWidget {
 		this._commentEditor.layout({ height: (this._commentEditor.hasWidgetFocus() ? 5 : 1) * 18, width: widthInPixel - 40 /* margin */ });
 	}
 
-	display(lineNumber: number) {
-		this._commentGlyph = new CommentGlyphWidget(`review_${lineNumber}`, this.editor, lineNumber, false, () => {
+	display(lineNumber: number, commentsOptions: ModelDecorationOptions) {
+		this._commentGlyph = new CommentGlyphWidget(this.editor, lineNumber, commentsOptions, () => {
 			this.toggleExpand();
 		});
-		this.editor.layoutContentWidget(this._commentGlyph);
 
 		this._localToDispose.push(this.editor.onMouseDown(e => this.onEditorMouseDown(e)));
 		this._localToDispose.push(this.editor.onMouseUp(e => this.onEditorMouseUp(e)));
@@ -300,7 +301,6 @@ export class ReviewZoneWidget extends ZoneWidget {
 			if (this.position) {
 				if (this.position.lineNumber !== this._commentGlyph.getPosition().position.lineNumber) {
 					this._commentGlyph.setLineNumber(this.position.lineNumber);
-					this.editor.layoutContentWidget(this._commentGlyph);
 				}
 			} else {
 				// Otherwise manually calculate position change :(
@@ -312,7 +312,6 @@ export class ReviewZoneWidget extends ZoneWidget {
 					}
 				}).reduce((prev, curr) => prev + curr, 0);
 				this._commentGlyph.setLineNumber(this._commentGlyph.getPosition().position.lineNumber + positionChange);
-				this.editor.layoutContentWidget(this._commentGlyph);
 			}
 		}));
 		var headHeight = Math.ceil(this.editor.getConfiguration().lineHeight * 1.2);
@@ -501,42 +500,58 @@ export class ReviewZoneWidget extends ZoneWidget {
 		}
 	}
 
-	private mouseDownInfo: { lineNumber: number, iconClicked: boolean };
+	private mouseDownInfo: { lineNumber: number };
 
 	private onEditorMouseDown(e: IEditorMouseEvent): void {
+		this.mouseDownInfo = null;
+
+		const range = e.target.range;
+
+		if (!range) {
+			return;
+		}
+
 		if (!e.event.leftButton) {
 			return;
 		}
 
-		let range = e.target.range;
-		if (!range) {
+		if (e.target.type !== MouseTargetType.GUTTER_LINE_DECORATIONS) {
 			return;
 		}
 
-		let iconClicked = false;
-		switch (e.target.type) {
-			case MouseTargetType.GUTTER_GLYPH_MARGIN:
-				iconClicked = true;
-				break;
-			default:
-				return;
+		const data = e.target.detail as IMarginData;
+		const gutterOffsetX = data.offsetX - data.glyphMarginWidth - data.lineNumbersWidth - data.glyphMarginLeft;
+
+		// don't collide with folding and git decorations
+		if (gutterOffsetX < 10 && gutterOffsetX > 19) {
+			return;
 		}
 
-		this.mouseDownInfo = { lineNumber: range.startLineNumber, iconClicked };
+		this.mouseDownInfo = { lineNumber: range.startLineNumber };
 	}
 
 	private onEditorMouseUp(e: IEditorMouseEvent): void {
 		if (!this.mouseDownInfo) {
 			return;
 		}
-		let lineNumber = this.mouseDownInfo.lineNumber;
-		let iconClicked = this.mouseDownInfo.iconClicked;
 
-		let range = e.target.range;
+		const { lineNumber } = this.mouseDownInfo;
+		this.mouseDownInfo = null;
+
+		const range = e.target.range;
+
 		if (!range || range.startLineNumber !== lineNumber) {
 			return;
 		}
 
+		if (e.target.type !== MouseTargetType.GUTTER_LINE_DECORATIONS) {
+			return;
+		}
+
+		if (!e.target.element) {
+			return;
+		}
+
 		if (this.position && this.position.lineNumber !== lineNumber) {
 			return;
 		}
@@ -545,17 +560,14 @@ export class ReviewZoneWidget extends ZoneWidget {
 			return;
 		}
 
-		if (iconClicked) {
-			if (e.target.type !== MouseTargetType.GUTTER_GLYPH_MARGIN) {
-				return;
+		if (e.target.element.className.indexOf('comment-thread') >= 0) {
+			if (this._isCollapsed) {
+				this.show({ lineNumber: lineNumber, column: 1 }, 2);
+			} else {
+				this.hide();
+				this._onDidClose.fire();
 			}
 		}
-
-		if (this._isCollapsed) {
-			this.show({ lineNumber: lineNumber, column: 1 }, 2);
-		} else {
-			this.hide();
-		}
 	}
 
 	private _applyTheme(theme: ITheme) {
@@ -623,7 +635,7 @@ export class ReviewZoneWidget extends ZoneWidget {
 		}
 
 		if (this._commentGlyph) {
-			this.editor.removeContentWidget(this._commentGlyph);
+			this._commentGlyph.dispose();
 			this._commentGlyph = null;
 		}
 

src/vs/workbench/parts/comments/electron-browser/commentsEditorContribution.ts

@@ -26,7 +26,6 @@ import { editorForeground, registerColor } from 'vs/platform/theme/common/colorR
 import { IThemeService, registerThemingParticipant } from 'vs/platform/theme/common/themeService';
 import { CommentThreadCollapsibleState } from 'vs/workbench/api/node/extHostTypes';
 import { ReviewModel } from 'vs/workbench/parts/comments/common/reviewModel';
-import { CommentGlyphWidget } from 'vs/workbench/parts/comments/electron-browser/commentGlyphWidget';
 import { ReviewZoneWidget, COMMENTEDITOR_DECORATION_KEY } from 'vs/workbench/parts/comments/electron-browser/commentThreadWidget';
 import { ICommentService } from 'vs/workbench/parts/comments/electron-browser/commentService';
 import { IModelService } from 'vs/editor/common/services/modelService';
@@ -74,22 +73,24 @@ class CommentingRangeDecorator {
 	}
 
 	private commentingOptions: ModelDecorationOptions;
-	private decorations: string[] = [];
+	public commentsOptions: ModelDecorationOptions;
+	private commentingRangeDecorations: string[] = [];
 	private disposables: IDisposable[] = [];
 
 	constructor(
 	) {
 		const options = { gutter: true, overview: false };
 		this.commentingOptions = CommentingRangeDecorator.createDecoration('comment-diff-added', overviewRulerCommentingRangeForeground, options);
+		this.commentsOptions = CommentingRangeDecorator.createDecoration('comment-thread', overviewRulerCommentingRangeForeground, options);
 	}
 
-	update(editor: ICodeEditor, ranges: Range[]) {
+	update(editor: ICodeEditor, commentsRanges: Range[], commentingRanges: Range[]) {
 		let model = editor.getModel();
 		if (!model) {
 			return;
 		}
 
-		let decorations = ranges.map((change) => {
+		let commentingRangesDecorations = commentingRanges.map((change) => {
 			const startLineNumber = change.startLineNumber;
 			const endLineNumber = change.endLineNumber;
 
@@ -102,12 +103,27 @@ class CommentingRangeDecorator {
 			};
 		});
 
-		this.decorations = model.deltaDecorations(this.decorations, decorations);
+		this.commentingRangeDecorations = model.deltaDecorations(this.commentingRangeDecorations, commentingRangesDecorations);
+	}
+
+
+	getActiveRanges(editor: ICodeEditor): Range[] {
+		let model = editor.getModel();
+		if (!model) {
+			return [];
+		}
+
+		let ranges = [];
+		for (let i = 0; i < this.commentingRangeDecorations.length; i++) {
+			ranges.push(model.getDecorationRange(this.commentingRangeDecorations[i]));
+		}
+
+		return ranges;
 	}
 
 	dispose(): void {
 		this.disposables = dispose(this.disposables);
-		this.decorations = [];
+		this.commentingRangeDecorations = [];
 	}
 }
 
@@ -120,10 +136,10 @@ export class ReviewController implements IEditorContribution {
 	private _reviewPanelVisible: IContextKey<boolean>;
 	private _commentInfos: modes.CommentInfo[];
 	private _reviewModel: ReviewModel;
-	private _newCommentGlyph: CommentGlyphWidget;
 	// private _hasSetComments: boolean;
 	private _commentingRangeDecorator: CommentingRangeDecorator;
 	private mouseDownInfo: { lineNumber: number } | null = null;
+	private _commentingRangeSpaceReserved = false;
 
 
 	constructor(
@@ -144,7 +160,6 @@ export class ReviewController implements IEditorContribution {
 		this._commentInfos = [];
 		this._commentWidgets = [];
 		this._newCommentWidget = null;
-		this._newCommentGlyph = null;
 		// this._hasSetComments = false;
 
 		this._reviewPanelVisible = ctxReviewPanelVisible.bindTo(contextKeyService);
@@ -164,7 +179,7 @@ export class ReviewController implements IEditorContribution {
 			this._commentInfos.forEach(info => {
 				info.threads.forEach(thread => {
 					let zoneWidget = new ReviewZoneWidget(this.instantiationService, this.modeService, this.modelService, this.themeService, this.commentService, this.openerService, this.editor, info.owner, thread, {});
-					zoneWidget.display(thread.range.startLineNumber);
+					zoneWidget.display(thread.range.startLineNumber, this._commentingRangeDecorator.commentsOptions);
 					this._commentWidgets.push(zoneWidget);
 				});
 			});
@@ -177,11 +192,6 @@ export class ReviewController implements IEditorContribution {
 				this._newCommentWidget = null;
 			}
 
-			if (this._newCommentGlyph) {
-				this.editor.removeContentWidget(this._newCommentGlyph);
-				this._newCommentGlyph = null;
-			}
-
 			this.getComments();
 		}));
 
@@ -294,11 +304,6 @@ export class ReviewController implements IEditorContribution {
 			this._newCommentWidget = null;
 		}
 
-		if (this._newCommentGlyph) {
-			this.editor.removeContentWidget(this._newCommentGlyph);
-			this._newCommentGlyph = null;
-		}
-
 		this._commentWidgets.forEach(zone => {
 			zone.dispose();
 		});
@@ -306,12 +311,7 @@ export class ReviewController implements IEditorContribution {
 
 		this.localToDispose.push(this.editor.onMouseDown(e => this.onEditorMouseDown(e)));
 		this.localToDispose.push(this.editor.onMouseUp(e => this.onEditorMouseUp(e)));
-		this.localToDispose.push(this.editor.onMouseLeave(() => this.onMouseLeave()));
 		this.localToDispose.push(this.editor.onDidChangeModelContent(() => {
-			if (this._newCommentGlyph) {
-				this.editor.removeContentWidget(this._newCommentGlyph);
-				this._newCommentGlyph = null;
-			}
 		}));
 		this.localToDispose.push(this.commentService.onDidUpdateCommentThreads(e => {
 			const editorURI = this.editor && this.editor.getModel() && this.editor.getModel().uri;
@@ -340,7 +340,7 @@ export class ReviewController implements IEditorContribution {
 			});
 			added.forEach(thread => {
 				let zoneWidget = new ReviewZoneWidget(this.instantiationService, this.modeService, this.modelService, this.themeService, this.commentService, this.openerService, this.editor, e.owner, thread, {});
-				zoneWidget.display(thread.range.startLineNumber);
+				zoneWidget.display(thread.range.startLineNumber, this._commentingRangeDecorator.commentsOptions);
 				this._commentWidgets.push(zoneWidget);
 				this._commentInfos.filter(info => info.owner === e.owner)[0].threads.push(thread);
 			});
@@ -371,41 +371,12 @@ export class ReviewController implements IEditorContribution {
 		}, {});
 
 		this._newCommentWidget.onDidClose(e => {
+			this._newCommentWidget.dispose();
 			this._newCommentWidget = null;
 		});
-		this._newCommentWidget.display(lineNumber);
+		this._newCommentWidget.display(lineNumber, this._commentingRangeDecorator.commentsOptions);
 	}
 
-	/* 	private onEditorMouseMove(e: IEditorMouseEvent): void {
-			if (!this._hasSetComments) {
-				return;
-			}
-
-			if (!this.editor.hasTextFocus()) {
-				return;
-			}
-
-			const hasCommentingRanges = this._commentInfos.length && this._commentInfos.some(info => !!info.commentingRanges.length);
-			if (hasCommentingRanges && e.target.position && e.target.position.lineNumber !== undefined) {
-				if (this._newCommentGlyph && e.target.element.className !== 'comment-hint') {
-					this.editor.removeContentWidget(this._newCommentGlyph);
-				}
-
-				const lineNumber = e.target.position.lineNumber;
-				if (!this.isExistingCommentThreadAtLine(lineNumber)) {
-					this._newCommentGlyph = this.isLineInCommentingRange(lineNumber)
-						? this._newCommentGlyph = new CommentGlyphWidget('comment-hint', this.editor, lineNumber, false, () => {
-							this.addComment(lineNumber);
-						})
-						: this._newCommentGlyph = new CommentGlyphWidget('comment-hint', this.editor, lineNumber, true, () => {
-							this.notificationService.warn('Commenting is not supported outside of diff hunk areas.');
-						});
-
-					this.editor.layoutContentWidget(this._newCommentGlyph);
-				}
-			}
-		} */
-
 	private onEditorMouseDown(e: IEditorMouseEvent): void {
 		this.mouseDownInfo = null;
 
@@ -426,8 +397,8 @@ export class ReviewController implements IEditorContribution {
 		const data = e.target.detail as IMarginData;
 		const gutterOffsetX = data.offsetX - data.glyphMarginWidth - data.lineNumbersWidth - data.glyphMarginLeft;
 
-		// TODO@joao TODO@alex TODO@martin this is such that we don't collide with folding
-		if (gutterOffsetX > 10) {
+		// don't collide with folding and git decorations
+		if (gutterOffsetX < 10 && gutterOffsetX > 19) {
 			return;
 		}
 
@@ -456,7 +427,7 @@ export class ReviewController implements IEditorContribution {
 			return;
 		}
 
-		if (e.target.element.className.indexOf('comment-dirty-diff-glyph') >= 0) {
+		if (e.target.element.className.indexOf('comment-diff-added') >= 0) {
 			const lineNumber = e.target.position.lineNumber;
 			if (!this.isExistingCommentThreadAtLine(lineNumber)) {
 				if (this.isLineInCommentingRange(lineNumber)) {
@@ -468,12 +439,6 @@ export class ReviewController implements IEditorContribution {
 		}
 	}
 
-	private onMouseLeave(): void {
-		if (this._newCommentGlyph) {
-			this.editor.removeContentWidget(this._newCommentGlyph);
-		}
-	}
-
 	private getNewCommentAction(line: number): { replyCommand: modes.Command, ownerId: number } {
 		for (let i = 0; i < this._commentInfos.length; i++) {
 			const commentInfo = this._commentInfos[i];
@@ -493,11 +458,10 @@ export class ReviewController implements IEditorContribution {
 	}
 
 	private isLineInCommentingRange(line: number): boolean {
-		return this._commentInfos.some(commentInfo => {
-			return commentInfo.commentingRanges.some(range =>
-				range.startLineNumber <= line && line <= range.endLineNumber
-			);
-		});
+		let ranges = this._commentingRangeDecorator.getActiveRanges(this.editor);
+		return ranges.some(range =>
+			range.startLineNumber <= line && line <= range.endLineNumber
+		);
 	}
 
 	private isExistingCommentThreadAtLine(line: number): boolean {
@@ -514,6 +478,29 @@ export class ReviewController implements IEditorContribution {
 
 	setComments(commentInfos: modes.CommentInfo[]): void {
 		this._commentInfos = commentInfos;
+		let lineDecorationsWidth: number = this.editor.getConfiguration().layoutInfo.decorationsWidth;
+
+		if (this._commentInfos.some(info => Boolean(info.commentingRanges && info.commentingRanges.length))) {
+			if (!this._commentingRangeSpaceReserved) {
+				this._commentingRangeSpaceReserved = true;
+				let extraEditorClassName = [];
+				if (this.editor.getRawConfiguration().extraEditorClassName) {
+					extraEditorClassName = this.editor.getRawConfiguration().extraEditorClassName.split(' ');
+				}
+
+				if (this.editor.getConfiguration().contribInfo.folding) {
+					lineDecorationsWidth -= 16;
+				}
+				lineDecorationsWidth += 9;
+				extraEditorClassName.push('inline-comment');
+				this.editor.updateOptions({
+					extraEditorClassName: extraEditorClassName.join(' '),
+					lineDecorationsWidth: lineDecorationsWidth
+				});
+				this.editor.layout();
+			}
+		}
+
 		// this._hasSetComments = true;
 
 		// create viewzones
@@ -524,17 +511,22 @@ export class ReviewController implements IEditorContribution {
 		this._commentInfos.forEach(info => {
 			info.threads.forEach(thread => {
 				let zoneWidget = new ReviewZoneWidget(this.instantiationService, this.modeService, this.modelService, this.themeService, this.commentService, this.openerService, this.editor, info.owner, thread, {});
-				zoneWidget.display(thread.range.startLineNumber);
+				zoneWidget.display(thread.range.startLineNumber, this._commentingRangeDecorator.commentsOptions);
 				this._commentWidgets.push(zoneWidget);
 			});
 		});
 
 		const commentingRanges = [];
-		this._commentInfos.forEach(info => commentingRanges.push(...info.commentingRanges));
-		this._commentingRangeDecorator.update(this.editor, commentingRanges);
+		const commentsRanges = [];
+		this._commentInfos.forEach(info => {
+			commentingRanges.push(...info.commentingRanges);
+			info.threads.forEach(thread => {
+				commentsRanges.push(thread.range);
+			});
+		});
+		this._commentingRangeDecorator.update(this.editor, commentsRanges, commentingRanges);
 	}
 
-
 	public closeWidget(): void {
 		this._reviewPanelVisible.reset();
 
@@ -661,6 +653,12 @@ registerThemingParticipant((theme, collector) => {
 			.monaco-editor .comment-diff-added:before {
 				background: ${commentingRangeForeground};
 			}
+			.monaco-editor .comment-thread {
+				border-left: 3px solid ${commentingRangeForeground};
+			}
+			.monaco-editor .comment-thread:before {
+				background: ${commentingRangeForeground};
+			}
 		`);
 	}
 });

src/vs/workbench/parts/comments/electron-browser/media/review.css

@@ -242,7 +242,7 @@
 }
 
 .monaco-editor .comment-dirty-diff-glyph {
-	margin-left: 1px;
+	margin-left: 14px;
 	cursor: pointer;
 }
 
@@ -255,11 +255,26 @@
 	transition: width 80ms linear, left 80ms linear;
 }
 
-.monaco-editor .margin-view-overlays>div:hover>.comment-dirty-diff-glyph:before {
+.monaco-editor .margin-view-overlays>div:hover>.comment-dirty-diff-glyph.comment-diff-added:before {
 	position: absolute;
 	content: '+';
 	height: 100%;
-	width: 8px;
-	left: -4px;
+	width: 9px;
+	left: -6px;
 	z-index: 10;
+	color: black;
+}
+
+.monaco-editor .comment-dirty-diff-glyph.comment-thread:before {
+	position: absolute;
+	content: '·';
+	height: 100%;
+	width: 9px;
+	left: -6px;
+	z-index: 20;
+	color: black;
+}
+
+.monaco-editor.inline-comment .margin-view-overlays .folding {
+	margin-left: 14px;
 }
\ No newline at end of file