From 0db8cc420655e01cbbed57c4658489b761a15899 Mon Sep 17 00:00:00 2001 From: undergroundwires Date: Thu, 4 Nov 2021 18:42:44 +0100 Subject: [PATCH] Fix website not loading on Safari It's caused by lookahead regex used in dash comment regex for inlining PowerShell. This commit changes dash comment inlining. - Change regex to one without lookahead. - Add more test cases for inlining dash comment in tricky situations. - Refactor makeInlineComment to be it's own function to easily test other regex options. - Document all regex alternatives. - Remove redundant null check (`||`) with adding safe navigation operator (`?`) to allow variable before check to be null instead of throwing exception. --- .../Pipes/PipeDefinitions/InlinePowerShell.ts | 67 +++++++++++++++++-- .../PipeDefinitions/InlinePowerShell.spec.ts | 22 ++++-- 2 files changed, 78 insertions(+), 11 deletions(-) diff --git a/src/application/Parser/Script/Compiler/Expressions/Pipes/PipeDefinitions/InlinePowerShell.ts b/src/application/Parser/Script/Compiler/Expressions/Pipes/PipeDefinitions/InlinePowerShell.ts index f7eee436..006b3ca9 100644 --- a/src/application/Parser/Script/Compiler/Expressions/Pipes/PipeDefinitions/InlinePowerShell.ts +++ b/src/application/Parser/Script/Compiler/Expressions/Pipes/PipeDefinitions/InlinePowerShell.ts @@ -6,7 +6,7 @@ export class InlinePowerShell implements IPipe { if (!code || !hasLines(code)) { return code; } - code = replaceComments(code); + code = inlineComments(code); code = mergeLinesWithBacktick(code); code = mergeHereStrings(code); const lines = getLines(code) @@ -25,18 +25,71 @@ function hasLines(text: string) { Line comments using "#" are replaced with inline comment syntax <# comment.. #> Otherwise single # comments out rest of the code */ -function replaceComments(code: string) { - return code.replaceAll(/#(?])(.*)$/gm, (_$, match1 ) => { - const value = match1?.trim(); +function inlineComments(code: string): string { + const makeInlineComment = (comment: string) => { + const value = comment?.trim(); if (!value) { return '<##>'; } return `<# ${value} #>`; + }; + return code.replaceAll(/<#.*?#>|#(.*)/g, (match, captureComment) => { + if (captureComment === undefined) { + return match; + } + return makeInlineComment(captureComment); }); + /* + Other alternatives considered: + -------------------------- + /#(?])(.*)$/gm + ------------------------- + ✅ Simple, yet matches and captures only what's necessary + ❌ Fails to match some cases + ❌ `Write-Host "hi" # Comment ending line inline comment but not one #>` + ❌ `Write-Host "hi" <#Comment starting like inline comment start but not one` + ❌ `Write-Host "hi" #>Comment starting like inline comment end but not one` + ❌ Uses lookbehind + Safari does not yet support lookbehind and syntax, leading application to not + load and throw "Invalid regular expression: invalid group specifier name" + https://caniuse.com/js-regexp-lookbehind + ⏩ Usage + return code.replaceAll(/#(?])(.*)$/gm, (match, captureComment) => { + return makeInlineComment(captureComment) + }); + ---------------- + /<#.*?#>|#(.*)/g + ---------------- + ✅ Simple yet affective + ❌ Matches all comments, but only captures dash comments + ❌ Fails to match some cases + ❌ `Write-Host "hi" # Comment ending line inline comment but not one #>` + ❌ `Write-Host "hi" <#Comment starting like inline comment start but not one` + ⏩ Usage + return code.replaceAll(/<#.*?#>|#(.*)/g, (match, captureComment) => { + if (captureComment === undefined) { + return match; + } + return makeInlineComment(captureComment); + }); + ------------------------------------ + /(^(?:<#.*?#>|[^#])*)(?:(#)(.*))?/gm + ------------------------------------ + ✅ Covers all cases + ❌ Matches every line, three capture groups are used to build result + ⏩ Usage + return code.replaceAll(/(^(?:<#.*?#>|[^#])*)(?:(#)(.*))?/gm, + (match, captureLeft, captureDash, captureComment) => { + if (!captureDash) { + return match; + } + return captureLeft + makeInlineComment(captureComment); + }); + */ } -function getLines(code: string) { - return (code.split(/\r\n|\r|\n/) || []); +function getLines(code: string): string [] { + return (code?.split(/\r\n|\r|\n/) || []); } /* @@ -59,7 +112,7 @@ interface IInlinedHereString { readonly escapedQuotes: string; readonly separator: string; } - // We handle @' and @" differently so single quotes are interpreted literally and doubles are expandable +// We handle @' and @" differently so single quotes are interpreted literally and doubles are expandable function getHereStringHandler(quotes: string): IInlinedHereString { const expandableNewLine = '`r`n'; switch (quotes) { diff --git a/tests/unit/application/Parser/Script/Compiler/Expressions/Pipes/PipeDefinitions/InlinePowerShell.spec.ts b/tests/unit/application/Parser/Script/Compiler/Expressions/Pipes/PipeDefinitions/InlinePowerShell.spec.ts index 0ac595af..30f59ba5 100644 --- a/tests/unit/application/Parser/Script/Compiler/Expressions/Pipes/PipeDefinitions/InlinePowerShell.spec.ts +++ b/tests/unit/application/Parser/Script/Compiler/Expressions/Pipes/PipeDefinitions/InlinePowerShell.spec.ts @@ -283,7 +283,7 @@ function getCommentCases(): IPipeTestCase[] { ), }, { - name: 'can convert comment with inline comment parts', + name: 'can convert comment with inline comment parts inside', input: getWindowsLines( '$text+= #Comment with < inside', '$text+= #Comment ending with >', @@ -295,14 +295,28 @@ function getCommentCases(): IPipeTestCase[] { '$text+= <# Comment with <# inline comment #> #>', ), }, + { + name: 'can convert comment with inline comment parts around', // Pretty uncommon + input: getWindowsLines( + 'Write-Host "hi" # Comment ending line inline comment but not one #>', + 'Write-Host "hi" #>Comment starting like inline comment end but not one', + // Following line does not compile as valid PowerShell referring to missing #> for inline comment + 'Write-Host "hi" <#Comment starting like inline comment start but not one', + ), + expectedOutput: getSingleLinedOutput( + 'Write-Host "hi" <# Comment ending line inline comment but not one #> #>', + 'Write-Host "hi" <# >Comment starting like inline comment end but not one #>', + 'Write-Host "hi" <<# Comment starting like inline comment start but not one #>', + ), + }, { name: 'converts empty hash comment', input: getWindowsLines( - 'Write-Host "Lorem ipsus" #', + 'Write-Host "Comment without text" #', 'Write-Host "Non-empty line"', ), expectedOutput: getSingleLinedOutput( - 'Write-Host "Lorem ipsus" <##>', + 'Write-Host "Comment without text" <##>', 'Write-Host "Non-empty line"', ), }, @@ -318,7 +332,7 @@ function getCommentCases(): IPipeTestCase[] { ), }, { - name: 'trims whitespaces around from match', + name: 'trims whitespaces around comment', input: getWindowsLines( '# Comment with whitespaces around ', '#\tComment with tabs around\t\t',