Skip to content

Commit

Permalink
Fix website not loading on Safari
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
undergroundwires committed Nov 4, 2021
1 parent 97ddc02 commit 0db8cc4
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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/) || []);
}

/*
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 >',
Expand All @@ -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"',
),
},
Expand All @@ -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',
Expand Down

0 comments on commit 0db8cc4

Please sign in to comment.