Merge pull request #5413 from Microsoft/dev/ianhu/releaseVarColumnSorting

IanMatthewHuff · web-flow · commit eec6aaaea462 · 2019-04-22T16:00:57.000-07:00
Port variable column sorting from master to release
diff --git a/news/1 Enhancements/5228.md b/news/1 Enhancements/5228.md
@@ -0,0 +1 @@
+Show a message when no variables are defined
diff --git a/news/1 Enhancements/5281.md b/news/1 Enhancements/5281.md
@@ -0,0 +1 @@
+Allow column sorting in variable explorer
diff --git a/package.nls.json b/package.nls.json
@@ -236,5 +236,6 @@
     "Common.noIWillDoItLater": "No, I will do it later",
     "Common.notNow": "Not now",
     "Common.gotIt": "Got it!",
-    "Interpreters.selectInterpreterTip": "Tip: you can change the Python interpreter used by the Python extension by clicking on the Python version in the status bar"
+    "Interpreters.selectInterpreterTip": "Tip: you can change the Python interpreter used by the Python extension by clicking on the Python version in the status bar",
+    "DataScience.noRowsInVariableExplorer": "No variables defined"
 }
diff --git a/src/datascience-ui/history-react/variableExplorer.tsx b/src/datascience-ui/history-react/variableExplorer.tsx
@@ -12,6 +12,7 @@ import { getSettings } from '../react-common/settingsReactSide';
 import { CollapseButton } from './collapseButton';
 import { VariableExplorerButtonCellFormatter } from './variableExplorerButtonCellFormatter';
 import { CellStyle, VariableExplorerCellFormatter } from './variableExplorerCellFormatter';
+import { VariableExplorerEmptyRowsView } from './variableExplorerEmptyRows';
 
 import * as AdazzleReactDataGrid from 'react-data-grid';
 
@@ -32,21 +33,27 @@ interface IVariableExplorerState {
     gridHeight: number;
     height: number;
     fontSize: number;
+    sortDirection: string;
+    sortColumn: string | number;
 }
 
 const defaultColumnProperties = {
     filterable: false,
-    sortable: false,
+    sortable: true,
     resizable: true
 };
 
+// Sanity check on our string comparisons
+const MaxStringCompare = 400;
+
 interface IGridRow {
     // tslint:disable-next-line:no-any
     [name: string]: any;
 }
 
 export class VariableExplorer extends React.Component<IVariableExplorerProps, IVariableExplorerState> {
     private divRef: React.RefObject<HTMLDivElement>;
+    private variableFetchCount: number;
 
     constructor(prop: IVariableExplorerProps) {
         super(prop);
@@ -55,16 +62,19 @@ export class VariableExplorer extends React.Component<IVariableExplorerProps, IV
             {key: 'type', name: getLocString('DataScience.variableExplorerTypeColumn', 'Type'), type: 'string', width: 120},
             {key: 'size', name: getLocString('DataScience.variableExplorerSizeColumn', 'Count'), type: 'string', width: 120, formatter: <VariableExplorerCellFormatter cellStyle={CellStyle.numeric} />},
             {key: 'value', name: getLocString('DataScience.variableExplorerValueColumn', 'Value'), type: 'string', width: 300},
-            {key: 'buttons', name: '', type: 'boolean', width: 34, formatter: <VariableExplorerButtonCellFormatter showDataExplorer={this.props.showDataExplorer} baseTheme={this.props.baseTheme} /> }
+            {key: 'buttons', name: '', type: 'boolean', width: 34, sortable: false, resizable: false, formatter: <VariableExplorerButtonCellFormatter showDataExplorer={this.props.showDataExplorer} baseTheme={this.props.baseTheme} /> }
         ];
         this.state = { open: false,
                         gridColumns: columns,
                         gridRows: [],
                         gridHeight: 200,
                         height: 0,
-                        fontSize: 14};
+                        fontSize: 14,
+                        sortColumn: 'name',
+                        sortDirection: 'NONE'};
 
         this.divRef = React.createRef<HTMLDivElement>();
