From fa20c25ac504b150b71b9b7b4996d880ee9d3366 Mon Sep 17 00:00:00 2001 From: "C. Marvin Sautter" Date: Thu, 12 Sep 2024 03:10:19 +0200 Subject: [PATCH 1/4] fix alias recursion problem by not applying the same rule twice --- src/Alias.ts | 10 ++++++---- test/jest/Alias/Alias.test.ts | 18 +++++++++++++++--- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/Alias.ts b/src/Alias.ts index f18daa6a83..51e90c8efc 100644 --- a/src/Alias.ts +++ b/src/Alias.ts @@ -93,16 +93,18 @@ function applyAliases(origCommand: string, depth = 0, currentlyProcessingAliases // First get non-global aliases, and recursively apply them // (unless there are any reference loops or the reference chain is too deep) const localAlias = Aliases.get(commandArray[0]); - if (localAlias && !currentlyProcessingAliases.includes(localAlias)) { - const appliedAlias = applyAliases(localAlias, depth + 1, [commandArray[0], ...currentlyProcessingAliases]); + const localrule = commandArray[0] + "->" + localAlias + "(g)"; + if (localAlias && !currentlyProcessingAliases.includes(localrule)) { + const appliedAlias = applyAliases(localAlias, depth + 1, [localrule, ...currentlyProcessingAliases]); commandArray.splice(0, 1, ...appliedAlias.split(" ")); } // Once local aliasing is complete (or if none are present) handle any global aliases const processedCommands = commandArray.reduce((resolvedCommandArray: string[], command) => { const globalAlias = GlobalAliases.get(command); - if (globalAlias && !currentlyProcessingAliases.includes(globalAlias)) { - const appliedAlias = applyAliases(globalAlias, depth + 1, [command, ...currentlyProcessingAliases]); + const rule = command + "->" + globalAlias + "(g)"; + if (globalAlias && !currentlyProcessingAliases.includes(rule)) { + const appliedAlias = applyAliases(globalAlias, depth + 1, [rule, ...currentlyProcessingAliases]); resolvedCommandArray.push(appliedAlias); } else { // If there is no alias, or if the alias has a circular reference, leave the command as-is diff --git a/test/jest/Alias/Alias.test.ts b/test/jest/Alias/Alias.test.ts index d5f594fb77..c2d6ef81ce 100644 --- a/test/jest/Alias/Alias.test.ts +++ b/test/jest/Alias/Alias.test.ts @@ -7,7 +7,19 @@ describe("substituteAliases Tests", () => { parseAliasDeclaration("d=recursiveAlias"); const result = substituteAliases("recursiveAlias"); - expect(result).toEqual("d"); + expect(result).toEqual("recursiveAlias"); + }); + + it("Should gracefully handle recursive local aliases II", () => { + parseAliasDeclaration("recursiveAlias=recursiveAlias"); + const result = substituteAliases("recursiveAlias"); + expect(result).toEqual("recursiveAlias"); + }); + + it("Should gracefully handle recursive local aliases II", () => { + parseAliasDeclaration('recursiveAlias="recursiveAlias -l"'); + const result = substituteAliases("recursiveAlias"); + expect(result).toEqual("recursiveAlias -l"); }); it("Should only change local aliases if they are the start of the command", () => { @@ -27,7 +39,7 @@ describe("substituteAliases Tests", () => { parseAliasDeclaration("d=a", true); const result = substituteAliases("a b c d"); - expect(result).toEqual("d a b c"); + expect(result).toEqual("a b c d"); }); it("Should gracefully handle recursive mixed local and global aliases", () => { @@ -37,7 +49,7 @@ describe("substituteAliases Tests", () => { parseAliasDeclaration("d=recursiveAlias", false); const result = substituteAliases("recursiveAlias"); - expect(result).toEqual("d"); + expect(result).toEqual("recursiveAlias"); }); it("Should replace chained aliases", () => { From 68d86face2fb52962d4fe66e5f7832d89f0262e4 Mon Sep 17 00:00:00 2001 From: "C. Marvin Sautter" Date: Thu, 12 Sep 2024 03:20:41 +0200 Subject: [PATCH 2/4] fix small typo --- src/Alias.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Alias.ts b/src/Alias.ts index 51e90c8efc..2876815e3c 100644 --- a/src/Alias.ts +++ b/src/Alias.ts @@ -93,7 +93,7 @@ function applyAliases(origCommand: string, depth = 0, currentlyProcessingAliases // First get non-global aliases, and recursively apply them // (unless there are any reference loops or the reference chain is too deep) const localAlias = Aliases.get(commandArray[0]); - const localrule = commandArray[0] + "->" + localAlias + "(g)"; + const localrule = commandArray[0] + "->" + localAlias + "(l)"; if (localAlias && !currentlyProcessingAliases.includes(localrule)) { const appliedAlias = applyAliases(localAlias, depth + 1, [localrule, ...currentlyProcessingAliases]); commandArray.splice(0, 1, ...appliedAlias.split(" ")); From c17f3d8b9f65438c4384f279bc8a06746a647298 Mon Sep 17 00:00:00 2001 From: "C. Marvin Sautter" Date: Thu, 12 Sep 2024 03:52:33 +0200 Subject: [PATCH 3/4] Improved tests --- src/Alias.ts | 6 +++++- test/jest/Alias/Alias.test.ts | 35 ++++++++++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/Alias.ts b/src/Alias.ts index 2876815e3c..143fbbed29 100644 --- a/src/Alias.ts +++ b/src/Alias.ts @@ -62,9 +62,13 @@ export function removeAlias(name: string): boolean { return hadAlias; } +export function clearAliases(): void { + Aliases.clear(); + GlobalAliases.clear(); +} + /** * Returns the original string with any aliases substituted in. - * Aliases are only applied to "whole words", one level deep * @param origCommand the original command string */ export function substituteAliases(origCommand: string): string { diff --git a/test/jest/Alias/Alias.test.ts b/test/jest/Alias/Alias.test.ts index c2d6ef81ce..ddb484fb12 100644 --- a/test/jest/Alias/Alias.test.ts +++ b/test/jest/Alias/Alias.test.ts @@ -1,6 +1,9 @@ -import { substituteAliases, parseAliasDeclaration } from "../../../src/Alias"; +import { substituteAliases, parseAliasDeclaration, clearAliases } from "../../../src/Alias"; describe("substituteAliases Tests", () => { - it("Should gracefully handle recursive local aliases", () => { + beforeEach(() => { + clearAliases(); + }); + it("Should gracefully handle recursive local aliases I", () => { parseAliasDeclaration("recursiveAlias=b"); parseAliasDeclaration("b=c"); parseAliasDeclaration("c=d"); @@ -16,12 +19,38 @@ describe("substituteAliases Tests", () => { expect(result).toEqual("recursiveAlias"); }); - it("Should gracefully handle recursive local aliases II", () => { + it("Should gracefully handle recursive local aliases III", () => { + parseAliasDeclaration('recursiveAlias="recursiveAlias"'); + const result = substituteAliases("recursiveAlias"); + expect(result).toEqual("recursiveAlias"); + }); + + it("Should gracefully handle recursive local aliases IV", () => { parseAliasDeclaration('recursiveAlias="recursiveAlias -l"'); const result = substituteAliases("recursiveAlias"); expect(result).toEqual("recursiveAlias -l"); }); + it("Should not substitute quoted commands I", () => { + parseAliasDeclaration("a=b"); + const result = substituteAliases('"a"'); + expect(result).toEqual('"a"'); + }); + + it("Should not substitute quoted commands II", () => { + parseAliasDeclaration("a=b"); + const result = substituteAliases("'a'"); + expect(result).toEqual("'a'"); + }); + + it("Should not substitute quoted commands III", () => { + parseAliasDeclaration("a=b"); + parseAliasDeclaration("b='c'"); + parseAliasDeclaration("c=d"); + const result = substituteAliases("a"); + //expect(result).toEqual("'c'"); // Currently FAILS + }); + it("Should only change local aliases if they are the start of the command", () => { parseAliasDeclaration("a=b"); parseAliasDeclaration("b=c"); From b469baf07949be18549b61587f35ef2646545eb9 Mon Sep 17 00:00:00 2001 From: "C. Marvin Sautter" Date: Thu, 12 Sep 2024 16:51:48 +0200 Subject: [PATCH 4/4] Use Set for currently processing aliases and skip failing tests --- src/Alias.ts | 12 ++++++------ test/jest/Alias/Alias.test.ts | 12 ++++++++++-- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/Alias.ts b/src/Alias.ts index 143fbbed29..f5b4d77522 100644 --- a/src/Alias.ts +++ b/src/Alias.ts @@ -83,7 +83,7 @@ export function substituteAliases(origCommand: string): string { * @param currentlyProcessingAliases any aliases that have been applied in the recursive evaluation leading to this point * @return { string } the provided command with all of its referenced aliases evaluated */ -function applyAliases(origCommand: string, depth = 0, currentlyProcessingAliases: string[] = []) { +function applyAliases(origCommand: string, depth = 0, currentlyProcessingAliases: Set = new Set()) { if (!origCommand) { return origCommand; } @@ -98,17 +98,17 @@ function applyAliases(origCommand: string, depth = 0, currentlyProcessingAliases // (unless there are any reference loops or the reference chain is too deep) const localAlias = Aliases.get(commandArray[0]); const localrule = commandArray[0] + "->" + localAlias + "(l)"; - if (localAlias && !currentlyProcessingAliases.includes(localrule)) { - const appliedAlias = applyAliases(localAlias, depth + 1, [localrule, ...currentlyProcessingAliases]); + if (localAlias && !currentlyProcessingAliases.has(localrule)) { + const appliedAlias = applyAliases(localAlias, depth + 1, new Set([localrule, ...currentlyProcessingAliases])); commandArray.splice(0, 1, ...appliedAlias.split(" ")); } // Once local aliasing is complete (or if none are present) handle any global aliases const processedCommands = commandArray.reduce((resolvedCommandArray: string[], command) => { const globalAlias = GlobalAliases.get(command); - const rule = command + "->" + globalAlias + "(g)"; - if (globalAlias && !currentlyProcessingAliases.includes(rule)) { - const appliedAlias = applyAliases(globalAlias, depth + 1, [rule, ...currentlyProcessingAliases]); + const globalrule = command + "->" + globalAlias + "(g)"; + if (globalAlias && !currentlyProcessingAliases.has(globalrule)) { + const appliedAlias = applyAliases(globalAlias, depth + 1, new Set([globalrule, ...currentlyProcessingAliases])); resolvedCommandArray.push(appliedAlias); } else { // If there is no alias, or if the alias has a circular reference, leave the command as-is diff --git a/test/jest/Alias/Alias.test.ts b/test/jest/Alias/Alias.test.ts index ddb484fb12..059d0cc725 100644 --- a/test/jest/Alias/Alias.test.ts +++ b/test/jest/Alias/Alias.test.ts @@ -43,12 +43,20 @@ describe("substituteAliases Tests", () => { expect(result).toEqual("'a'"); }); - it("Should not substitute quoted commands III", () => { + it.skip("Should not substitute quoted commands III", () => { parseAliasDeclaration("a=b"); parseAliasDeclaration("b='c'"); parseAliasDeclaration("c=d"); const result = substituteAliases("a"); - //expect(result).toEqual("'c'"); // Currently FAILS + expect(result).toEqual("'c'"); + }); + + it.skip("Should not substitute quoted commands IV", () => { + parseAliasDeclaration("a=b"); + parseAliasDeclaration('b="c"'); + parseAliasDeclaration("c=d"); + const result = substituteAliases("a"); + expect(result).toEqual('"c"'); }); it("Should only change local aliases if they are the start of the command", () => {