From ea2fe19c2d0fe589fdce687ee720f4d9a33dfdb4 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Mon, 26 Jul 2021 15:57:46 +0100 Subject: [PATCH] Use functions instead of classes for formatter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both of these classes were performing side-effect work in their constructor and eventually calling the callback. Instead, these should become proper functions. In a follow-up CL, I will also remove the callback and refactor to return proper promises instead. R=szuend@chromium.org Bug: none Change-Id: I91d91f04c5ade75ee7e124719f1285a8a09a44be Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3053748 Commit-Queue: Tim van der Lippe Reviewed-by: Simon Zünd --- front_end/models/formatter/ScriptFormatter.ts | 69 +++++++------------ .../models/formatter/formatter-legacy.ts | 3 - .../components/source_frame/SourceFrame.ts | 2 +- 3 files changed, 25 insertions(+), 49 deletions(-) diff --git a/front_end/models/formatter/ScriptFormatter.ts b/front_end/models/formatter/ScriptFormatter.ts index 41e3ee5ab4..b66ff6aa43 100644 --- a/front_end/models/formatter/ScriptFormatter.ts +++ b/front_end/models/formatter/ScriptFormatter.ts @@ -33,7 +33,7 @@ import * as Common from '../../core/common/common.js'; import * as Platform from '../../core/platform/platform.js'; -import type {FormatMapping, FormatResult} from './FormatterWorkerPool.js'; +import type {FormatMapping} from './FormatterWorkerPool.js'; import {formatterWorkerPool} from './FormatterWorkerPool.js'; function locationToPosition(lineEndings: number[], lineNumber: number, columnNumber: number): number { @@ -57,55 +57,34 @@ export function format( contentType: Common.ResourceType.ResourceType, mimeType: string, content: string, callback: (arg0: string, arg1: FormatterSourceMapping) => Promise): void { if (contentType.isDocumentOrScriptOrStyleSheet()) { - new ScriptFormatter(mimeType, content, callback); + formatScriptContent(mimeType, content, callback); } else { - new ScriptIdentityFormatter(mimeType, content, callback); - } -} - -export class ScriptFormatter { - _mimeType: string; - _originalContent: string; - _callback: (arg0: string, arg1: FormatterSourceMapping) => Promise; - - constructor( - mimeType: string, content: string, callback: (arg0: string, arg1: FormatterSourceMapping) => Promise) { - this._mimeType = mimeType; - this._originalContent = content.replace(/\r\n?|[\n\u2028\u2029]/g, '\n').replace(/^\uFEFF/, ''); - this._callback = callback; - - this._initialize(); - } - - async _initialize(): Promise { - const pool = formatterWorkerPool(); - const indent = Common.Settings.Settings.instance().moduleSetting('textEditorIndent').get(); - - const formatResult = await pool.format(this._mimeType, this._originalContent, indent); - if (!formatResult) { - this._callback(this._originalContent, new IdentityFormatterSourceMapping()); - } else { - this._didFormatContent(formatResult); - } - } - - _didFormatContent(formatResult: FormatResult): void { - const originalContentLineEndings = Platform.StringUtilities.findLineEndingIndexes(this._originalContent); - const formattedContentLineEndings = Platform.StringUtilities.findLineEndingIndexes(formatResult.content); - - const sourceMapping = - new FormatterSourceMappingImpl(originalContentLineEndings, formattedContentLineEndings, formatResult.mapping); - this._callback(formatResult.content, sourceMapping); - } -} - -class ScriptIdentityFormatter { - constructor( - mimeType: string, content: string, callback: (arg0: string, arg1: FormatterSourceMapping) => Promise) { callback(content, new IdentityFormatterSourceMapping()); } } +export async function formatScriptContent( + mimeType: string, content: string, + callback: (arg0: string, arg1: FormatterSourceMapping) => Promise): Promise { + const originalContent = content.replace(/\r\n?|[\n\u2028\u2029]/g, '\n').replace(/^\uFEFF/, ''); + + const pool = formatterWorkerPool(); + const indent = Common.Settings.Settings.instance().moduleSetting('textEditorIndent').get(); + + const formatResult = await pool.format(mimeType, originalContent, indent); + if (!formatResult) { + callback(originalContent, new IdentityFormatterSourceMapping()); + return; + } + + const originalContentLineEndings = Platform.StringUtilities.findLineEndingIndexes(originalContent); + const formattedContentLineEndings = Platform.StringUtilities.findLineEndingIndexes(formatResult.content); + + const sourceMapping = + new FormatterSourceMappingImpl(originalContentLineEndings, formattedContentLineEndings, formatResult.mapping); + callback(formatResult.content, sourceMapping); +} + export abstract class FormatterSourceMapping { abstract originalToFormatted(lineNumber: number, columnNumber?: number): number[]; abstract formattedToOriginal(lineNumber: number, columnNumber?: number): number[]; diff --git a/front_end/models/formatter/formatter-legacy.ts b/front_end/models/formatter/formatter-legacy.ts index 4cad282acc..14e81280b7 100644 --- a/front_end/models/formatter/formatter-legacy.ts +++ b/front_end/models/formatter/formatter-legacy.ts @@ -14,9 +14,6 @@ Formatter.FormatterWorkerPool = FormatterModule.FormatterWorkerPool.FormatterWor Formatter.formatterWorkerPool = FormatterModule.FormatterWorkerPool.formatterWorkerPool; -/** @constructor */ -Formatter.ScriptFormatter = FormatterModule.ScriptFormatter.ScriptFormatter; - /** @interface */ Formatter.FormatterSourceMapping = FormatterModule.ScriptFormatter.FormatterSourceMapping; diff --git a/front_end/ui/legacy/components/source_frame/SourceFrame.ts b/front_end/ui/legacy/components/source_frame/SourceFrame.ts index 8dd138e68e..b13dcf0820 100644 --- a/front_end/ui/legacy/components/source_frame/SourceFrame.ts +++ b/front_end/ui/legacy/components/source_frame/SourceFrame.ts @@ -450,7 +450,7 @@ export class SourceFrameImpl extends UI.View.SimpleView implements UI.Searchable this._formattedContentPromise = new Promise(x => { fulfill = x; }); - new Formatter.ScriptFormatter.ScriptFormatter( + Formatter.ScriptFormatter.formatScriptContent( this._highlighterType, this._rawContent || '', async (content, map) => { fulfill({content, map}); });