From 344ca478efaba040634f1bc9159f3653b7d17a0a Mon Sep 17 00:00:00 2001 From: Alex Ma Date: Thu, 9 Mar 2023 16:10:31 -0800 Subject: [PATCH] Fix for Edit Data grid newline on multiline strings (#22269) * Fix for newline replacement * added proper null regex and null character check * removed cell class * removed unnecessary function * revert back to old check * change onbeforeeditcell * small fix to typo in onBeforeEditCell * made changes based on feedback * added comments clarifying isDBCellValue --- .../editData/browser/editDataGridPanel.ts | 66 +++++++++++++++---- 1 file changed, 55 insertions(+), 11 deletions(-) diff --git a/src/sql/workbench/contrib/editData/browser/editDataGridPanel.ts b/src/sql/workbench/contrib/editData/browser/editDataGridPanel.ts index bfa13da202..c55c6091b9 100644 --- a/src/sql/workbench/contrib/editData/browser/editDataGridPanel.ts +++ b/src/sql/workbench/contrib/editData/browser/editDataGridPanel.ts @@ -37,6 +37,9 @@ import * as DOM from 'vs/base/browser/dom'; import { onUnexpectedError } from 'vs/base/common/errors'; import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility'; import { IQuickInputService } from 'vs/platform/quickinput/common/quickInput'; +import { localize } from 'vs/nls'; + +const cellWithNullCharMessage = localize('editData.cellWithNullCharMessage', "This cell contains the Unicode null character which is currently not supported for editing."); export class EditDataGridPanel extends GridParentComponent { // The time(in milliseconds) we wait before refreshing the grid. @@ -84,7 +87,7 @@ export class EditDataGridPanel extends GridParentComponent { public onCellEditEnd: (event: Slick.OnCellChangeEventArgs) => void; public onIsCellEditValid: (row: number, column: number, newValue: any) => boolean; public onIsColumnEditable: (column: number) => boolean; - public overrideCellFn: (rowNumber, columnId, value?, data?) => string; + public overrideCellFn: (columnId, value?, data?) => string; public loadDataFunction: (offset: number, count: number) => Promise<{}[]>; public onBeforeAppendCell: (row: number, column: number) => string; public onRefreshComplete: Promise; @@ -192,7 +195,7 @@ export class EditDataGridPanel extends GridParentComponent { self.currentEditCellValue = event.item[event.cell]; }; - this.overrideCellFn = (rowNumber, columnId, value?, data?): string => { + this.overrideCellFn = (columnId, value?, data?): string => { let returnVal = ''; // replace the line breaks with space since the edit text control cannot // render line breaks and strips them, updating the value. @@ -206,9 +209,12 @@ export class EditDataGridPanel extends GridParentComponent { returnVal = '\'NULL\''; } else if (Services.DBCellValue.isDBCellValue(value)) { + // If a cell is not edited and retrieved direct from the SQL server, it would be in the form of a DBCellValue. + // We use the DBCellValue's displayValue as the text value. returnVal = this.replaceLinebreaks(value.displayValue); } else if (typeof value === 'string') { + // Once a cell has been edited, the cell value will no longer be a DBCellValue until refresh. returnVal = this.replaceLinebreaks(value); } return returnVal; @@ -643,7 +649,8 @@ export class EditDataGridPanel extends GridParentComponent { ? self.rowIdMappings[self.currentCell.row] : self.currentCell.row; - return self.dataService.updateCell(sessionRowId, self.currentCell.column - 1, this.newlinePattern ? self.currentEditCellValue.replace('\u0000', this.newlinePattern) : self.currentEditCellValue); + let restoredValue = this.newlinePattern ? self.currentEditCellValue.replace(/\u0000/g, this.newlinePattern) : self.currentEditCellValue; + return self.dataService.updateCell(sessionRowId, self.currentCell.column - 1, restoredValue); }).then( result => { // last entered input is no longer needed as we have entered a valid input to commit. @@ -943,15 +950,15 @@ export class EditDataGridPanel extends GridParentComponent { this._textEditor.setValue(val); } - loadValue(item, rowNumber): void { - const itemForDisplay = deepClone(item); + loadValue(item): void { + let itemForEdit = deepClone(item); if (self.overrideCellFn) { - let overrideValue = self.overrideCellFn(rowNumber, this._args.column.id, itemForDisplay[this._args.column.id]); + let overrideValue = self.overrideCellFn(this._args.column.id, itemForEdit[this._args.column.id]); if (overrideValue !== undefined) { - itemForDisplay[this._args.column.id] = overrideValue; + itemForEdit[this._args.column.id] = overrideValue; } } - this._textEditor.loadValue(itemForDisplay); + this._textEditor.loadValue(itemForEdit); } serializeValue(): string { @@ -1110,7 +1117,7 @@ export class EditDataGridPanel extends GridParentComponent { this.onCellEditEnd(args); }); this.table.grid.onBeforeEditCell.subscribe((e, args) => { - this.onBeforeEditCell(args); + return this.onBeforeEditCell(args); }); // Subscribe to all active cell changes to be able to catch when we tab to the header on the next row this.table.grid.onActiveCellChanged.subscribe((e, args) => { @@ -1126,9 +1133,41 @@ export class EditDataGridPanel extends GridParentComponent { }); } - onBeforeEditCell(event: Slick.OnBeforeEditCellEventArgs): void { + onBeforeEditCell(event: Slick.OnBeforeEditCellEventArgs): boolean { + let result = true; this.logService.debug('onBeforeEditCell called with grid: ' + event.grid + ' row: ' + event.row + ' cell: ' + event.cell + ' item: ' + event.item + ' column: ' + event.column); + + let itemToEdit = event.item[event.cell]; + + if (Services.DBCellValue.isDBCellValue(itemToEdit)) { + // If a cell is not edited and retrieved direct from the SQL server, it would be in the form of a DBCellValue. + // We use it's displayValue to check if the cell contains unicode NULL and linebreaks. + result = !this.hasNullAndLinebreak(itemToEdit.displayValue) + } + else if (typeof itemToEdit === 'string' || (itemToEdit && itemToEdit.text)) { + // Once a cell has been edited, the cell value will no longer be a DBCellValue until refresh. + // In this case, just check directly if it's a string or if it's an item with .text, use the text. + if (itemToEdit.text) { + result = !this.hasNullAndLinebreak(itemToEdit.text); + } else { + result = !this.hasNullAndLinebreak(itemToEdit); + } + } + + if (!result) { + this.notificationService.warn(cellWithNullCharMessage); + } + + return result; + } + + private hasNullAndLinebreak(inputString: string): boolean { + if (inputString) { + let linebreakMatch = inputString.match(/(\r\n|\n|\r)/); + return linebreakMatch?.length > 0 && inputString.indexOf('\u0000') !== -1; + } + return false; } handleInitializeTable(): void { @@ -1146,7 +1185,7 @@ export class EditDataGridPanel extends GridParentComponent { /*Formatter for Column*/ - private getColumnFormatter(row: number | undefined, cell: any | undefined, value: any, columnDef: any | undefined, dataContext: any | undefined): string { + private getColumnFormatter(row: number | undefined, cell: number | undefined, value: any, columnDef: any | undefined, dataContext: any | undefined): string { let valueToDisplay = ''; let cellClasses = 'grid-cell-value-container'; /* tslint:disable:no-null-keyword */ @@ -1160,10 +1199,15 @@ export class EditDataGridPanel extends GridParentComponent { valueToDisplay = '\'NULL\''; } else if (Services.DBCellValue.isDBCellValue(value)) { + // If a cell is not edited and retrieved direct from the SQL server, it would be in the form of a DBCellValue. + // We use it's displayValue and remove newlines for display purposes only. valueToDisplay = (value.displayValue + ''); + valueToDisplay = valueToDisplay.replace(/(\r\n|\n|\r)/g, '\u0000'); valueToDisplay = escape(valueToDisplay.length > 250 ? valueToDisplay.slice(0, 250) + '...' : valueToDisplay); } else if (typeof value === 'string' || (value && value.text)) { + // Once a cell has been edited, the cell value will no longer be a DBCellValue until refresh. + // In this case, use directly if it's a string or if it's an item with .text, use the text. if (value.text) { valueToDisplay = value.text; } else {