Bug 1520957 - [release 119] [Breakpoints] include breakpoint in column breakpoint (#7718). r=dwalsh
☠☠ backed out by 5b1c54cbac38 ☠ ☠
authorJason Laster <jason.laster.11@gmail.com>
Fri, 18 Jan 2019 12:17:33 -0500
changeset 454576 b74cd7c3ae9b23815cc5b863f1db2cc0d8c4a6fc
parent 454575 f1f01efee7672d83fc3940fa3785ebc7a4d68307
child 454577 07d417fb91d28f5b9566999ba672660e13f54a77
push id35400
push usercsabou@mozilla.com
push dateSat, 19 Jan 2019 09:59:33 +0000
treeherdermozilla-central@f90bab5af97e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdwalsh
bugs1520957
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1520957 - [release 119] [Breakpoints] include breakpoint in column breakpoint (#7718). r=dwalsh
devtools/client/debugger/new/src/components/Editor/ColumnBreakpoint.js
devtools/client/debugger/new/src/selectors/test/__snapshots__/visibleColumnBreakpoints.spec.js.snap
devtools/client/debugger/new/src/selectors/visibleColumnBreakpoints.js
--- a/devtools/client/debugger/new/src/components/Editor/ColumnBreakpoint.js
+++ b/devtools/client/debugger/new/src/components/Editor/ColumnBreakpoint.js
@@ -1,47 +1,48 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
 // @flow
 import React, { PureComponent } from "react";
 import ReactDOM from "react-dom";
 import classnames from "classnames";
+import actions from "../../actions";
 import { getDocument } from "../../utils/editor";
 import Svg from "../shared/Svg";
 
 // eslint-disable-next-line max-len
 import type { ColumnBreakpoint as ColumnBreakpointType } from "../../selectors/visibleColumnBreakpoints";
 
 type Bookmark = {
   clear: Function
 };
 
 type Props = {
   editor: Object,
   source: Object,
   enabled: boolean,
-  toggleBreakpoint: (number, number) => void,
+  toggleBreakpoint: typeof actions.toggleBreakpoint,
   columnBreakpoint: ColumnBreakpointType
 };
 
 const breakpointImg = document.createElement("div");
 ReactDOM.render(<Svg name={"column-marker"} />, breakpointImg);
-function makeBookmark(isActive, condition, { onClick }) {
+function makeBookmark({ breakpoint }, { onClick }) {
   const bp = breakpointImg.cloneNode(true);
-  const className = isActive ? "active" : "disabled";
+  const isActive = breakpoint && !breakpoint.disabled;
+  const condition = breakpoint && breakpoint.condition;
 
-  bp.className = classnames(
-    "column-breakpoint",
-    {
-      "has-condition": condition
-    },
-    className
-  );
+  bp.className = classnames("column-breakpoint", {
+    "has-condition": condition,
+    active: isActive,
+    disabled: !isActive
+  });
+
   if (condition) {
     bp.setAttribute("title", condition);
   }
   bp.onclick = onClick;
 
   return bp;
 }
 
@@ -54,23 +55,19 @@ export default class ColumnBreakpoint ex
 
     const sourceId = source.id;
     const doc = getDocument(sourceId);
     if (!doc) {
       return;
     }
 
     const { line, column } = columnBreakpoint.location;
-    const widget = makeBookmark(
-      columnBreakpoint.enabled,
-      columnBreakpoint.condition,
-      {
-        onClick: this.toggleBreakpoint
-      }
-    );
+    const widget = makeBookmark(columnBreakpoint, {
+      onClick: this.toggleBreakpoint
+    });
 
     this.bookmark = doc.setBookmark({ line: line - 1, ch: column }, { widget });
   };
 
   clearColumnBreakpoint = () => {
     if (this.bookmark) {
       this.bookmark.clear();
       this.bookmark = null;
--- a/devtools/client/debugger/new/src/selectors/test/__snapshots__/visibleColumnBreakpoints.spec.js.snap
+++ b/devtools/client/debugger/new/src/selectors/test/__snapshots__/visibleColumnBreakpoints.spec.js.snap
@@ -1,85 +1,101 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`visible column breakpoints duplicate generated locations 1`] = `
 Array [
   Object {
-    "condition": undefined,
-    "enabled": true,
+    "breakpoint": Object {
+      "location": Object {
+        "column": 1,
+        "line": 1,
+        "sourceId": "foo",
+      },
+    },
     "location": Object {
       "column": 1,
       "line": 1,
     },
   },
   Object {
-    "condition": null,
-    "enabled": false,
+    "breakpoint": undefined,
     "location": Object {
       "column": 3,
       "line": 1,
     },
   },
 ]
 `;
 
 exports[`visible column breakpoints ignores single breakpoints 1`] = `
 Array [
   Object {
-    "condition": undefined,
-    "enabled": true,
+    "breakpoint": Object {
+      "location": Object {
+        "column": 1,
+        "line": 1,
+        "sourceId": "foo",
+      },
+    },
     "location": Object {
       "column": 1,
       "line": 1,
     },
   },
   Object {
-    "condition": null,
-    "enabled": false,
+    "breakpoint": undefined,
     "location": Object {
       "column": 3,
       "line": 1,
     },
   },
 ]
 `;
 
 exports[`visible column breakpoints only shows visible breakpoints 1`] = `
 Array [
   Object {
-    "condition": undefined,
-    "enabled": true,
+    "breakpoint": Object {
+      "location": Object {
+        "column": 1,
+        "line": 1,
+        "sourceId": "foo",
+      },
+    },
     "location": Object {
       "column": 1,
       "line": 1,
     },
   },
   Object {
-    "condition": null,
-    "enabled": false,
+    "breakpoint": undefined,
     "location": Object {
       "column": 3,
       "line": 1,
     },
   },
 ]
 `;
 
 exports[`visible column breakpoints simple 1`] = `
 Array [
   Object {
-    "condition": undefined,
-    "enabled": true,
+    "breakpoint": Object {
+      "location": Object {
+        "column": 1,
+        "line": 1,
+        "sourceId": "foo",
+      },
+    },
     "location": Object {
       "column": 1,
       "line": 1,
     },
   },
   Object {
-    "condition": null,
-    "enabled": false,
+    "breakpoint": undefined,
     "location": Object {
       "column": 5,
       "line": 1,
     },
   },
 ]
 `;
--- a/devtools/client/debugger/new/src/selectors/visibleColumnBreakpoints.js
+++ b/devtools/client/debugger/new/src/selectors/visibleColumnBreakpoints.js
@@ -1,49 +1,62 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
-import { groupBy, get, sortedUniqBy } from "lodash";
+// @flow
+
+import { groupBy, sortedUniqBy } from "lodash";
 import { createSelector } from "reselect";
 
 import { getViewport } from "../selectors";
 import { getVisibleBreakpoints } from "./visibleBreakpoints";
 import { getVisiblePausePoints } from "./visiblePausePoints";
 import { makeLocationId } from "../utils/breakpoint";
+import type { Selector, PausePoint } from "../reducers/types";
 
-import type { SourceLocation } from "../types";
+import type {
+  SourceLocation,
+  PartialPosition,
+  Breakpoint,
+  Range
+} from "../types";
 
 export type ColumnBreakpoint = {|
   +location: SourceLocation,
-  +enabled: boolean,
-  +condition: ?string
+  +breakpoint: ?Breakpoint
 |};
 
-function contains(location, range) {
+export type ColumnBreakpoints = Array<ColumnBreakpoint>;
+
+function contains(location: PartialPosition, range: Range) {
   return (
     location.line >= range.start.line &&
     location.line <= range.end.line &&
-    location.column >= range.start.column &&
-    location.column <= range.end.column
+    (!location.column ||
+      (location.column >= range.start.column &&
+        location.column <= range.end.column))
   );
 }
 
 function groupBreakpoints(breakpoints) {
-  const map = groupBy(breakpoints, ({ location }) => location.line);
+  if (!breakpoints) {
+    return {};
+  }
+  const map: any = groupBy(breakpoints, ({ location }) => location.line);
   for (const line in map) {
     map[line] = groupBy(map[line], ({ location }) => location.column);
   }
 
   return map;
 }
 
 function findBreakpoint(location, breakpointMap) {
   const { line, column } = location;
-  const breakpoints = get(breakpointMap, [line, column]);
+  const breakpoints = breakpointMap[line] && breakpointMap[line][column];
 
   if (breakpoints) {
     return breakpoints[0];
   }
 }
 
 function getLineCount(columnBreakpoints) {
   const lineCount = {};
@@ -53,29 +66,35 @@ function getLineCount(columnBreakpoints)
     }
 
     lineCount[line] = lineCount[line] + 1;
   });
 
   return lineCount;
 }
 
-export function formatColumnBreakpoints(columnBreakpoints) {
+export function formatColumnBreakpoints(columnBreakpoints: ColumnBreakpoints) {
   console.log(
     "Column Breakpoints\n\n",
     columnBreakpoints
       .map(
-        ({ location, enabled }) =>
-          `(${location.line}, ${location.column}) ${enabled}`
+        ({ location, breakpoint }) =>
+          `(${location.line}, ${location.column || ""}) ${
+            breakpoint && breakpoint.disabled ? "disabled" : ""
+          }`
       )
       .join("\n")
   );
 }
 
-export function getColumnBreakpoints(pausePoints, breakpoints, viewport) {
+export function getColumnBreakpoints(
+  pausePoints: ?(PausePoint[]),
+  breakpoints: ?(Breakpoint[]),
+  viewport: Range
+) {
   if (!pausePoints) {
     return [];
   }
 
   const breakpointMap = groupBreakpoints(breakpoints);
 
   // We only want to show a column breakpoint if several conditions are matched
   // 1. it is a "break" point and not a "step" point
@@ -101,26 +120,22 @@ export function getColumnBreakpoints(pau
   );
 
   // 5. Check that there is atleast one other possible breakpoint on the line
   const lineCount = getLineCount(columnBreakpoints);
   columnBreakpoints = columnBreakpoints.filter(
     ({ location: { line } }) => lineCount[line] > 1
   );
 
-  return columnBreakpoints.map(({ location }) => {
-    // Find the breakpoint so if we know it's enabled and has condition
-    const foundBreakpoint = findBreakpoint(location, breakpointMap);
-
-    return {
-      location,
-      enabled: !!foundBreakpoint && !foundBreakpoint.disabled,
-      condition: foundBreakpoint ? foundBreakpoint.condition : null
-    };
-  });
+  return (columnBreakpoints: any).map(({ location }) => ({
+    location,
+    breakpoint: findBreakpoint(location, breakpointMap)
+  }));
 }
 
-export const visibleColumnBreakpoints = createSelector(
+export const visibleColumnBreakpoints: Selector<
+  ColumnBreakpoints
+> = createSelector(
   getVisiblePausePoints,
   getVisibleBreakpoints,
   getViewport,
   getColumnBreakpoints
 );