+        this.variableFetchCount = 0;
     }
 
     public render() {
@@ -86,13 +96,15 @@ export class VariableExplorer extends React.Component<IVariableExplorerProps, IV
                     <div className={contentClassName}>
                         <div id='variable-explorer-data-grid'>
                             <AdazzleReactDataGrid
-                                columns = {this.state.gridColumns.map(c => { return {...c, ...defaultColumnProperties}; })}
+                                columns = {this.state.gridColumns.map(c => { return {...defaultColumnProperties, ...c }; })}
                                 rowGetter = {this.getRow}
                                 rowsCount = {this.state.gridRows.length}
                                 minHeight = {this.state.gridHeight}
                                 headerRowHeight = {this.state.fontSize + 9}
                                 rowHeight = {this.state.fontSize + 9}
                                 onRowDoubleClick = {this.rowDoubleClick}
+                                onGridSort = {this.sortRows}
+                                emptyRowsView = {VariableExplorerEmptyRowsView}
                             />
                         </div>
                     </div>
@@ -126,10 +138,15 @@ export class VariableExplorer extends React.Component<IVariableExplorerProps, IV
     // Help to keep us independent of main history window state if we choose to break out the variable explorer
     public newVariablesData(newVariables: IJupyterVariable[]) {
         const newGridRows = newVariables.map(newVar => {
-            return { buttons: {name: newVar.name, supportsDataExplorer: newVar.supportsDataExplorer}, name: newVar.name, type: newVar.type, size: '', value: getLocString('DataScience.variableLoadingValue', 'Loading...')};
+            return { buttons: {name: newVar.name, supportsDataExplorer: newVar.supportsDataExplorer},
+             name: newVar.name,
+             type: newVar.type,
+             size: '',
+             value: getLocString('DataScience.variableLoadingValue', 'Loading...')};
         });
 
         this.setState({ gridRows: newGridRows});
+        this.variableFetchCount = newGridRows.length;
     }
 
     // Update the value of a single variable already in our list
@@ -148,13 +165,22 @@ export class VariableExplorer extends React.Component<IVariableExplorerProps, IV
                     newSize = newVariable.count.toString();
                 }
 
-                const newGridRow = {...newGridRows[i], value: newVariable.value, size: newSize};
+                const newGridRow = {...newGridRows[i],
+                    value: newVariable.value,
+                    size: newSize};
 
                 newGridRows[i] = newGridRow;
             }
         }
 
-        this.setState({ gridRows: newGridRows });
+        // Update that we have retreived a new variable
+        // When we hit zero we have all the vars and can sort our values
+        this.variableFetchCount = this.variableFetchCount - 1;
+        if (this.variableFetchCount === 0) {
+            this.setState({ gridRows: this.internalSortRows(newGridRows, this.state.sortColumn, this.state.sortDirection) });
+        } else {
+            this.setState({ gridRows: newGridRows });
+        }
     }
 
     public toggleInputBlock = () => {
@@ -169,6 +195,94 @@ export class VariableExplorer extends React.Component<IVariableExplorerProps, IV
         this.props.variableExplorerToggled(!this.state.open);
     }
 
