Merge pull request #56110 from Microsoft/rebornix/comment-affordance

Move comment affordance into gutter

src/vs/editor/browser/widget/diffEditorWidget.ts

@@ -944,7 +944,7 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE
 		clonedOptions.folding = false;
 		clonedOptions.codeLens = false;
 		clonedOptions.fixedOverflowWidgets = true;
-		clonedOptions.lineDecorationsWidth = '2ch';
+		// clonedOptions.lineDecorationsWidth = '2ch';
 		if (!clonedOptions.minimap) {
 			clonedOptions.minimap = {};
 		}

src/vs/editor/browser/widget/media/diffEditor.css

@@ -41,6 +41,8 @@
 	opacity: 0.7;
 	background-repeat: no-repeat;
 	background-position: 50% 50%;
+	background-position: center;
+	background-size: 11px 11px;
 }
 .monaco-editor.hc-black .insert-sign,
 .monaco-diff-editor.hc-black .insert-sign,

src/vs/editor/contrib/folding/folding.ts

@@ -332,10 +332,13 @@ export class FoldingController implements IEditorContribution {
 		switch (e.target.type) {
 			case MouseTargetType.GUTTER_LINE_DECORATIONS:
 				const data = e.target.detail as IMarginData;
-				const gutterOffsetX = data.offsetX - data.glyphMarginWidth - data.lineNumbersWidth - data.glyphMarginLeft;
+				const offsetLeftInGutter = (e.target.element as HTMLElement).offsetLeft;
+				const gutterOffsetX = data.offsetX - offsetLeftInGutter;
+
+				// 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 dirty diff
-				if (gutterOffsetX <= 10) {
+				if (gutterOffsetX < 10) {
 					return;
 				}
 

src/vs/workbench/browser/parts/editor/textDiffEditor.ts

@@ -232,6 +232,7 @@ export class TextDiffEditor extends BaseTextEditor implements ITextDiffEditor {
 		const options: IDiffEditorOptions = super.getConfigurationOverrides();
 
 		options.readOnly = this.isReadOnly();
+		options.lineDecorationsWidth = '2ch';
 
 		return options;
 	}

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

@@ -25,7 +25,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti
 import { IModelService } from 'vs/editor/common/services/modelService';
 import { SimpleCommentEditor } from './simpleCommentEditor';
 import URI from 'vs/base/common/uri';
-import { transparent, editorForeground, textLinkActiveForeground, textLinkForeground, focusBorder, textBlockQuoteBackground, textBlockQuoteBorder, contrastBorder } from 'vs/platform/theme/common/colorRegistry';
+import { transparent, editorForeground, textLinkActiveForeground, textLinkForeground, focusBorder, textBlockQuoteBackground, textBlockQuoteBorder, contrastBorder, inputValidationErrorBorder, inputValidationErrorBackground } from 'vs/platform/theme/common/colorRegistry';
 import { IModeService } from 'vs/editor/common/services/modeService';
 import { IKeyboardEvent } from 'vs/base/browser/keyboardEvent';
 import { KeyCode } from 'vs/base/common/keyCodes';
@@ -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';
@@ -45,9 +47,11 @@ export class CommentNode {
 	private _body: HTMLElement;
 	private _md: HTMLElement;
 	private _clearTimeout: any;
+
 	public get domNode(): HTMLElement {
 		return this._domNode;
 	}
+
 	constructor(
 		public comment: modes.Comment,
 		private markdownRenderer: MarkdownRenderer,
@@ -115,6 +119,7 @@ export class ReviewZoneWidget extends ZoneWidget {
 	private _localToDispose: IDisposable[];
 	private _markdownRenderer: MarkdownRenderer;
 	private _styleElement: HTMLStyleElement;
+	private _error: HTMLElement;
 
 	public get owner(): number {
 		return this._owner;
@@ -284,14 +289,13 @@ export class ReviewZoneWidget extends ZoneWidget {
 	}
 
 	protected _doLayout(heightInPixel: number, widthInPixel: number): void {
-		this._commentEditor.layout({ height: (this._commentEditor.hasWidgetFocus() ? 5 : 1) * 18, width: widthInPixel - 40 /* margin */ });
+		this._commentEditor.layout({ height: (this._commentEditor.hasWidgetFocus() ? 5 : 1) * 18, width: widthInPixel - 42 /* 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 +304,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 +315,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);
@@ -366,6 +368,8 @@ export class ReviewZoneWidget extends ZoneWidget {
 			}
 		}));
 
+		this._error = $('.validation-error.hidden').appendTo(this._commentForm).getHTMLElement();
+
 		const formActions = $('.form-actions').appendTo(this._commentForm).getHTMLElement();
 
 		const button = new Button(formActions);
@@ -382,7 +386,32 @@ export class ReviewZoneWidget extends ZoneWidget {
 		}));
 
 		button.onDidClick(async () => {
+			this.createComment(lineNumber);
+		});
+
+		this._resizeObserver = new MutationObserver(this._refresh.bind(this));
+
+		this._resizeObserver.observe(this._bodyElement, {
+			attributes: true,
+			childList: true,
+			characterData: true,
+			subtree: true
+		});
+
+		if (this._commentThread.collapsibleState === modes.CommentThreadCollapsibleState.Expanded) {
+			this.show({ lineNumber: lineNumber, column: 1 }, 2);
+		}
+
+		// If there are no existing comments, place focus on the text area. This must be done after show, which also moves focus.
+		if (this._commentThread.reply && !this._commentThread.comments.length) {
+			this._commentEditor.focus();
+		}
+	}
+
+	private async createComment(lineNumber: number): Promise<void> {
+		try {
 			let newCommentThread;
+
 			if (this._commentThread.threadId) {
 				// reply
 				newCommentThread = await this.commentService.replyToCommentThread(
@@ -400,36 +429,28 @@ export class ReviewZoneWidget extends ZoneWidget {
 					this._commentEditor.getValue()
 				);
 
-				this.createReplyButton();
-				this.createParticipantsLabel();
-			}
-
-			this._commentEditor.setValue('');
-			if (dom.hasClass(this._commentForm, 'expand')) {
-				dom.removeClass(this._commentForm, 'expand');
+				if (newCommentThread) {
+					this.createReplyButton();
+					this.createParticipantsLabel();
+				}
 			}
 
 			if (newCommentThread) {
+				this._commentEditor.setValue('');
+				if (dom.hasClass(this._commentForm, 'expand')) {
+					dom.removeClass(this._commentForm, 'expand');
+				}
+				this._commentEditor.getDomNode().style.outline = '';
+				this._error.textContent = '';
+				dom.addClass(this._error, 'hidden');
 				this.update(newCommentThread);
 			}
-		});
-
-		this._resizeObserver = new MutationObserver(this._refresh.bind(this));
-
-		this._resizeObserver.observe(this._bodyElement, {
-			attributes: true,
-			childList: true,
-			characterData: true,
-			subtree: true
-		});
-
-		if (this._commentThread.collapsibleState === modes.CommentThreadCollapsibleState.Expanded) {
-			this.show({ lineNumber: lineNumber, column: 1 }, 2);
-		}
-
-		// If there are no existing comments, place focus on the text area. This must be done after show, which also moves focus.
-		if (this._commentThread.reply && !this._commentThread.comments.length) {
-			this._commentEditor.focus();
+		} catch (e) {
+			this._error.textContent = e.message
+				? nls.localize('commentCreationError', "Adding a comment failed: {0}.", e.message)
+				: nls.localize('commentCreationDefaultError', "Adding a comment failed. Please try again or report an issue with the extension if the problem persists.");
+			this._commentEditor.getDomNode().style.outline = `1px solid ${this.themeService.getTheme().getColor(inputValidationErrorBorder)}`;
+			dom.removeClass(this._error, 'hidden');
 		}
 	}
 
@@ -500,60 +521,71 @@ 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 > 14) {
+			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 (this.position && this.position.lineNumber !== lineNumber) {
+		if (e.target.type !== MouseTargetType.GUTTER_LINE_DECORATIONS) {
 			return;
 		}
 
-		if (!this.position && lineNumber !== this._commentThread.range.startLineNumber) {
+		if (!e.target.element) {
 			return;
 		}
 
-		if (iconClicked) {
-			if (e.target.type !== MouseTargetType.GUTTER_GLYPH_MARGIN) {
-				return;
-			}
+		if (this._commentGlyph && this._commentGlyph.getPosition().position.lineNumber !== lineNumber) {
+			return;
 		}
 
-		if (this._isCollapsed) {
-			this.show({ lineNumber: lineNumber, column: 1 }, 2);
-		} else {
-			this.hide();
+		if (e.target.element.className.indexOf('comment-thread') >= 0) {
+			if (this._isCollapsed) {
+				this.show({ lineNumber: lineNumber, column: 1 }, 2);
+			} else {
+				this.hide();
+				if (this._commentThread === null) {
+					this.dispose();
+				}
+			}
 		}
 	}
 
@@ -597,6 +629,16 @@ export class ReviewZoneWidget extends ZoneWidget {
 			content.push(`.monaco-editor .review-widget .body .comment-form .monaco-editor { outline: 1px solid ${hcBorder}; }`);
 		}
 
+		const errorBorder = theme.getColor(inputValidationErrorBorder);
+		if (errorBorder) {
+			content.push(`.monaco-editor .review-widget .body .comment-form .validation-error { border: 1px solid ${errorBorder}; }`);
+		}
+
+		const errorBackground = theme.getColor(inputValidationErrorBackground);
+		if (errorBackground) {
+			content.push(`.monaco-editor .review-widget .body .comment-form .validation-error { background: ${errorBackground}; }`);
+		}
+
 		this._styleElement.innerHTML = content.join('\n');
 
 		// Editor decorations should also be responsive to theme changes
@@ -622,7 +664,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

@@ -10,27 +10,30 @@ import { $ } from 'vs/base/browser/builder';
 import { findFirstInSorted } from 'vs/base/common/arrays';
 import { KeyCode, KeyMod } from 'vs/base/common/keyCodes';
 import { IDisposable, dispose } from 'vs/base/common/lifecycle';
-import { ICodeEditor, IEditorMouseEvent, IViewZone } from 'vs/editor/browser/editorBrowser';
+import { ICodeEditor, IEditorMouseEvent, IViewZone, MouseTargetType } from 'vs/editor/browser/editorBrowser';
 import { registerEditorContribution, EditorAction, registerEditorAction } from 'vs/editor/browser/editorExtensions';
 import { ICodeEditorService } from 'vs/editor/browser/services/codeEditorService';
 import { EmbeddedCodeEditorWidget } from 'vs/editor/browser/widget/embeddedCodeEditorWidget';
 import { IEditorContribution } from 'vs/editor/common/editorCommon';
+import { IRange } from 'vs/editor/common/core/range';
 import * as modes from 'vs/editor/common/modes';
 import { peekViewEditorBackground, peekViewResultsBackground, peekViewResultsSelectionBackground } from 'vs/editor/contrib/referenceSearch/referencesWidget';
 import { IContextKey, IContextKeyService, RawContextKey } from 'vs/platform/contextkey/common/contextkey';
 import { ServicesAccessor, IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { KeybindingsRegistry, KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry';
-import { INotificationService } from 'vs/platform/notification/common/notification';
-import { editorForeground } from 'vs/platform/theme/common/colorRegistry';
+import { editorForeground, registerColor } from 'vs/platform/theme/common/colorRegistry';
 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';
 import { IModeService } from 'vs/editor/common/services/modeService';
 import { IOpenerService } from 'vs/platform/opener/common/opener';
+import { ModelDecorationOptions } from 'vs/editor/common/model/textModel';
+import { IModelDecorationOptions } from 'vs/editor/common/model';
+import { Color, RGBA } from 'vs/base/common/color';
+import { IMarginData } from 'vs/editor/browser/controller/mouseTarget';
 
 export const ctxReviewPanelVisible = new RawContextKey<boolean>('reviewPanelVisible', false);
 
@@ -53,6 +56,103 @@ export class ReviewViewZone implements IViewZone {
 	}
 }
 
+const overviewRulerDefault = new Color(new RGBA(197, 197, 197, 1));
+
+export const overviewRulerCommentingRangeForeground = registerColor('editorGutter.commentRangeForeground', { dark: overviewRulerDefault, light: overviewRulerDefault, hc: overviewRulerDefault }, nls.localize('editorGutterCommentRangeForeground', 'Editor gutter decoration color for commenting ranges.'));
+
+class CommentingRangeDecoration {
+	private _decorationId: string;
+
+	constructor(private _editor: ICodeEditor, private _ownerId: number, private _range: IRange, private _reply: modes.Command, commentingOptions: ModelDecorationOptions) {
+		const startLineNumber = _range.startLineNumber;
+		const endLineNumber = _range.endLineNumber;
+		let commentingRangeDecorations = [{
+			range: {
+				startLineNumber: startLineNumber, startColumn: 1,
+				endLineNumber: endLineNumber, endColumn: 1
+			},
+			options: commentingOptions
+		}];
+
+		let model = this._editor.getModel();
+		if (model) {
+			this._decorationId = model.deltaDecorations([this._decorationId], commentingRangeDecorations)[0];
+		}
+	}
+
+	getCommentAction(): { replyCommand: modes.Command, ownerId: number } {
+		return {
+			replyCommand: this._reply,
+			ownerId: this._ownerId
+		};
+	}
+
+	getOriginalRange() {
+		return this._range;
+	}
+
+	getActiveRange() {
+		return this._editor.getModel().getDecorationRange(this._decorationId);
+	}
+}
+class CommentingRangeDecorator {
+
+	static createDecoration(className: string, foregroundColor: string, options: { gutter: boolean, overview: boolean }): ModelDecorationOptions {
+		const decorationOptions: IModelDecorationOptions = {
+			isWholeLine: true,
+		};
+
+		decorationOptions.linesDecorationsClassName = `comment-range-glyph ${className}`;
+		return ModelDecorationOptions.createDynamic(decorationOptions);
+	}
+
+	private commentingOptions: ModelDecorationOptions;
+	public commentsOptions: ModelDecorationOptions;
+	private commentingRangeDecorations: CommentingRangeDecoration[] = [];
+	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, commentInfos: modes.CommentInfo[]) {
+		let model = editor.getModel();
+		if (!model) {
+			return;
+		}
+
+		let commentingRangeDecorations = [];
+		for (let i = 0; i < commentInfos.length; i++) {
+			let info = commentInfos[i];
+			info.commentingRanges.forEach(range => {
+				commentingRangeDecorations.push(new CommentingRangeDecoration(editor, info.owner, range, info.reply, this.commentingOptions));
+			});
+		}
+
+		this.commentingRangeDecorations = commentingRangeDecorations;
+	}
+
+	getMatchedCommentAction(line: number) {
+		for (let i = 0; i < this.commentingRangeDecorations.length; i++) {
+			let range = this.commentingRangeDecorations[i].getActiveRange();
+
+			if (range.startLineNumber <= line && line <= range.endLineNumber) {
+				return this.commentingRangeDecorations[i].getCommentAction();
+			}
+		}
+
+		return null;
+	}
+
+	dispose(): void {
+		this.disposables = dispose(this.disposables);
+		this.commentingRangeDecorations = [];
+	}
+}
+
 export class ReviewController implements IEditorContribution {
 	private globalToDispose: IDisposable[];
 	private localToDispose: IDisposable[];
@@ -62,15 +162,17 @@ export class ReviewController implements IEditorContribution {
 	private _reviewPanelVisible: IContextKey<boolean>;
 	private _commentInfos: modes.CommentInfo[];
 	private _reviewModel: ReviewModel;
-	private _newCommentGlyph: CommentGlyphWidget;
-	private _hasSetComments: boolean;
+	// private _hasSetComments: boolean;
+	private _commentingRangeDecorator: CommentingRangeDecorator;
+	private mouseDownInfo: { lineNumber: number } | null = null;
+	private _commentingRangeSpaceReserved = false;
+
 
 	constructor(
 		editor: ICodeEditor,
 		@IContextKeyService contextKeyService: IContextKeyService,
 		@IThemeService private themeService: IThemeService,
 		@ICommentService private commentService: ICommentService,
-		@INotificationService private notificationService: INotificationService,
 		@IInstantiationService private instantiationService: IInstantiationService,
 		@IModeService private modeService: IModeService,
 		@IModelService private modelService: IModelService,
@@ -83,11 +185,11 @@ export class ReviewController implements IEditorContribution {
 		this._commentInfos = [];
 		this._commentWidgets = [];
 		this._newCommentWidget = null;
-		this._newCommentGlyph = null;
-		this._hasSetComments = false;
+		// this._hasSetComments = false;
 
 		this._reviewPanelVisible = ctxReviewPanelVisible.bindTo(contextKeyService);
 		this._reviewModel = new ReviewModel();
+		this._commentingRangeDecorator = new CommentingRangeDecorator();
 
 		this._reviewModel.onDidChangeStyle(style => {
 			if (this._newCommentWidget) {
@@ -102,7 +204,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);
 				});
 			});
@@ -115,11 +217,6 @@ export class ReviewController implements IEditorContribution {
 				this._newCommentWidget = null;
 			}
 
-			if (this._newCommentGlyph) {
-				this.editor.removeContentWidget(this._newCommentGlyph);
-				this._newCommentGlyph = null;
-			}
-
 			this.getComments();
 		}));
 
@@ -232,23 +329,14 @@ 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();
 		});
 		this._commentWidgets = [];
 
-		this.localToDispose.push(this.editor.onMouseMove(e => this.onEditorMouseMove(e)));
-		this.localToDispose.push(this.editor.onMouseLeave(() => this.onMouseLeave()));
+		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.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;
@@ -277,7 +365,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);
 			});
@@ -285,7 +373,7 @@ export class ReviewController implements IEditorContribution {
 	}
 
 	private addComment(lineNumber: number) {
-		let newCommentInfo = this.getNewCommentAction(lineNumber);
+		let newCommentInfo = this._commentingRangeDecorator.getMatchedCommentAction(lineNumber);
 		if (!newCommentInfo) {
 			return;
 		}
@@ -310,82 +398,91 @@ export class ReviewController implements IEditorContribution {
 		this._newCommentWidget.onDidClose(e => {
 			this._newCommentWidget = null;
 		});
-		this._newCommentWidget.display(lineNumber);
+		this._newCommentWidget.display(lineNumber, this._commentingRangeDecorator.commentsOptions);
 	}
 
-	private onEditorMouseMove(e: IEditorMouseEvent): void {
-		if (!this._hasSetComments) {
+	private onEditorMouseDown(e: IEditorMouseEvent): void {
+		this.mouseDownInfo = null;
+
+		const range = e.target.range;
+
+		if (!range) {
 			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);
-			}
+		if (!e.event.leftButton) {
+			return;
 		}
-	}
 
-	private onMouseLeave(): void {
-		if (this._newCommentGlyph) {
-			this.editor.removeContentWidget(this._newCommentGlyph);
+		if (e.target.type !== MouseTargetType.GUTTER_LINE_DECORATIONS) {
+			return;
 		}
-	}
 
-	private getNewCommentAction(line: number): { replyCommand: modes.Command, ownerId: number } {
-		for (let i = 0; i < this._commentInfos.length; i++) {
-			const commentInfo = this._commentInfos[i];
-			const lineWithinRange = commentInfo.commentingRanges.some(range =>
-				range.startLineNumber <= line && line <= range.endLineNumber
-			);
+		const data = e.target.detail as IMarginData;
+		const gutterOffsetX = data.offsetX - data.glyphMarginWidth - data.lineNumbersWidth - data.glyphMarginLeft;
 
-			if (lineWithinRange) {
-				return {
-					replyCommand: commentInfo.reply,
-					ownerId: commentInfo.owner
-				};
-			}
+		// don't collide with folding and git decorations
+		if (gutterOffsetX > 14) {
+			return;
 		}
 
-		return null;
+		this.mouseDownInfo = { lineNumber: range.startLineNumber };
 	}
 
-	private isLineInCommentingRange(line: number): boolean {
-		return this._commentInfos.some(commentInfo => {
-			return commentInfo.commentingRanges.some(range =>
-				range.startLineNumber <= line && line <= range.endLineNumber
-			);
-		});
-	}
+	private onEditorMouseUp(e: IEditorMouseEvent): void {
+		if (!this.mouseDownInfo) {
+			return;
+		}
 
-	private isExistingCommentThreadAtLine(line: number): boolean {
-		const existingThread = this._commentInfos.some(commentInfo => {
-			return commentInfo.threads.some(thread =>
-				thread.range.startLineNumber === line
-			);
-		});
+		const { lineNumber } = this.mouseDownInfo;
+		this.mouseDownInfo = null;
+
+		const range = e.target.range;
 
-		const existingNewComment = this._newCommentWidget && this._newCommentWidget.position && this._newCommentWidget.position.lineNumber === line;
+		if (!range || range.startLineNumber !== lineNumber) {
+			return;
+		}
 
-		return existingThread || existingNewComment;
+		if (e.target.type !== MouseTargetType.GUTTER_LINE_DECORATIONS) {
+			return;
+		}
+
+		if (!e.target.element) {
+			return;
+		}
+
+		if (e.target.element.className.indexOf('comment-diff-added') >= 0) {
+			const lineNumber = e.target.position.lineNumber;
+			this.addComment(lineNumber);
+		}
 	}
 
 	setComments(commentInfos: modes.CommentInfo[]): void {
 		this._commentInfos = commentInfos;
-		this._hasSetComments = true;
+		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
 		this._commentWidgets.forEach(zone => {
@@ -395,12 +492,17 @@ 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, this._commentInfos);
+	}
 
 	public closeWidget(): void {
 		this._reviewPanelVisible.reset();
@@ -518,4 +620,22 @@ registerThemingParticipant((theme, collector) => {
 			`}`
 		);
 	}
+
+	const commentingRangeForeground = theme.getColor(overviewRulerCommentingRangeForeground);
+	if (commentingRangeForeground) {
+		collector.addRule(`
+			.monaco-editor .comment-diff-added {
+				border-left: 3px solid ${commentingRangeForeground};
+			}
+			.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

@@ -10,7 +10,7 @@
 	background-position: center center;
 }
 
-.monaco-editor .comment-hint{
+.monaco-editor .comment-hint {
 	height: 20px;
 	width: 20px;
 	padding-left: 2px;
@@ -126,6 +126,29 @@
 	padding: 8px 0;
 }
 
+.monaco-editor .review-widget .body .comment-form .validation-error {
+	display: inline-block;
+	overflow: hidden;
+	text-align: left;
+	width: 100%;
+	box-sizing:	border-box;
+	-webkit-box-sizing:	border-box;
+	-o-box-sizing:		border-box;
+	-moz-box-sizing:	border-box;
+	-ms-box-sizing:		border-box;
+	padding: 0.4em;
+	font-size: 12px;
+	line-height: 17px;
+	min-height: 34px;
+	margin-top: -1px;
+	margin-left: -1px;
+	word-wrap: break-word;
+}
+
+.monaco-editor .review-widget .body .comment-form .validation-error.hidden {
+	display: none;
+}
+
 .monaco-editor .review-widget .body .comment-form.expand .review-thread-reply-button {
 	display: none;
 }
@@ -239,4 +262,52 @@
 .monaco-editor .review-widget>.body {
 	border-top: 1px solid;
 	position: relative;
-}
\ No newline at end of file
+}
+
+.monaco-editor .comment-range-glyph {
+	margin-left: 5px;
+	width: 4px !important;
+	cursor: pointer;
+	z-index: 10;
+}
+
+.monaco-editor .comment-range-glyph:before {
+	position: absolute;
+	content: '';
+	height: 100%;
+	width: 0;
+	left: -2px;
+	transition: width 80ms linear, left 80ms linear;
+}
+
+.monaco-editor .margin-view-overlays>div:hover>.comment-range-glyph.comment-diff-added:before {
+	position: absolute;
+	content: '+';
+	height: 100%;
+	width: 9px;
+	left: -6px;
+	z-index: 10;
+	color: black;
+}
+
+.monaco-editor .comment-range-glyph.comment-thread {
+	z-index: 20;
+}
+
+.monaco-editor .comment-range-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;
+}
+
+.monaco-editor.inline-comment .margin-view-overlays .dirty-diff-glyph {
+	margin-left: 14px;
+}

src/vs/workbench/parts/scm/electron-browser/dirtydiffDecorator.ts

@@ -703,11 +703,16 @@ export class DirtyDiffController implements IEditorContribution {
 			return;
 		}
 
+		if (e.target.element.className.indexOf('dirty-diff-glyph') < 0) {
+			return;
+		}
+
 		const data = e.target.detail as IMarginData;
-		const gutterOffsetX = data.offsetX - data.glyphMarginWidth - data.lineNumbersWidth - data.glyphMarginLeft;
+		const offsetLeftInGutter = (e.target.element as HTMLElement).offsetLeft;
+		const gutterOffsetX = data.offsetX - offsetLeftInGutter;
 
 		// TODO@joao TODO@alex TODO@martin this is such that we don't collide with folding
-		if (gutterOffsetX > 10) {
+		if (gutterOffsetX < 0 || gutterOffsetX > 10) {
 			return;
 		}
 

src/vs/workbench/parts/scm/electron-browser/media/dirtydiffDecorator.css

@@ -6,6 +6,7 @@
 .monaco-editor .dirty-diff-glyph {
 	margin-left: 5px;
 	cursor: pointer;
+	z-index: 5;
 }
 
 .monaco-editor .dirty-diff-deleted:after {