Skip to content

Fix parseVersion regex, theme switcher init, changelog regex cache, and workflow label escaping - #70

Open
mobileskyfi with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-regex-for-pre-release-identifiers
Open

Fix parseVersion regex, theme switcher init, changelog regex cache, and workflow label escaping#70
mobileskyfi with Copilot wants to merge 3 commits into
mainfrom
copilot/fix-regex-for-pre-release-identifiers

Conversation

Copilot AI commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

Four independent correctness bugs in docs/restraml-shared.js and validate-workflows.mjs.

parseVersion — regex and preNum logic (restraml-shared.js:53)

The qualifier and digit groups were independent optionals, so 7.22beta (no trailing number) silently matched with preNum: 0 instead of returning null, causing incorrect version sorting.

// Before: two independent optionals — qualifier without digit silently matched
str.match(/^(\d+)\.(\d+)(?:\.(\d+))?(beta|rc)?(\d+)?$/)
// After: single optional group — qualifier requires digit
str.match(/^(\d+)\.(\d+)(?:\.(\d+))?(?:(beta|rc)(\d+))?$/)

The new regex also makes the preNum ternary simpler — m[5] is now guaranteed when m[4] exists:

// Before (m[4] ? 0 branch unreachable with new regex)
preNum: m[5] ? parseInt(m[5], 10) : (m[4] ? 0 : Infinity)
// After
preNum: m[4] ? parseInt(m[5], 10) : Infinity

initThemeSwitcher — icon never initialized (restraml-shared.js:211)

The initial icon was set inside a DOMContentLoaded listener registered inside initThemeSwitcher(), which is called after DOM load. The listener never fires, leaving the theme icon blank.

Replaced with a direct assignment at call time.

Changelog header regex — per-render new RegExp (restraml-shared.js)

showChangelog constructed new RegExp(...) on every call. Added _clHeaderRegexCache (a Map) so the compiled regex is reused across repeated lookups for the same version.

Workflow label escaping — broken character class (validate-workflows.mjs:40)

/[.*+?^${}()|[\\\]]/g has a misplaced backslash that breaks the character class boundary. Fixed by moving \\ to the end: /[.*+?^${}()|[\]\\]/g.

Original prompt
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"The regex pattern doesn't account for pre-release identifiers without numbers (e.g., 'beta' or 'rc' alone). Line 60 assumes `m[5]` exists when `m[4]` is present, but the regex makes `m[5]` optional even when a qualifier exists. This could cause parseVersion to return `preNum: 0` for a version like '7.22beta' (without a number), which would sort incorrectly. Consider making the pre-release number mandatory when a qualifier is present: `(beta|rc)(\\d+)` instead of `(beta|rc)?(\\d+)?`.","fixFiles":[{"filePath":"docs/restraml-shared.js","diff":"diff --git a/docs/restraml-shared.js b/docs/restraml-shared.js\n--- a/docs/restraml-shared.js\n+++ b/docs/restraml-shared.js\n@@ -50,7 +50,7 @@\n  *           \"7.15.3\" -> {major:7, minor:15, patch:3, pre:\"\", preNum:Infinity}\n  */\n function parseVersion(str) {\n-    const m = str.match(/^(\\d+)\\.(\\d+)(?:\\.(\\d+))?(beta|rc)?(\\d+)?$/)\n+    const m = str.match(/^(\\d+)\\.(\\d+)(?:\\.(\\d+))?(?:(beta|rc)(\\d+))?$/)\n     if (!m) return null\n     return {\n         major: parseInt(m[1], 10),\n"}]},{"message":"The DOMContentLoaded listener is registered inside `initThemeSwitcher()`, which is called after the DOM is loaded according to the documentation. If `initThemeSwitcher()` is called after DOMContentLoaded fires (as suggested by usage patterns in CLAUDE.md), this listener will never execute, leaving the icon uninitialized. Consider either setting `el.innerHTML` directly during initialization or documenting that `initThemeSwitcher()` must be called before DOMContentLoaded.","fixFiles":[{"filePath":"docs/restraml-shared.js","diff":"diff --git a/docs/restraml-shared.js b/docs/restraml-shared.js\n--- a/docs/restraml-shared.js\n+++ b/docs/restraml-shared.js\n@@ -208,9 +208,7 @@\n     const el = document.getElementById(id)\n     let state = 'auto'\n \n-    document.addEventListener('DOMContentLoaded', () => {\n-        el.innerHTML = _THEME_ICONS.osDefault\n-    })\n+    el.innerHTML = _THEME_ICONS.osDefault\n \n     el.addEventListener('click', e => {\n         e.preventDefault()\n"}]},{"message":"Creating a new RegExp object on every changelog render is inefficient. Since the version changes per call but the pattern is predictable, consider constructing the regex once or caching it. However, if performance is acceptable for the use case, this may be a minor optimization.","fixFiles":[{"filePath":"docs/restraml-shared.js","diff":"diff --git a/docs/restraml-shared.js b/docs/restraml-shared.js\n--- a/docs/restraml-shared.js\n+++ b/docs/restraml-shared.js\n@@ -422,6 +422,16 @@\n  * @param {string}               [opts.diffPage]    - Relative URL of the diff page (default: 'diff.html')\n  * @returns {{ showChangelog: function(version: string): void }}\n  */\n+const _clHeaderRegexCache = new Map()\n+function _clGetHeaderRegex(version) {\n+    let re = _clHeaderRegexCache.get(version)\n+    if (!re) {\n+        re = new RegExp(`What's new in ${_clEscapeRegex(version)} \\\\(([^)]+)\\\\)`, 'i')\n+        _clHeaderRegexCache.set(version, re)\n+    }\n+    return re\n+}\n+\n function initChangelogModal(opts) {\n     const modal = document.getElementById('changelog-modal')\n     if (!modal) return { showChangelog: () => {} }\n@@ -498,7 +508,7 @@\n             _rawText = text\n \n             // Extract release date for the subtitle\n-            const headerMatch = text.match(new RegExp(`What's new in ${_clEscapeRegex(version)} \\\\(([^)]+)\\\\)`, 'i'))\n+            const headerMatch = text.match(_clGetHeaderRegex(version))\n             if (headerMatch) subtitleEl.textContent = headerMatch[1]\n \n             renderChangelogContent(text, version, '', contentEl, itemCountEl)\n"}]},{"message":"The regex character class has an unescaped closing bracket `]` which breaks the escaping pattern. The pattern `[.*+?^${}()|[\\\\\\]]` should have the `]` escaped or moved to the start of the character class. Change to `/[.*+?^${}()|[\\]\\\\]/g` (moving backslash to end) or `/[.*+?^${}()|[\\\\\\]]/g` with proper escaping.","fixFiles":[{"filePath":"validate-workflows.mjs","diff":"diff --git a/validate-workflows.mjs b/validate-workflows.mjs\n--- a/validate-workflows.mjs\n+++ b/validate-workflows.mjs\n@@ -37,7 +37,7 @@\n \t\t// Match the label surrounded by quotes to avoid false positives from prefix\n \t\t// matches (e.g. allowed \"ubuntu-24.04-arm\" must not suppress an error about\n \t\t// the unknown label \"ubuntu-24.04-arm64\").\n-\t\tconst escapedLabel = label.replace(/[.*+?^${}()|[\\\\\\]]/g, \"\\\\$&\");\n+\t\tconst escapedLabel = label.replace(/[.*+?^${}()|[\\]\\\\]/g, \"\\\\$&\");\n \t\tconst quotedLabelPattern = new RegExp(`\"${escapedLabel}\"`, \"i\");\n \t\tif (quotedLabelPattern.test(message)) {\n \t\t\treturn true;\n"}]}]

