Skip to content

Commit

Permalink
Refactor text utilities and expand their usage
Browse files Browse the repository at this point in the history
This commit refactors existing text utility functions into the
application layer for broad reuse and integrates them across
the codebase. Initially, these utilities were confined to test
code, which limited their application.

Changes:

- Move text utilities to the application layer.
- Centralize text utilities into dedicated files for better
  maintainability.
- Improve robustness of utility functions with added type checks.
- Replace duplicated logic with centralized utility functions
  throughout the codebase.
- Expand unit tests to cover refactored code parts.
  • Loading branch information
undergroundwires committed Jul 18, 2024
1 parent 8d7a7eb commit 851917e
Show file tree
Hide file tree
Showing 45 changed files with 563 additions and 117 deletions.
2 changes: 1 addition & 1 deletion scripts/verify-build-artifacts.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ async function verifyFilesExist(directoryPath, filePatterns) {
if (!match) {
die(
`No file matches the pattern ${pattern.source} in directory \`${directoryPath}\``,
`\nFiles in directory:\n${files.map((file) => `\t- ${file}`).join('\n')}`,
`\nFiles in directory:\n${files.map((file) => `- ${file}`).join('\n')}`,
);
}
}
Expand Down
25 changes: 25 additions & 0 deletions src/application/Common/Text/FilterEmptyStrings.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { isArray } from '@/TypeHelpers';

export type OptionalString = string | undefined | null;

export function filterEmptyStrings(
texts: readonly OptionalString[],
isArrayType: typeof isArray = isArray,
): string[] {
if (!isArrayType(texts)) {
throw new Error(`Invalid input: Expected an array, but received type ${typeof texts}.`);
}
assertArrayItemsAreStringLike(texts);
return texts
.filter((title): title is string => Boolean(title));
}

function assertArrayItemsAreStringLike(
texts: readonly unknown[],
): asserts texts is readonly OptionalString[] {
const invalidItems = texts.filter((item) => !(typeof item === 'string' || item === undefined || item === null));
if (invalidItems.length > 0) {
const invalidTypes = invalidItems.map((item) => typeof item).join(', ');
throw new Error(`Invalid array items: Expected items as string, undefined, or null. Received invalid types: ${invalidTypes}.`);
}
}
29 changes: 29 additions & 0 deletions src/application/Common/Text/IndentText.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { isString } from '@/TypeHelpers';
import { splitTextIntoLines } from './SplitTextIntoLines';

export function indentText(
text: string,
indentLevel = 1,
utilities: TextIndentationUtilities = DefaultUtilities,
): string {
if (!utilities.isStringType(text)) {
throw new Error(`Indentation error: The input must be a string. Received type: ${typeof text}.`);
}
if (indentLevel <= 0) {
throw new Error(`Indentation error: The indent level must be a positive integer. Received: ${indentLevel}.`);
}
const indentation = '\t'.repeat(indentLevel);
return utilities.splitIntoLines(text)
.map((line) => (line ? `${indentation}${line}` : line))
.join('\n');
}

interface TextIndentationUtilities {
readonly splitIntoLines: typeof splitTextIntoLines;
readonly isStringType: typeof isString;
}

const DefaultUtilities: TextIndentationUtilities = {
splitIntoLines: splitTextIntoLines,
isStringType: isString,
};
11 changes: 11 additions & 0 deletions src/application/Common/Text/SplitTextIntoLines.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { isString } from '@/TypeHelpers';

export function splitTextIntoLines(
text: string,
isStringType = isString,
): string[] {
if (!isStringType(text)) {
throw new Error(`Line splitting error: Expected a string but received type '${typeof text}'.`);
}
return text.split(/\r\n|\r|\n/);
}
5 changes: 3 additions & 2 deletions src/application/Context/State/Code/Event/CodeChangedEvent.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { Script } from '@/domain/Executables/Script/Script';
import type { ICodePosition } from '@/application/Context/State/Code/Position/ICodePosition';
import type { SelectedScript } from '@/application/Context/State/Selection/Script/SelectedScript';
import { splitTextIntoLines } from '@/application/Common/Text/SplitTextIntoLines';
import type { ICodeChangedEvent } from './ICodeChangedEvent';