+    public sortRows = (sortColumn: string | number, sortDirection: string) => {
+        this.setState({
+            sortColumn,
+            sortDirection,
+            gridRows: this.internalSortRows(this.state.gridRows, sortColumn, sortDirection)
+        });
+    }
+
+    private getColumnType(key: string | number) : string | undefined {
+        let column;
+        if (typeof key === 'string') {
+            //tslint:disable-next-line:no-any
+            column = this.state.gridColumns.find(c => c.key === key) as any;
+        } else {
+            // This is the index lookup
+            column = this.state.gridColumns[key];
+        }
+
+        // Special case our size column, it's displayed as a string
+        // but we will sort it like a number
+        if (column && column.key === 'size') {
+            return 'number';
+        } else if (column && column.type) {
+            return column.type;
+        }
+    }
+
+    private internalSortRows = (gridRows: IGridRow[], sortColumn: string | number, sortDirection: string): IGridRow[] => {
+        // Default to the name column
+        if (sortDirection === 'NONE') {
+            sortColumn = 'name';
+            sortDirection = 'ASC';
+        }
+
+        const columnType = this.getColumnType(sortColumn);
+        const isStringColumn = columnType === 'string' || columnType === 'object';
+        const invert = sortDirection !== 'DESC';
+
+        // Use a special comparer for string columns as we can't compare too much of a string
+        // or it will take too long
+        const comparer = isStringColumn ?
+            //tslint:disable-next-line:no-any
+            (a: any, b: any): number => {
+                const aVal = a[sortColumn] as string;
+                const bVal = b[sortColumn] as string;
+                const aStr = aVal ? aVal.substring(0, Math.min(aVal.length, MaxStringCompare)).toUpperCase() : aVal;
+                const bStr = bVal ? bVal.substring(0, Math.min(bVal.length, MaxStringCompare)).toUpperCase() : bVal;
+                const result = aStr > bStr ? -1 : 1;
+                return invert ? -1 * result : result;
+            } :
+            //tslint:disable-next-line:no-any
+            (a: any, b: any): number => {
+                const aVal = this.getComparisonValue(a, sortColumn);
+                const bVal = this.getComparisonValue(b, sortColumn);
+                const result = aVal > bVal ? -1 : 1;
+                return invert ? -1 * result : result;
+            };
+
+        return gridRows.sort(comparer);
+    }
+
+    // Get the numerical comparison value for a column
+    private getComparisonValue(gridRow: IGridRow, sortColumn: string | number): number {
+        return (sortColumn === 'size') ? this.sizeColumnComparisonValue(gridRow) : gridRow[sortColumn];
+    }
+
+    // The size column needs special casing
+    private sizeColumnComparisonValue(gridRow: IGridRow): number {
+        const sizeStr: string = gridRow.size as string;
+
+        if (!sizeStr) {
+            return -1;
+        }
+
+        let sizeNumber = -1;
+        const commaIndex = sizeStr.indexOf(',');
+        // First check the shape case like so (5000,1000) in this case we want the 5000 to compare with
+        if (sizeStr[0] === '(' && commaIndex > 0) {
+            sizeNumber = parseInt(sizeStr.substring(1, commaIndex), 10);
+        } else {
+            // If not in the shape format, assume a to i conversion
+            sizeNumber = parseInt(sizeStr, 10);
+        }
+
+        // If our parse fails we get NaN for any case that like return -1
+        return isNaN(sizeNumber) ? -1 : sizeNumber;
+    }
+
     private rowDoubleClick = (_rowIndex: number, row: IGridRow) => {
         // On row double click, see if data explorer is supported and open it if it is
         if (row.buttons && row.buttons.supportsDataExplorer !== undefined
diff --git a/src/datascience-ui/history-react/variableExplorerEmptyRows.css b/src/datascience-ui/history-react/variableExplorerEmptyRows.css
@@ -0,0 +1,4 @@
+#variable-explorer-empty-rows {
+    margin: 5px;
+    font-family: var(--code-font-family);
+}
diff --git a/src/datascience-ui/history-react/variableExplorerEmptyRows.tsx b/src/datascience-ui/history-react/variableExplorerEmptyRows.tsx
@@ -0,0 +1,18 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+'use strict';
+import './variableExplorerEmptyRows.css';
+
+import * as React from 'react';
+import { getLocString } from '../react-common/locReactSide';
+
+export const VariableExplorerEmptyRowsView = () => {
+    // IANHU: Change
+    const message = getLocString('DataScience.noRowsInVariableExplorer', 'No variables defined');
+
+    return (
+        <div id='variable-explorer-empty-rows'>
+            {message}
+        </div>
+    );
+};
diff --git a/src/test/datascience/variableexplorer.functional.test.tsx b/src/test/datascience/variableexplorer.functional.test.tsx
@@ -220,6 +220,8 @@ myTuple = 1,2,3,4,5,6,7,8,9
             {name: 'myDataframe', value: '                  0\n0           0.00000\n1           2.00004\n2           4.00008\n3           6.00012\n4           8.00016\n5          10.00020\n6          12.00024\n7          14.00028\n8          16.00032\n', supportsDataExplorer: true, type: 'DataFrame', size: 54, shape: '', count: 0, truncated: false},
             {name: 'myFloat', value: '9999.9999', supportsDataExplorer: false, type: 'float', size: 58, shape: '', count: 0, truncated: false},
             {name: 'myInt', value: '99999999', supportsDataExplorer: false, type: 'int', size: 56, shape: '', count: 0, truncated: false},
+            {name: 'mynpArray', value: `[0.00000000e+00 2.00004000e+00 4.00008000e+00 ... 9.99959999e+04
+ 9.99980000e+04 1.00000000e+05]`, supportsDataExplorer: true, type: 'ndarray', size: 54, shape: '', count: 0, truncated: false},
             {name: 'mySeries', value: `0             0.00000
 1             2.00004
 2             4.00008
@@ -230,9 +232,44 @@ myTuple = 1,2,3,4,5,6,7,8,9
 7            14.00028
 8            16.00032
 9 `, supportsDataExplorer: true, type: 'Series', size: 54, shape: '', count: 0, truncated: false},
-            {name: 'myTuple', value: '(1, 2, 3, 4, 5, 6, 7, 8, 9)', supportsDataExplorer: false, type: 'tuple', size: 54, shape: '', count: 0, truncated: false},
-            {name: 'mynpArray', value: `[0.00000000e+00 2.00004000e+00 4.00008000e+00 ... 9.99959999e+04
- 9.99980000e+04 1.00000000e+05]`, supportsDataExplorer: true, type: 'ndarray', size: 54, shape: '', count: 0, truncated: false}
+            {name: 'myTuple', value: '(1, 2, 3, 4, 5, 6, 7, 8, 9)', supportsDataExplorer: false, type: 'tuple', size: 54, shape: '', count: 0, truncated: false}
+        ];
+        verifyVariables(wrapper, targetVariables);
+    }, () => { return ioc; });
+
+    runMountedTest('Variable explorer - Sorting', async (wrapper) => {
+        const basicCode: string = `b = 2
+c = 3
+stra = 'a'
+strb = 'b'
+strc = 'c'`;
+
+        openVariableExplorer(wrapper);
+
+        await addCode(getOrCreateHistory, wrapper, 'a=1\na');
+        await addCode(getOrCreateHistory, wrapper, basicCode, 4);
+
+        await waitForUpdate(wrapper, VariableExplorer, 7);
+
+        let targetVariables: IJupyterVariable[] = [
+            {name: 'a', value: '1', supportsDataExplorer: false, type: 'int', size: 54, shape: '', count: 0, truncated: false},
+            {name: 'b', value: '2', supportsDataExplorer: false, type: 'int', size: 54, shape: '', count: 0, truncated: false},
+            {name: 'c', value: '3', supportsDataExplorer: false, type: 'int', size: 54, shape: '', count: 0, truncated: false},
+            {name: 'stra', value: 'a', supportsDataExplorer: false, type: 'str', size: 54, shape: '', count: 0, truncated: false},
+            {name: 'strb', value: 'b', supportsDataExplorer: false, type: 'str', size: 54, shape: '', count: 0, truncated: false},
+            {name: 'strc', value: 'c', supportsDataExplorer: false, type: 'str', size: 54, shape: '', count: 0, truncated: false},
+        ];
+        verifyVariables(wrapper, targetVariables);
+
+        sortVariableExplorer(wrapper, 'value', 'DESC');
+
+        targetVariables = [
+            {name: 'strc', value: 'c', supportsDataExplorer: false, type: 'str', size: 54, shape: '', count: 0, truncated: false},
+            {name: 'strb', value: 'b', supportsDataExplorer: false, type: 'str', size: 54, shape: '', count: 0, truncated: false},
+            {name: 'stra', value: 'a', supportsDataExplorer: false, type: 'str', size: 54, shape: '', count: 0, truncated: false},
+            {name: 'c', value: '3', supportsDataExplorer: false, type: 'int', size: 54, shape: '', count: 0, truncated: false},
+            {name: 'b', value: '2', supportsDataExplorer: false, type: 'int', size: 54, shape: '', count: 0, truncated: false},
+            {name: 'a', value: '1', supportsDataExplorer: false, type: 'int', size: 54, shape: '', count: 0, truncated: false},
         ];
         verifyVariables(wrapper, targetVariables);
     }, () => { return ioc; });
