From 7d12d136b03dc4b1e4ec12e6a0bdf19e764314f3 Mon Sep 17 00:00:00 2001 From: theseanl Date: Tue, 23 Jan 2024 02:31:13 +0900 Subject: [PATCH] chore: simplify codes --- .../libs/argspec-parser.ts | 10 +- .../libs/argspec-types.ts | 4 +- .../__snapshots__/argspec-parser.test.ts.snap | 8 +- .../tests/argspec-parser.test.ts | 6 +- .../libs/gobble-arguments.ts | 10 +- .../libs/gobble-single-argument.ts | 2 +- .../libs/newcommand.ts | 138 +++++++++--------- .../grammars/macro-substitutions.pegjs | 2 +- .../grammars/xparse-argspec.pegjs | 6 +- packages/unified-latex-util-scan/libs/scan.ts | 8 +- 10 files changed, 92 insertions(+), 102 deletions(-) diff --git a/packages/unified-latex-util-argspec/libs/argspec-parser.ts b/packages/unified-latex-util-argspec/libs/argspec-parser.ts index 1f308174..cda6063a 100644 --- a/packages/unified-latex-util-argspec/libs/argspec-parser.ts +++ b/packages/unified-latex-util-argspec/libs/argspec-parser.ts @@ -35,7 +35,7 @@ function printRawInner(node: ArgSpec.Node) { const decorators = getDecorators(node); let spec = decorators; function appendDefaultArg() { - if ("defaultArg" in node && node.defaultArg) { + if (node.defaultArg) { spec = appendTokenOrGroup(spec, node.defaultArg); } } @@ -71,10 +71,10 @@ function printRawInner(node: ArgSpec.Node) { appendDefaultArg(); return spec; case "embellishment": - spec += node.embellishmentDefaultArg ? "E" : "e"; - appendCollection(node.embellishmentTokens); - if (node.embellishmentDefaultArg) { - appendCollection(node.embellishmentDefaultArg); + spec += node.defaultArgs ? "E" : "e"; + appendCollection(node.tokens); + if (node.defaultArgs) { + appendCollection(node.defaultArgs); } return spec; case "verbatim": diff --git a/packages/unified-latex-util-argspec/libs/argspec-types.ts b/packages/unified-latex-util-argspec/libs/argspec-types.ts index cc2dab2a..b20a3fa1 100644 --- a/packages/unified-latex-util-argspec/libs/argspec-types.ts +++ b/packages/unified-latex-util-argspec/libs/argspec-types.ts @@ -48,8 +48,8 @@ interface OptionalToken extends LeadingWhitespace, AstNode { } export interface Embellishment extends AstNode { type: "embellishment"; - embellishmentTokens: string[]; - embellishmentDefaultArg?: string[]; // Embellishment default arguments are always a collection of arguments + tokens: string[]; + defaultArgs?: string[]; // Embellishment default arguments are always a collection of arguments } interface Mandatory extends DefaultArgument, AstNode, Arg { type: "mandatory"; diff --git a/packages/unified-latex-util-argspec/tests/__snapshots__/argspec-parser.test.ts.snap b/packages/unified-latex-util-argspec/tests/__snapshots__/argspec-parser.test.ts.snap index 868687c5..67ad6417 100644 --- a/packages/unified-latex-util-argspec/tests/__snapshots__/argspec-parser.test.ts.snap +++ b/packages/unified-latex-util-argspec/tests/__snapshots__/argspec-parser.test.ts.snap @@ -37,11 +37,11 @@ exports[`unified-latex-util-argspec > parses xparse argument specification strin exports[`unified-latex-util-argspec > parses xparse argument specification string "E{\\token ^}{{D1}2}" 1`] = ` [ { - "embellishmentDefaultArg": [ + "defaultArgs": [ "D1", "2", ], - "embellishmentTokens": [ + "tokens": [ "\\\\token", "^", ], @@ -128,7 +128,7 @@ exports[`unified-latex-util-argspec > parses xparse argument specification strin "type": "mandatory", }, { - "embellishmentTokens": [ + "tokens": [ "^", ], "type": "embellishment", @@ -144,7 +144,7 @@ exports[`unified-latex-util-argspec > parses xparse argument specification strin "type": "mandatory", }, { - "embellishmentTokens": [ + "tokens": [ "_", "^", ], diff --git a/packages/unified-latex-util-argspec/tests/argspec-parser.test.ts b/packages/unified-latex-util-argspec/tests/argspec-parser.test.ts index f52a39a9..d48519da 100644 --- a/packages/unified-latex-util-argspec/tests/argspec-parser.test.ts +++ b/packages/unified-latex-util-argspec/tests/argspec-parser.test.ts @@ -53,14 +53,14 @@ describe("unified-latex-util-argspec", () => { it("Embellishments always return a string", () => { let ast = argspecParser.parse("e{{x}y{z}}"); expect(ast).toEqual([ - { type: "embellishment", embellishmentTokens: ["x", "y", "z"] }, + { type: "embellishment", tokens: ["x", "y", "z"] }, ]); ast = argspecParser.parse("E{{x}y{z}}{{One}{Two}{Three}}"); expect(ast).toEqual([ { type: "embellishment", - embellishmentTokens: ["x", "y", "z"], - embellishmentDefaultArg: ["One", "Two", "Three"], + tokens: ["x", "y", "z"], + defaultArgs: ["One", "Two", "Three"], }, ]); }); diff --git a/packages/unified-latex-util-arguments/libs/gobble-arguments.ts b/packages/unified-latex-util-arguments/libs/gobble-arguments.ts index ba47caa3..b1fead6a 100644 --- a/packages/unified-latex-util-arguments/libs/gobble-arguments.ts +++ b/packages/unified-latex-util-arguments/libs/gobble-arguments.ts @@ -36,9 +36,9 @@ export function gobbleArguments( // We need special behavior for embellishment argspecs. // Because an embellishment argspec specifies more than one argument, // we need to keep gobbling arguments until we've got them all. - const { embellishmentTokens } = spec; + const { tokens } = spec; - const remainingTokens = new Set(embellishmentTokens); + const remainingTokens = new Set(tokens); const tokenToArgs = new Map(); for (;;) { @@ -57,9 +57,7 @@ export function gobbleArguments( } args.push( - ...spec.embellishmentTokens.map( - (t) => tokenToArgs.get(t) || emptyArg() - ) + ...spec.tokens.map((t) => tokenToArgs.get(t) || emptyArg()) ); } else { const { argument, nodesRemoved: removed } = gobbleSingleArgument( @@ -81,6 +79,6 @@ export function gobbleArguments( function embellishmentSpec(tokens: Set): ArgSpec.Embellishment { return { type: "embellishment", - embellishmentTokens: [...tokens], + tokens: [...tokens], }; } diff --git a/packages/unified-latex-util-arguments/libs/gobble-single-argument.ts b/packages/unified-latex-util-arguments/libs/gobble-single-argument.ts index b86c81e6..c8a2e61a 100644 --- a/packages/unified-latex-util-arguments/libs/gobble-single-argument.ts +++ b/packages/unified-latex-util-arguments/libs/gobble-single-argument.ts @@ -201,7 +201,7 @@ export function gobbleSingleArgument( break; } case "embellishment": { - for (const token of argSpec.embellishmentTokens) { + for (const token of argSpec.tokens) { const bracePos = findBracePositions(nodes, currPos, token); if (!bracePos) { continue; diff --git a/packages/unified-latex-util-macros/libs/newcommand.ts b/packages/unified-latex-util-macros/libs/newcommand.ts index 088dc085..fa45a1d2 100644 --- a/packages/unified-latex-util-macros/libs/newcommand.ts +++ b/packages/unified-latex-util-macros/libs/newcommand.ts @@ -192,9 +192,9 @@ export function createMacroExpander( signature?: string ): (macro: Ast.Macro) => Ast.Node[] { const cachedSubstitutionTree = structuredClone(substitution); - const res = getMacroSubstitutionHashNumbers(cachedSubstitutionTree); + const hasSubstitutions = attachHashNumbers(cachedSubstitutionTree); - if (res.size === 0) { + if (!hasSubstitutions) { return () => structuredClone(cachedSubstitutionTree); } @@ -203,10 +203,8 @@ export function createMacroExpander( .map((node) => { if (node.type === "embellishment") { return ( - node.embellishmentDefaultArg || - (Array(node.embellishmentTokens.length).fill( - undefined - ) as undefined[]) + node.defaultArgs || + (Array(node.tokens.length).fill(undefined) as undefined[]) ); } return node.defaultArg; @@ -215,13 +213,12 @@ export function createMacroExpander( return (macro: Ast.Macro) => { const retTree = structuredClone(cachedSubstitutionTree); - const args = macro.args; const stack: number[] = []; let lastSelfReference: number | null = null; - // Recursively expand macro arguments. If a self-reference is found, it returns - // the corresponding hash number, which is used to special-case `O{#2} O{#1}`. + // Recursively expand macro arguments. If a self-reference is found, sets a + // lastSelfReference variable, which is used to special-case `O{#2} O{#1}`. function expandArgs(retTree: Ast.Node[]): void { replaceNode(retTree, (node) => { if (node.type !== "hash_number") { @@ -229,68 +226,66 @@ export function createMacroExpander( } const hashNum = node.number; - const arg = args?.[hashNum - 1]; + const arg = macro.args?.[hashNum - 1]; - // Check if this argument is -NoValue- - if (!arg || match.blankArgument(arg)) { - // Check if there exists a default argument for this hash number - const defaultArg = defaultArgs[hashNum - 1]; - if (!defaultArg) { - return s(`#${hashNum}`); - } - - // Detect self-references - if (stack.includes(hashNum)) { - lastSelfReference = hashNum; - return s(`#${hashNum}`); - } - - // `defaultArg` is a string expression. The same `defaultArg` may be parsed - // differently depending on the context of `macro`, so we cannot cache - // the parse result of `defaultArg`. Currently we just call `parse` without - // taking account of parsing contexts, so actually the result can be cached, - // but this is not the correct thing to do. FIXME: we should probably pass - // some options that is provided to whatever function that called this to - // the below parse call. Note that `parse` is done in several passes, and we - // may be able to cache result of a first few passes that aren't context-dependent. - const subst = parse(defaultArg).content; - const nextHashNums = getMacroSubstitutionHashNumbers(subst); + // If `arg` is provided, return it + if (arg && !match.blankArgument(arg)) { + return arg.content; + } - if (nextHashNums.size === 0) { - return subst; - } + // Check if there exists a default argument for this hash number + const defaultArg = defaultArgs[hashNum - 1]; + if (!defaultArg) { + return s(`#${hashNum}`); + } - stack.push(hashNum); - try { - expandArgs(subst); + // Detect self-references + if (stack.includes(hashNum)) { + lastSelfReference = hashNum; + return s(`#${hashNum}`); + } + // `defaultArg` is a string expression. The same `defaultArg` may be parsed + // differently depending on the context of `macro`, so we cannot cache + // the parse result of `defaultArg`. Currently we just call `parse` without + // taking account of parsing contexts, so actually the result can be cached, + // but this is not the correct thing to do. FIXME: we should probably pass + // some options that is provided to whatever function that called this to + // the below parse call. Note that `parse` is done in several passes, and we + // may be able to cache result of a first few passes that aren't context-dependent. + const subst = parse(defaultArg).content; + const hasSubstitutions = attachHashNumbers(subst); - if (lastSelfReference !== hashNum) { - return subst; - } + if (!hasSubstitutions) { + return subst; + } - // At this point, we have encountered #n while expanding #n. - // Check if we got exactly #n by expanding #n, - // in which case we should return the -NoValue-. - if (`#${hashNum}` === printRaw(subst)) { - // We are good, clear the last self-reference variable - lastSelfReference = null; - return emptyArg(); - } + stack.push(hashNum); + try { + expandArgs(subst); - console.warn( - `Detected unrecoverable self-reference while expanding macro: ${printRaw( - macro - )}` - ); - // Return a placeholder string, so that we know that - // this code path is not taken in unit tests. - return s("-Circular-"); - } finally { - stack.pop(); + if (lastSelfReference !== hashNum) { + return subst; + } + // At this point, we have encountered #n while expanding #n. + // Check if we got exactly #n by expanding #n, + // in which case we should return the -NoValue-. + if (`#${hashNum}` === printRaw(subst)) { + // We are good, clear the last self-reference variable + lastSelfReference = null; + return emptyArg(); } - } - return arg.content; + console.warn( + `Detected unrecoverable self-reference while expanding macro: ${printRaw( + macro + )}` + ); + // Return a placeholder string, so that we know that + // this code path is not taken in unit tests. + return s("-Circular-"); + } finally { + stack.pop(); + } }); } @@ -300,19 +295,18 @@ export function createMacroExpander( } /** - * Parses macro substitutions, mutates tree, and returns a list of hashnumbers that were encountered. + * Parses macro substitutions, mutates tree, and signal if there are any hash numbers. */ -export function getMacroSubstitutionHashNumbers(tree: Ast.Node[]) { - const hashNumbers = new Set(); +export function attachHashNumbers(tree: Ast.Node[]) { + let hasSubstitutions = false; visit( tree, (nodes) => { const parsed = parseMacroSubstitutions(nodes); - parsed.forEach((node) => { - if (node.type === "hash_number") { - hashNumbers.add(node.number); - } - }); + // Keep track of whether there are any substitutions so we can bail early if not. + hasSubstitutions = + hasSubstitutions || + parsed.some((node) => node.type === "hash_number"); nodes.length = 0; nodes.push(...(parsed as Ast.Node[])); }, @@ -321,5 +315,5 @@ export function getMacroSubstitutionHashNumbers(tree: Ast.Node[]) { test: Array.isArray, } ); - return hashNumbers; + return hasSubstitutions; } diff --git a/packages/unified-latex-util-pegjs/grammars/macro-substitutions.pegjs b/packages/unified-latex-util-pegjs/grammars/macro-substitutions.pegjs index b0a0736c..baa282ca 100644 --- a/packages/unified-latex-util-pegjs/grammars/macro-substitutions.pegjs +++ b/packages/unified-latex-util-pegjs/grammars/macro-substitutions.pegjs @@ -32,7 +32,7 @@ } catch (e) { console.warn("Error when initializing parser", e); } - }2 + } } body diff --git a/packages/unified-latex-util-pegjs/grammars/xparse-argspec.pegjs b/packages/unified-latex-util-pegjs/grammars/xparse-argspec.pegjs index 977de25b..8f5b4c8b 100644 --- a/packages/unified-latex-util-pegjs/grammars/xparse-argspec.pegjs +++ b/packages/unified-latex-util-pegjs/grammars/xparse-argspec.pegjs @@ -68,13 +68,13 @@ optional_standard optional_embellishment = "e" args:token_or_collection { return createNode("embellishment", { - embellishmentTokens:args + tokens:args }); } / "E" args:token_or_collection g:token_or_collection { return createNode("embellishment", { - embellishmentTokens: args, - embellishmentDefaultArg: g + tokens: args, + defaultArgs: g }); } diff --git a/packages/unified-latex-util-scan/libs/scan.ts b/packages/unified-latex-util-scan/libs/scan.ts index 4e1dffed..2350f8c9 100644 --- a/packages/unified-latex-util-scan/libs/scan.ts +++ b/packages/unified-latex-util-scan/libs/scan.ts @@ -32,18 +32,16 @@ export function scan( } ): number | null { const { - startIndex, - endIndex, + startIndex = 0, + endIndex = nodes.length - 1, onlySkipWhitespaceAndComments, allowSubstringMatches, } = options || {}; if (typeof token === "string") { token = { type: "string", content: token } as Ast.String; } - const start = typeof startIndex === "number" ? startIndex : 0; - const end = typeof endIndex === "number" ? endIndex : nodes.length - 1; - for (let i = start; i <= end; i++) { + for (let i = startIndex; i <= endIndex; i++) { const node = nodes[i]; if (node.type === token.type) { switch (node.type) {