export class CodeChangedEvent implements ICodeChangedEvent {
Expand Down Expand Up @@ -52,12 +53,12 @@ export class CodeChangedEvent implements ICodeChangedEvent {
}

function ensureAllPositionsExist(script: string, positions: ReadonlyArray<ICodePosition>) {
const totalLines = script.split(/\r\n|\r|\n/).length;
const totalLines = splitTextIntoLines(script).length;
const missingPositions = positions.filter((position) => position.endLine > totalLines);
if (missingPositions.length > 0) {
throw new Error(
`Out of range script end line: "${missingPositions.map((pos) => pos.endLine).join('", "')}"`
+ `(total code lines: ${totalLines}).`,
+ ` (total code lines: ${totalLines}).`,
);
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/application/Context/State/Code/Generation/CodeBuilder.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { splitTextIntoLines } from '@/application/Common/Text/SplitTextIntoLines';
import type { ICodeBuilder } from './ICodeBuilder';

const TotalFunctionSeparatorChars = 58;
Expand All @@ -15,7 +16,7 @@ export abstract class CodeBuilder implements ICodeBuilder {
this.lines.push('');
return this;
}
const lines = code.match(/[^\r\n]+/g);
const lines = splitTextIntoLines(code);
if (lines) {
this.lines.push(...lines);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { splitTextIntoLines } from '@/application/Common/Text/SplitTextIntoLines';
import type { IPipe } from '../IPipe';

export class InlinePowerShell implements IPipe {
Expand Down Expand Up @@ -89,10 +90,6 @@ function inlineComments(code: string): string {
*/
}

function getLines(code: string): string[] {
return (code?.split(/\r\n|\r|\n/) || []);
}

/*
Merges inline here-strings to a single lined string with Windows line terminator (\r\n)
https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_quoting_rules?view=powershell-7.4#here-strings
Expand All @@ -102,7 +99,7 @@ function mergeHereStrings(code: string) {
return code.replaceAll(regex, (_$, quotes, scope) => {
const newString = getHereStringHandler(quotes);
const escaped = scope.replaceAll(quotes, newString.escapedQuotes);
const lines = getLines(escaped);
const lines = splitTextIntoLines(escaped);
const inlined = lines.join(newString.separator);
const quoted = `${newString.quotesAround}${inlined}${newString.quotesAround}`;
return quoted;
Expand Down Expand Up @@ -159,7 +156,7 @@ function mergeLinesWithBacktick(code: string) {
}

function mergeNewLines(code: string) {
return getLines(code)
return splitTextIntoLines(code)
.map((line) => line.trim())
.filter((line) => line.length > 0)
.join('; ');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { filterEmptyStrings } from '@/application/Common/Text/FilterEmptyStrings';
import type { CompiledCode } from '../CompiledCode';
import type { CodeSegmentMerger } from './CodeSegmentMerger';

Expand All @@ -8,11 +9,9 @@ export class NewlineCodeSegmentMerger implements CodeSegmentMerger {
}
return {
code: joinCodeParts(codeSegments.map((f) => f.code)),
revertCode: joinCodeParts(
codeSegments
.map((f) => f.revertCode)
.filter((code): code is string => Boolean(code)),
),
revertCode: joinCodeParts(filterEmptyStrings(
codeSegments.map((f) => f.revertCode),
)),
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { IExpressionsCompiler } from '@/application/Parser/Executable/Scrip
import { FunctionBodyType, type ISharedFunction } from '@/application/Parser/Executable/Script/Compiler/Function/ISharedFunction';
import type { FunctionCall } from '@/application/Parser/Executable/Script/Compiler/Function/Call/FunctionCall';
import type { CompiledCode } from '@/application/Parser/Executable/Script/Compiler/Function/Call/Compiler/CompiledCode';
import { indentText } from '@/application/Common/Text/IndentText';
import type { SingleCallCompilerStrategy } from '../SingleCallCompilerStrategy';

export class InlineFunctionCallCompiler implements SingleCallCompilerStrategy {
Expand All @@ -22,10 +23,12 @@ export class InlineFunctionCallCompiler implements SingleCallCompilerStrategy {
if (calledFunction.body.type !== FunctionBodyType.Code) {
throw new Error([
'Unexpected function body type.',
`\tExpected: "${FunctionBodyType[FunctionBodyType.Code]}"`,
`\tActual: "${FunctionBodyType[calledFunction.body.type]}"`,
indentText([
`Expected: "${FunctionBodyType[FunctionBodyType.Code]}"`,
`Actual: "${FunctionBodyType[calledFunction.body.type]}"`,
].join('\n')),
'Function:',
`\t${JSON.stringify(callToFunction)}`,
indentText(JSON.stringify(callToFunction)),
].join('\n'));
}
const { code } = calledFunction.body;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { NoDuplicatedLines } from '@/application/Parser/Executable/Script/Valida
import type { ICodeValidator } from '@/application/Parser/Executable/Script/Validation/ICodeValidator';
import { isArray, isNullOrUndefined, isPlainObject } from '@/TypeHelpers';
import { wrapErrorWithAdditionalContext, type ErrorWithContextWrapper } from '@/application/Parser/Common/ContextualError';
import { filterEmptyStrings } from '@/application/Common/Text/FilterEmptyStrings';
import { createFunctionWithInlineCode, createCallerFunction } from './SharedFunction';
import { SharedFunctionCollection } from './SharedFunctionCollection';
import { parseFunctionCalls, type FunctionCallsParser } from './Call/FunctionCallsParser';
Expand Down Expand Up @@ -82,8 +83,7 @@ function validateCode(
syntax: ILanguageSyntax,
validator: ICodeValidator,
): void {
[data.code, data.revertCode]
.filter((code): code is string => Boolean(code))
filterEmptyStrings([data.code, data.revertCode])
.forEach(
(code) => validator.throwIfInvalid(
code,
Expand Down Expand Up @@ -204,9 +204,9 @@ function ensureNoDuplicateCode(functions: readonly FunctionData[]) {
if (duplicateCodes.length > 0) {
throw new Error(`duplicate "code" in functions: ${printList(duplicateCodes)}`);
}
const duplicateRevertCodes = getDuplicates(callFunctions
.map((func) => func.revertCode)
.filter((code): code is string => Boolean(code)));
const duplicateRevertCodes = getDuplicates(filterEmptyStrings(
callFunctions.map((func) => func.revertCode),
));
if (duplicateRevertCodes.length > 0) {
throw new Error(`duplicate "revertCode" in functions: ${printList(duplicateRevertCodes)}`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { NoEmptyLines } from '@/application/Parser/Executable/Script/Validation/
import type { ICodeValidator } from '@/application/Parser/Executable/Script/Validation/ICodeValidator';
import { wrapErrorWithAdditionalContext, type ErrorWithContextWrapper } from '@/application/Parser/Common/ContextualError';
import { createScriptCode, type ScriptCodeFactory } from '@/domain/Executables/Script/Code/ScriptCodeFactory';
import { filterEmptyStrings } from '@/application/Common/Text/FilterEmptyStrings';
import { FunctionCallSequenceCompiler } from './Function/Call/Compiler/FunctionCallSequenceCompiler';
import { parseFunctionCalls } from './Function/Call/FunctionCallsParser';
import { parseSharedFunctions, type SharedFunctionsParser } from './Function/SharedFunctionsParser';
Expand Down Expand Up @@ -71,9 +72,7 @@ export class ScriptCompiler implements IScriptCompiler {
}

function validateCompiledCode(compiledCode: CompiledCode, validator: ICodeValidator): void {
[compiledCode.code, compiledCode.revertCode]
.filter((code): code is string => Boolean(code))
.map((code) => code as string)
filterEmptyStrings([compiledCode.code, compiledCode.revertCode])
.forEach(
(code) => validator.throwIfInvalid(
code,
Expand Down
4 changes: 2 additions & 2 deletions src/application/Parser/Executable/Script/ScriptParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type { ScriptCodeFactory } from '@/domain/Executables/Script/Code/ScriptC
import { createScriptCode } from '@/domain/Executables/Script/Code/ScriptCodeFactory';
import type { Script } from '@/domain/Executables/Script/Script';
import { createEnumParser, type EnumParser } from '@/application/Common/Enum';
import { filterEmptyStrings } from '@/application/Common/Text/FilterEmptyStrings';
import { parseDocs, type DocsParser } from '../DocumentationParser';
import { ExecutableType } from '../Validation/ExecutableType';
import { createExecutableDataValidator, type ExecutableValidator, type ExecutableValidatorFactory } from '../Validation/ExecutableValidator';
Expand Down Expand Up @@ -86,8 +87,7 @@ function validateHardcodedCodeWithoutCalls(
validator: ICodeValidator,
syntax: ILanguageSyntax,
) {
[scriptCode.execute, scriptCode.revert]
.filter((code): code is string => Boolean(code))
filterEmptyStrings([scriptCode.execute, scriptCode.revert])
.forEach(
(code) => validator.throwIfInvalid(
code,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { splitTextIntoLines } from '@/application/Common/Text/SplitTextIntoLines';
import type { ICodeLine } from './ICodeLine';
import type { ICodeValidationRule, IInvalidCodeLine } from './ICodeValidationRule';
import type { ICodeValidator } from './ICodeValidator';
Expand All @@ -24,12 +25,11 @@ export class CodeValidator implements ICodeValidator {
}

function extractLines(code: string): ICodeLine[] {
return code
.split(/\r\n|\r|\n/)
.map((lineText, lineIndex): ICodeLine => ({
index: lineIndex + 1,
text: lineText,
}));
const lines = splitTextIntoLines(code);
return lines.map((lineText, lineIndex): ICodeLine => ({
index: lineIndex + 1,
text: lineText,
}));
}

function printLines(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

<script lang="ts">
import { defineComponent, type PropType, computed } from 'vue';
import { splitTextIntoLines } from '@/application/Common/Text/SplitTextIntoLines';
import MarkdownText from '../Markdown/MarkdownText.vue';
export default defineComponent({
Expand Down Expand Up @@ -43,7 +44,7 @@ function formatAsMarkdownListItem(content: string): string {
if (content.length === 0) {
throw new Error('missing content');
}
const lines = content.split(/\r\n|\r|\n/);
const lines = splitTextIntoLines(content);
return `- ${lines[0]}${lines.slice(1)
.map((line) => `\n ${line}`)
.join()}`;
Expand All @@ -61,3 +62,4 @@ function formatAsMarkdownListItem(content: string): string {
font-size: $font-size-absolute-normal;
}
</style>
@/application/Text/SplitTextIntoLines
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { indentText, splitTextIntoLines } from '@tests/shared/Text';
import { indentText } from '@/application/Common/Text/IndentText';
import { splitTextIntoLines } from '@/application/Common/Text/SplitTextIntoLines';
import { log, die } from '../utils/log';
import { readAppLogFile } from './app-logs';
import { STDERR_IGNORE_PATTERNS } from './error-ignore-patterns';
Expand Down Expand Up @@ -172,5 +173,5 @@ function describeError(

function getNonEmptyLines(text: string) {
return splitTextIntoLines(text)
.filter((line) => line?.trim().length > 0);
.filter((line) => line.trim().length > 0);
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { filterEmpty } from '@tests/shared/Text';
import { splitTextIntoLines } from '@/application/Common/Text/SplitTextIntoLines';
import { filterEmptyStrings } from '@/application/Common/Text/FilterEmptyStrings';
import { runCommand } from '../../utils/run-command';
import { log, LogLevel } from '../../utils/log';
import { SupportedPlatform, CURRENT_PLATFORM } from '../../utils/platform';
Expand Down Expand Up @@ -56,7 +57,7 @@ async function captureTitlesOnLinux(processId: number): Promise<string[]> {
return [];
}

const windowIds = windowIdsOutput.trim().split('\n');
const windowIds = splitTextIntoLines(windowIdsOutput.trim());

const titles = await Promise.all(windowIds.map(async (windowId) => {
const { stdout: titleOutput, error: titleError } = await runCommand(
Expand All @@ -68,7 +69,7 @@ async function captureTitlesOnLinux(processId: number): Promise<string[]> {
return titleOutput.trim();
}));

return filterEmpty(titles);
return filterEmptyStrings(titles);
}

let hasAssistiveAccessOnMac = true;
Expand All @@ -78,7 +79,7 @@ async function captureTitlesOnMac(processId: number): Promise<string[]> {
if (!hasAssistiveAccessOnMac) {
return [];
}
const script = `
const command = constructAppleScriptCommand(`
tell application "System Events"
try
set targetProcess to first process whose unix id is ${processId}
Expand All @@ -93,13 +94,8 @@ async function captureTitlesOnMac(processId: number): Promise<string[]> {
return allWindowNames
end tell
end tell
`;
const argument = script.trim()
.split(/[\r\n]+/)
.map((line) => `-e '${line.trim()}'`)
.join(' ');

const { stdout: titleOutput, error } = await runCommand(`osascript ${argument}`);
`);
const { stdout: titleOutput, error } = await runCommand(command);
if (error) {
let errorMessage = '';
if (error.includes('-25211')) {
Expand All @@ -116,3 +112,13 @@ async function captureTitlesOnMac(processId: number): Promise<string[]> {
}
return [title];
}

function constructAppleScriptCommand(appleScriptCode: string): string {
const scriptLines = splitTextIntoLines(appleScriptCode.trim());
const trimmedLines = scriptLines.map((line) => line.trim());
const nonEmptyLines = filterEmptyStrings(trimmedLines);
const formattedArguments = nonEmptyLines
.map((line) => `-e '${line.trim()}'`)
.join(' ');
return `osascript ${formattedArguments}`;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { indentText } from '@tests/shared/Text';
import { indentText } from '@/application/Common/Text/IndentText';
import { logCurrentArgs, CommandLineFlag, hasCommandLineFlag } from './cli-args';
import { log, die } from './utils/log';
import { ensureNpmProjectDir, npmInstall, npmBuild } from './utils/npm';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { exec } from 'child_process';
import { indentText } from '@tests/shared/Text';
import { indentText } from '@/application/Common/Text/IndentText';
import type { ExecOptions, ExecException } from 'child_process';

const TIMEOUT_IN_SECONDS = 180;
Expand Down
Loading

0 comments on commit 851917e

Please sign in to comment.