@@ -249,6 +286,16 @@ function openVariableExplorer(wrapper: ReactWrapper<any, Readonly<{}>, React.Com
     }
 }
 
+function sortVariableExplorer(wrapper: ReactWrapper<any, Readonly<{}>, React.Component>, sortColumn: string, sortDirection: string) {
+    const varExp: VariableExplorer = wrapper.find('VariableExplorer').instance() as VariableExplorer;
+
+    assert(varExp);
+
+    if (varExp) {
+        varExp.sortRows(sortColumn, sortDirection);
+    }
+}
+
 // Verify a set of rows versus a set of expected variables
 function verifyVariables(wrapper: ReactWrapper<any, Readonly<{}>, React.Component>, targetVariables: IJupyterVariable[]) {
     const foundRows = wrapper.find('div.react-grid-Row');

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Show a message when no variables are defined`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Allow column sorting in variable explorer`
Original file line number	Diff line number	Diff line change
`@@ -236,5 +236,6 @@`
`236`	`236`	`"Common.noIWillDoItLater": "No, I will do it later",`
`237`	`237`	`"Common.notNow": "Not now",`
`238`	`238`	`"Common.gotIt": "Got it!",`
`239`		`- "Interpreters.selectInterpreterTip": "Tip: you can change the Python interpreter used by the Python extension by clicking on the Python version in the status bar"`
	`239`	`+ "Interpreters.selectInterpreterTip": "Tip: you can change the Python interpreter used by the Python extension by clicking on the Python version in the status bar",`
	`240`	`+ "DataScience.noRowsInVariableExplorer": "No variables defined"`
`240`	`241`	`}`