Copilot AI and others added 2 commits April 25, 2026 01:34
…workflow validator

Agent-Logs-Url: https://github.com/tikoci/restraml/sessions/adb6bc76-a79e-4b2c-9cac-662eec232684

Co-authored-by: mobileskyfi <45924209+mobileskyfi@users.noreply.github.com>
…n m[4] does

Agent-Logs-Url: https://github.com/tikoci/restraml/sessions/adb6bc76-a79e-4b2c-9cac-662eec232684

Co-authored-by: mobileskyfi <45924209+mobileskyfi@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix regex pattern for pre-release identifiers handling Fix parseVersion regex, theme switcher init, changelog regex cache, and workflow label escaping Apr 25, 2026
Copilot AI requested a review from mobileskyfi April 25, 2026 01:39
@mobileskyfi
mobileskyfi marked this pull request as ready for review April 25, 2026 01:53
Copilot AI review requested due to automatic review settings April 25, 2026 01:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes several correctness issues in docs and tooling by tightening version parsing, ensuring theme switcher UI initializes reliably, reusing compiled changelog header regexes, and correcting workflow label escaping.

Changes:

  • Fix parseVersion() pre-release parsing by requiring digits after beta/rc and simplifying preNum logic.
  • Initialize the theme switcher icon immediately instead of waiting on a DOMContentLoaded handler that may never run.
  • Cache compiled changelog header regexes and fix a broken regex character class used for runner-label suppression.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
docs/restraml-shared.js Fix version parsing, initialize theme icon correctly, and add cached regex for changelog header matching.
validate-workflows.mjs Fix regex escaping so labels are safely embedded in a RegExp.

Comment thread docs/restraml-shared.js
Comment on lines +423 to +429
const _clHeaderRegexCache = new Map()
function _clGetHeaderRegex(version) {
let re = _clHeaderRegexCache.get(version)
if (!re) {
re = new RegExp(`What's new in ${_clEscapeRegex(version)} \\(([^)]+)\\)`, 'i')
_clHeaderRegexCache.set(version, re)
}

Copilot AI Apr 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cache can grow without bound because showChangelog(version) is a public API surface and can be called with arbitrarily many distinct version values (including user-driven input). Consider bounding the cache (LRU / max size), normalizing inputs, or avoiding caching unless the set of versions is known to be small and finite.

Suggested change
const _clHeaderRegexCache = new Map()
function _clGetHeaderRegex(version) {
let re = _clHeaderRegexCache.get(version)
if (!re) {
re = new RegExp(`What's new in ${_clEscapeRegex(version)} \\(([^)]+)\\)`, 'i')
_clHeaderRegexCache.set(version, re)
}
const _CL_HEADER_REGEX_CACHE_MAX = 128
const _clHeaderRegexCache = new Map()
function _clGetHeaderRegex(version) {
const cacheKey = String(version).trim()
let re = _clHeaderRegexCache.get(cacheKey)
if (re) {
_clHeaderRegexCache.delete(cacheKey)
_clHeaderRegexCache.set(cacheKey, re)
return re
}
re = new RegExp(`What's new in ${_clEscapeRegex(cacheKey)} \\(([^)]+)\\)`, 'i')
_clHeaderRegexCache.set(cacheKey, re)
if (_clHeaderRegexCache.size > _CL_HEADER_REGEX_CACHE_MAX) {
const oldestKey = _clHeaderRegexCache.keys().next().value
_clHeaderRegexCache.delete(oldestKey)
}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants