DevTools: Changes View

third_party/WebKit/Source/devtools/front_end/changes/ChangesView.js

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+Changes.ChangesView = class extends UI.VBox {
+  constructor() {
+    super(true);
+    this.registerRequiredCSS('changes/changesView.css');
+    var splitWidget = new UI.SplitWidget(true, false);

[lushnikov, 2017/03/24 04:02:34]

nit: lets' annotate booleans: true /* something */

[einbinder, 2017/03/28 00:07:24]

Done.

+    var mainWidget = new UI.Widget();
+    splitWidget.setMainWidget(mainWidget);
+    splitWidget.show(this.contentElement);
+    var diffArea = mainWidget.element.createChild('div', 'diff-area');

[lushnikov, 2017/03/24 04:02:33]

you don't need diffArea

[einbinder, 2017/03/28 00:07:13]

Done.

+
+    this._emptyWidget = new UI.EmptyWidget(Common.UIString('No changes'));
+    this._emptyWidget.show(diffArea);
+
+    this._workspaceDiff = WorkspaceDiff.workspaceDiff();
+    var uiSourceCodeList = new Changes.UISourceCodeList(this._workspaceDiff);
+    uiSourceCodeList.on(Changes.UISourceCodeList.SelectedUISourceCodeChanged, this._selectedUISourceCodeChanged, this);
+    splitWidget.setSidebarWidget(uiSourceCodeList);
+
+    /** @type {?Workspace.UISourceCode} */
+    this._uiSourceCode = null;

[lushnikov, 2017/03/24 04:02:34]

this._selectedUISourceCode = null;

[einbinder, 2017/03/28 00:07:24]

Done.

+
+    /** @type {!Array<!Changes.ChangesView.Row>} */
+    this._rows = [];
+
+    this._maxLineDigits = 1;
+
+    this._editor =
+        new TextEditor.CodeMirrorTextEditor({lineNumbers: true, lineWrapping: false, maxHighlightLength: Infinity});

[lushnikov, 2017/03/24 04:02:33]

let's put a comment why we're killing maxHighlightLength here

[einbinder, 2017/03/28 00:07:24]

Done.

+    this._editor.setReadOnly(true);
+    this._editor.show(diffArea);
+    this._editor.hideWidget();
+
+    this._editor.element.addEventListener('click', this._click.bind(this), false);
+
+    this._toolbar = new UI.Toolbar('changes-toolbar', mainWidget.element);
+    var revertButton = new UI.ToolbarButton(Common.UIString('Revert all changes'), 'largeicon-undo');
+    revertButton.addEventListener(UI.ToolbarButton.Events.Click, this._revert.bind(this));
+    this._toolbar.appendToolbarItem(revertButton);
+    this._diffStats = new UI.ToolbarText('');
+    this._toolbar.appendToolbarItem(this._diffStats);
+    this._toolbar.setEnabled(false);
+  }
+
+  /**
+   * @param {!Changes.UISourceCodeList.SelectedUISourceCodeChanged} event
+   */
+  _selectedUISourceCodeChanged(event) {
+    this._revealUISourceCode(event.uiSourceCode);
+  }
+
+  _revert() {
+    var uiSourceCode = this._uiSourceCode;
+    if (!uiSourceCode)
+      return;
+    uiSourceCode.requestOriginalContent().then(original => uiSourceCode.addRevision(original || ''));
+  }
+
+  _click(event) {

[lushnikov, 2017/03/24 04:02:34]

jsdoc

[einbinder, 2017/03/28 00:07:17]

Done.

+    var selection = this._editor.selection();
+    if (!selection.isEmpty())
+      return;
+    var row = this._rows[selection.startLine];
+    if (!row)

[lushnikov, 2017/03/24 04:02:33]

let's typecast rather then fast-return

[einbinder, 2017/03/28 00:07:24]

Removed.

+      return;
+    Common.Revealer.reveal(this._uiSourceCode.uiLocation(row.current - 1, selection.startColumn), false);

[lushnikov, 2017/03/24 04:02:33]

how come you need to "-1"?

[einbinder, 2017/03/28 00:07:14]

row.current is the displayed line number, 1-indexed.

+    event.consume(true);
+  }
+
+  /**
+   * @param {?Workspace.UISourceCode} uiSourceCode
+   */
+  _revealUISourceCode(uiSourceCode) {
+    if (this._uiSourceCode === uiSourceCode)
+      return;
+
+    if (this._uiSourceCode)
+      this._workspaceDiff.unsubscribeFromDiffChange(this._uiSourceCode, this._refreshDiff, this);
+    if (uiSourceCode && this.isShowing())
+      this._workspaceDiff.subscribeToDiffChange(uiSourceCode, this._refreshDiff, this);
+
+    this._uiSourceCode = uiSourceCode;
+    if (!uiSourceCode || this.isShowing())

[lushnikov, 2017/03/24 04:02:33]

let's kill this: !uiSourceCode ||

[einbinder, 2017/03/28 00:07:24]

Done.

+      this._refreshDiff();
+  }
+
+  /**
+   * @override
+   */
+  wasShown() {
+    if (!this._uiSourceCode)

[lushnikov, 2017/03/24 04:02:33]

consider using ThrottledWidget

[einbinder, 2017/03/28 00:07:23]

Not needed.

+      return;
+    this._workspaceDiff.subscribeToDiffChange(this._uiSourceCode, this._refreshDiff, this);

[lushnikov, 2017/03/24 04:02:34]

let's stay subscribed

[einbinder, 2017/03/28 00:07:24]

Done.

+    this._refreshDiff();
+  }
+
+  /**
+   * @override
+   */
+  willHide() {
+    if (this._uiSourceCode)
+      this._workspaceDiff.unsubscribeFromDiffChange(this._uiSourceCode, this._refreshDiff, this);
+  }
+
+  _refreshDiff() {
+    if (!this._uiSourceCode) {

[lushnikov, 2017/03/24 04:02:34]

you want to fast-return here if widget is not showin

[einbinder, 2017/03/28 00:07:24]

Yep

+      this._renderRows(null);
+      return;
+    }
+    var uiSourceCode = this._uiSourceCode;
+    this._workspaceDiff.requestDiff(uiSourceCode).then(diff => {
+      if (this._uiSourceCode !== uiSourceCode)
+        return;
+      this._renderRows(diff);
+    });
+  }
+
+  /**
+   * @param {?Diff.Diff.DiffArray} diff
+   */
+  _renderRows(diff) {
+    this._rows = [];
+
+    if (!diff || (diff.length === 1 && diff[0][0] === Diff.Diff.Operation.Equal)) {
+      this._diffStats.setText('');
+      this._toolbar.setEnabled(false);
+      this._editor.hideWidget();
+      this._emptyWidget.showWidget();
+      return;
+    }
+
+    var insertions = 0;
+    var deletions = 0;
+    var currentLineNumber = 0;
+    var baselineLineNumber = 0;
+    var paddingLines = 3;
+    var originalLines = [];
+    var currentLines = [];
+
+    for (var i = 0; i < diff.length; ++i) {
+      var token = diff[i];
+      switch (token[0]) {
+        case Diff.Diff.Operation.Equal:
+          this._rows.pushAll(createEqualRows(token[1], i === 0, i === diff.length - 1));
+          originalLines.pushAll(token[1]);
+          currentLines.pushAll(token[1]);

[lushnikov, 2017/03/24 04:02:33]

let's manage baselineLineNumber/currentLineNubmer/insertions/deletions right
here in the switch. It will make code easier to follow

[einbinder, 2017/03/28 00:07:23]

Done.

+          break;
+        case Diff.Diff.Operation.Insert:
+          for (var line of token[1])
+            this._rows.push(createRow(line, 'addition'));
+          currentLines.pushAll(token[1]);
+          break;
+        case Diff.Diff.Operation.Delete:
+          originalLines.pushAll(token[1]);
+          if (diff[i + 1] && diff[i + 1][0] === Diff.Diff.Operation.Insert) {
+            i++;
+            this._rows.pushAll(createModifyRows(token[1].join('\n'), diff[i][1].join('\n')));
+            currentLines.pushAll(diff[i][1]);
+          } else {
+            for (var line of token[1])
+              this._rows.push(createRow(line, 'deletion'));
+          }
+          break;
+      }
+    }
+
+    this._maxLineDigits = Math.max(Math.ceil(Math.log10(Math.max(currentLineNumber, baselineLineNumber))), 1);

[lushnikov, 2017/03/24 04:02:33]

let's drop the "1" in the end

[einbinder, 2017/03/28 00:07:12]

Done.

+
+    this._editor.operation(() => {
+      this._editor.setHighlightMode({
+        name: 'devtools-diff',
+        rows: this._rows,
+        mimeType:
+            Bindings.NetworkProject.uiSourceCodeMimeType(/** @type {!Workspace.UISourceCode} */ (this._uiSourceCode)),
+        baselineLines: originalLines,
+        currentLines: currentLines
+      });
+      this._editor.setText(this._rows.map(row => row.content.map(t => t.text).join('')).join('\n'));

[lushnikov, 2017/03/24 04:02:34]

consider using [].reduce?

[einbinder, 2017/03/28 00:07:15]

Still looks ugly.

+      this._editor.setLineNumberFormatter(this._lineFormatter.bind(this));
+    });
+
+    this._diffStats.setText(Common.UIString(
+        '%d insertion%s (+), %d deletion%s (-)', insertions, insertions > 1 ? 's' : '', deletions,

[lushnikov, 2017/03/24 04:02:34]

plurals!

[einbinder, 2017/03/28 00:07:12]

Done.

+        deletions > 1 ? 's' : ''));
+    this._toolbar.setEnabled(true);
+    this._emptyWidget.hideWidget();
+    this._editor.showWidget();
+
+    /**
+     * @param {!Array<string>} lines
+     * @param {boolean} atStart
+     * @param {boolean} atEnd
+     * @return {!Array<!Changes.ChangesView.Row>}}
+     */
+    function createEqualRows(lines, atStart, atEnd) {
+      var equalRows = [];
+      if (!atStart) {
+        for (var i = 0; i < paddingLines && i < lines.length; i++)
+          equalRows.push(createRow(lines[i], 'equal'));
+        if (lines.length > paddingLines * 2 + 1 && !atEnd) {
+          equalRows.push(createRow(
+              Common.UIString('( \u2026 Skipping ') + (lines.length - paddingLines * 2) +
+                  Common.UIString(' matching lines \u2026 )'),
+              'spacer'));
+        }
+      }
+      if (!atEnd) {
+        var start = Math.max(lines.length - paddingLines - 1, atStart ? 0 : paddingLines);

[lushnikov, 2017/03/24 04:02:33]

let's just increment |index| as you push rows in the beginning

+        var skip = lines.length - paddingLines - 1;
+        if (!atStart)
+          skip -= paddingLines;
+        if (skip > 0) {
+          baselineLineNumber += skip;
+          currentLineNumber += skip;
+        }
+
+        for (var i = start; i < lines.length; i++)

[lushnikov, 2017/03/24 04:02:33]

let's make this function to populate two different arrays and concat them in the
end.

+          equalRows.push(createRow(lines[i], 'equal'));
+      }
+      return equalRows;
+    }
+
+    /**
+     * @param {string} before
+     * @param {string} after
+     * @return {!Array<!Changes.ChangesView.Row>}}
+     */
+    function createModifyRows(before, after) {
+      var internalDiff = Diff.Diff.charDiff(before, after, true);

[lushnikov, 2017/03/24 04:02:33]

nit: let's annotate "true"

it feels scary to do diff without linebreaks!

[einbinder, 2017/03/28 00:07:12]

Done.

+      var deletionRows = [createRow('', 'deletion')];
+      var insertionRows = [createRow('', 'addition')];
+
+      for (var token of internalDiff) {
+        var text = token[1];
+        var type = token[0];
+        var className = type === Diff.Diff.Operation.Equal ? '' : 'double';

[lushnikov, 2017/03/24 04:02:34]

s/double/inner-diff/

+        var lines = text.split('\n');
+        for (var i = 0; i < lines.length; i++) {
+          if (i > 0 && type !== Diff.Diff.Operation.Insert)
+            deletionRows.push(createRow('', 'deletion'));
+          if (i > 0 && type !== Diff.Diff.Operation.Delete)
+            insertionRows.push(createRow('', 'addition'));
+          if (!lines[i])
+            continue;
+          if (type !== Diff.Diff.Operation.Insert)
+            deletionRows[deletionRows.length - 1].content.push({text: lines[i], className: className});
+          if (type !== Diff.Diff.Operation.Delete)
+            insertionRows[insertionRows.length - 1].content.push({text: lines[i], className: className});
+        }
+      }
+      return deletionRows.concat(insertionRows);
+    }
+
+    /**
+     * @param {string} text
+     * @param {string} className
+     * @return {!Changes.ChangesView.Row}
+     */
+    function createRow(text, className) {
+      if (className === 'addition') {
+        currentLineNumber++;
+        insertions++;
+      }
+      if (className === 'deletion') {
+        baselineLineNumber++;
+        deletions++;
+      }
+      if (className === 'equal') {
+        baselineLineNumber++;
+        currentLineNumber++;
+      }
+      return {
+        base: baselineLineNumber,
+        current: currentLineNumber,
+        content: text ? [{text: text, className: 'double'}] : [],

[lushnikov, 2017/03/24 04:02:33]

s/double/inline-diff/

[einbinder, 2017/03/28 00:07:24]

done.

+        className: className,
+        loc: currentLineNumber

[lushnikov, 2017/03/24 04:02:33]

drop loc

[einbinder, 2017/03/28 00:07:12]

Done.

+      };
+    }
+  }
+
+  /**
+   * @param {number} lineNumber
+   * @return {string}
+   */
+  _lineFormatter(lineNumber) {
+    var row = this._rows[lineNumber - 1];
+    if (!row)
+      return spacesPadding(this._maxLineDigits * 2 + 1);

[lushnikov, 2017/03/24 04:02:33]

this is not needed

[einbinder, 2017/03/28 00:07:12]

Done.

+    var showBaseNumber = row.className === 'deletion';
+    var showCurrentNumber = row.className === 'addition';
+    if (row.className === 'equal') {

[lushnikov, 2017/03/24 04:02:34]

can we have row.className as an enum?

[einbinder, 2017/03/28 00:07:12]

Done.

+      showBaseNumber = true;
+      showCurrentNumber = true;
+    }
+    var base = showBaseNumber ? numberToStringWithSpacesPadding(row.base, this._maxLineDigits) :
+                                spacesPadding(this._maxLineDigits);
+    var current = showCurrentNumber ? numberToStringWithSpacesPadding(row.current, this._maxLineDigits) :
+                                      spacesPadding(this._maxLineDigits);
+    return base + spacesPadding(1) + current;
+  }
+};
+
+/** @typedef {!{base: number, current: number, content: !Array<!{text: string, className: string}>, className: string, loc: number}} */

[lushnikov, 2017/03/24 04:02:33]

baseLineNumber

lineLevelClassName

loc???

[einbinder, 2017/03/28 00:07:24]

Done. I used type instaed of lineLevelClassName

+Changes.ChangesView.Row;