-
-
Notifications
You must be signed in to change notification settings - Fork 220
add minizinc language support #931
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for livecodes ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Deploying livecodes with
|
| Latest commit: |
0d35428
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e637e866.livecodes.pages.dev |
| Branch Preview URL: | https://minizinc.livecodes.pages.dev |
|
Size Change: +6.67 kB (+0.66%) Total Size: 1.02 MB
ℹ️ View Unchanged
|
WalkthroughAdds first-class MiniZinc support: editor language definitions (Monaco/CodeMirror), language specs and browser runtime, starter template, vendor URLs and Prettier plugin, i18n/docs updates, build entries, SDK type additions, a DOM-ready utility, and a README badge tweak. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Page / UI
participant Starter as Starter Template
participant LC as livecodes.minizinc
participant Vendor as Minizinc runtime (vendor)
User->>Starter: Click "Run" / provide data
Starter->>LC: livecodes.minizinc.run({dzn?, json?, config?})
rect `#dbeffd`
LC->>Vendor: dynamic import / initialize runtime
Note right of LC: collect <text/minizinc> scripts\nassemble model & files
LC->>Vendor: invoke solver with model + options
end
Vendor-->>LC: results / errors
LC-->>Starter: result payload / error
Starter-->>User: render output or error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/livecodes/templates/starter/minizinc-starter.ts (1)
31-55: Consider hardening result handling for unexpected MiniZinc outputs
run()assumesresult.solution.outputis always present whenstatus !== 'ERROR'. If the runner ever returns other statuses or missingsolution, this will throw. A small guard would make the starter more robust, e.g. checkresult.solution && result.solution.outputbefore accessing nested properties and fall back to a generic message.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/livecodes/assets/templates/minizinc.pngis excluded by!**/*.png
📒 Files selected for processing (24)
README.md(1 hunks)docs/src/components/LanguageSliders.tsx(1 hunks)docs/src/components/TemplateList.tsx(1 hunks)scripts/build.js(2 hunks)src/livecodes/UI/command-menu-actions.ts(1 hunks)src/livecodes/editor/codemirror/editor-languages.ts(2 hunks)src/livecodes/editor/monaco/languages/monaco-lang-minizinc.ts(1 hunks)src/livecodes/editor/monaco/languages/monaco-lang-prolog.ts(1 hunks)src/livecodes/editor/monaco/monaco.ts(2 hunks)src/livecodes/html/language-info.html(1 hunks)src/livecodes/i18n/locales/en/language-info.lokalise.json(1 hunks)src/livecodes/i18n/locales/en/language-info.ts(1 hunks)src/livecodes/i18n/locales/en/translation.lokalise.json(1 hunks)src/livecodes/i18n/locales/en/translation.ts(1 hunks)src/livecodes/languages/languages.ts(2 hunks)src/livecodes/languages/minizinc/index.ts(1 hunks)src/livecodes/languages/minizinc/lang-minizinc-script.ts(1 hunks)src/livecodes/languages/minizinc/lang-minizinc.ts(1 hunks)src/livecodes/languages/prettier.ts(1 hunks)src/livecodes/templates/starter/index.ts(2 hunks)src/livecodes/templates/starter/minizinc-starter.ts(1 hunks)src/livecodes/vendors.ts(3 hunks)src/sdk/models.ts(4 hunks)vendor-licenses.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/livecodes/languages/prettier.ts (1)
src/livecodes/vendors.ts (1)
vendorsBaseUrl(5-6)
src/livecodes/templates/starter/minizinc-starter.ts (1)
src/sdk/models.ts (1)
Template(1379-1386)
src/livecodes/templates/starter/index.ts (1)
src/livecodes/templates/starter/minizinc-starter.ts (1)
minizincStarter(3-168)
src/livecodes/languages/languages.ts (1)
src/livecodes/languages/minizinc/lang-minizinc.ts (1)
minizinc(4-19)
src/livecodes/languages/minizinc/lang-minizinc.ts (3)
src/sdk/models.ts (1)
LanguageSpecs(1200-1213)src/livecodes/languages/prettier.ts (1)
parserPlugins(4-15)scripts/build.js (1)
baseUrl(40-40)
src/livecodes/languages/minizinc/lang-minizinc-script.ts (1)
src/livecodes/vendors.ts (1)
minizincUrl(301-301)
src/livecodes/editor/monaco/monaco.ts (1)
scripts/build.js (1)
baseUrl(40-40)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Redirect rules - livecodes
- GitHub Check: Header rules - livecodes
- GitHub Check: Pages changed - livecodes
- GitHub Check: tests (24.x, 2)
- GitHub Check: tests (24.x, 3)
- GitHub Check: tests (24.x, 1)
- GitHub Check: tests (24.x, 4)
- GitHub Check: tests (24.x, 5)
- GitHub Check: build (24.x)
- GitHub Check: build
🔇 Additional comments (31)
vendor-licenses.md (1)
151-152: LGTM!The license entry for
minizinc-jswith MPL-2.0 is correctly formatted and properly placed in alphabetical order.README.md (1)
16-16: LGTM!Language count badge correctly updated to reflect the addition of MiniZinc.
scripts/build.js (2)
163-164: LGTM!Monaco language definitions for MiniZinc and Prolog correctly added to the ESM build entry points, following the established pattern for other custom languages.
233-233: LGTM!MiniZinc script correctly added to the IIFE build, consistent with other language runtime scripts in this section.
src/livecodes/editor/codemirror/editor-languages.ts (1)
37-38: LGTM!Module URLs for MiniZinc and Prolog correctly added following the established pattern.
src/livecodes/editor/monaco/monaco.ts (3)
257-258: LGTM!MiniZinc and Prolog correctly added to the custom languages map using the established URL pattern with build-time hash replacement.
263-267: Clean extension of the interface.The optional
completionsfield is a well-designed addition that enables custom languages to provide autocomplete functionality without breaking existing language definitions.
296-298: LGTM!The completion provider registration is correctly guarded and follows the same pattern as the existing
configandtokensregistrations.src/livecodes/editor/monaco/languages/monaco-lang-prolog.ts (3)
1-26: LGTM!The language configuration is well-structured with proper comment syntax, bracket pairs, and auto-closing/surrounding pairs for Prolog.
27-60: LGTM!The tokenizer setup with atom/variable patterns and built-in predicates list provides good coverage for Prolog syntax highlighting.
107-124: LGTM!The sub-states for block comments and strings are correctly implemented with proper escape handling and state transitions.
src/livecodes/templates/starter/index.ts (1)
40-40: LGTM! MiniZinc starter template integration follows the established pattern.The import and export are correctly placed and consistent with other starter templates in the codebase.
Also applies to: 140-140
src/livecodes/languages/minizinc/index.ts (1)
1-1: LGTM! Standard barrel export.This follows the established pattern for language module exports in the codebase.
docs/src/components/TemplateList.tsx (1)
69-69: LGTM! Template list entry is correctly structured.The MiniZinc template entry follows the established pattern and is positioned consistently with other language integrations.
docs/src/components/LanguageSliders.tsx (1)
109-109: LGTM! Language slider entry is correct.The MiniZinc entry is properly added to the script languages list with correct structure and positioning.
src/livecodes/i18n/locales/en/translation.ts (1)
1-1: Translation entry looks correct, but verify the editing workflow.The MiniZinc translation entry is properly structured and positioned. However, note that line 1 indicates this is an auto-generated file. Ensure that manually editing this file is the correct approach, or if translations should be added through a different mechanism (e.g., via the source file referenced in the comment).
Also applies to: 1023-1023
src/livecodes/UI/command-menu-actions.ts (1)
326-326: LGTM! Command menu action entry is correctly placed.The MiniZinc entry is properly added to the starter templates list with consistent positioning.
src/livecodes/languages/prettier.ts (1)
14-14: Parser plugin entry looks correct, but verify file availability.The MiniZinc parser plugin entry is properly structured and follows the pattern of other custom parser plugins. However, ensure that
prettier/parser-minizinc.jsexists or will be available in the vendors bundle at the specified URL.src/livecodes/html/language-info.html (1)
921-952: No action needed. Verification confirms only one MiniZinc section exists in the file at line 921. The AI summary's claim of duplicate sections is inaccurate.src/livecodes/vendors.ts (3)
5-7: Vendor base URL bump is internally consistentThe new
vendorsBaseUrlaligns with the PrettierparserPlugins.minizincpath under the same@live-codes/browser-compilers@0.22.6/dist/base, so the MiniZinc parser URL wiring is coherent with existing patterns.
93-93: CodeMirror base URL bump looks safe and localized
codeMirrorBaseUrlonly updates the version path and remains consistent with other vendor constants; no behavioral risks apparent from this change alone.
301-302: MiniZinc runtime URL wiring matches vendor conventions
minizincUrlfollows the samegetUrl('<pkg>@<version>/dist/...')convention as other runtimes, which should make it straightforward to consume from the MiniZinc language/runtime layer.src/livecodes/templates/starter/minizinc-starter.ts (1)
3-24: Template metadata and markup wiring are consistentTemplate name, i18n key (
templates.starter.minizinc), thumbnail path, and basic markup structure (logo, data textarea, output area) all follow existing starter-template conventions.src/livecodes/i18n/locales/en/language-info.ts (1)
219-223: MiniZinc language-info entry matches existing patternsThe
minizincblock mirrors surrounding language entries in shape and style (desc/link/name), so it will integrate cleanly with the language-info typing and UI.src/livecodes/i18n/locales/en/translation.lokalise.json (1)
2699-2702: Starter template translation key is correctly wired
"templates.starter.minizinc"matches the key used inminizincStarterand follows the naming of other starter entries, so localization will resolve as expected.src/livecodes/languages/languages.ts (1)
44-45: MiniZinc is correctly registered in the language registryImporting
minizincand adding it to thelanguagesarray ensures it participates in language discovery and configuration alongside Prolog/PostgreSQL/etc.Also applies to: 170-170
src/livecodes/i18n/locales/en/language-info.lokalise.json (1)
495-506: MiniZinc localization entries are consistent with generator outputThe
minizinc.desc,.link, and.nameentries align with the TypeScriptlanguageInfostructure and follow the same tag/notes conventions as other languages.src/sdk/models.ts (1)
1088-1090: Type unions correctly extended for MiniZinc
Language,ParserName,Compiler['scriptType'], andTemplateNamenow all include the MiniZinc-related literals ('minizinc','mzn','dzn','text/minizinc'), matching howlang-minizinc.tsandminizincStarterreference them. This keeps the public SDK types in sync with the new language.Also applies to: 1260-1261, 1362-1362, 1457-1457
src/livecodes/languages/minizinc/lang-minizinc.ts (1)
1-19: MiniZinc LanguageSpecs align with parser and compiler infrastructureThe MiniZinc spec cleanly plugs into existing abstractions: parser name matches
ParserName/parserPlugins.minizinc,scriptType: 'text/minizinc'is now part ofCompiler['scriptType'], and extensions cover both.mzn/.dznand the canonical name. Identity compilation with a dedicated runtime script is consistent with other interpreted languages here.src/livecodes/editor/monaco/languages/monaco-lang-minizinc.ts (1)
1-307: Well-structured Monaco language definition for MiniZinc.The language configuration covers essential editor features correctly:
- Comments syntax (
%for line,%{ }%for block) matches MiniZinc specification- Tokenizer properly handles nested block comments via
@push/@pop- Completions provide helpful snippets with placeholders
src/livecodes/languages/minizinc/lang-minizinc-script.ts (1)
1-101: MiniZinc integration script structure looks good overall.The script correctly:
- Uses idempotent initialization with
hasRunflag- Waits for page load before accessing DOM
- Provides sensible defaults (10s timeout, gecode solver)
- Exposes configurable options for users
| // Brackets and punctuation | ||
| [/[[]\{\}\(\)\.,|]/, 'delimiter'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Malformed regex for bracket/punctuation matching.
The character class /[[]\{\}\(\)\.,|]/ is incorrect. The [[] creates a character class containing only [, then ] prematurely closes the class. This will cause incorrect tokenization.
- [/[[]\{\}\(\)\.,|]/, 'delimiter'],
+ [/[\[\]\{\}\(\)\.,|]/, 'delimiter'],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Brackets and punctuation | |
| [/[[]\{\}\(\)\.,|]/, 'delimiter'], | |
| // Brackets and punctuation | |
| [/[\[\]\{\}\(\)\.,|]/, 'delimiter'], |
🤖 Prompt for AI Agents
In src/livecodes/editor/monaco/languages/monaco-lang-prolog.ts around lines 100
to 101, the regex /[[]\{\}\(\)\.,|]/ is malformed because the character class is
closed prematurely; replace it with a proper character-class that includes all
bracket and punctuation characters (for example /[{}\[\]\(\)\.,|]/) so the
tokenizer correctly matches [, ], {, }, (, ), ., , and |; update the single line
to use this corrected regex literal.
|
ايه الحلاوة دي 😆👏😍 |
لغة لطيفة جدا |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/livecodes/templates/starter/minizinc-starter.ts (1)
88-105: CSS nesting syntax issue already flagged.The use of Sass-style nested selectors (
&:hover,&:disabled) in a plain CSS block has already been identified in previous reviews and needs to be addressed.
🧹 Nitpick comments (1)
src/livecodes/editor/monaco/languages/monaco-lang-minizinc.ts (1)
360-371: Consider using CompletionItemKind enum for better readability.The magic numbers (17, 18, 27, 4) work correctly but using the enum names would improve code clarity.
🔎 View suggested refactor
+import type { languages } from 'monaco-editor'; + const suggestions = [ - ...keywords.map((k) => createItem(k, 17 /* Keyword */, k)), - ...builtins.map((b) => createItem(b, 18 /* Function */, b, 'Built-in Function')), + ...keywords.map((k) => createItem(k, languages.CompletionItemKind.Keyword, k)), + ...builtins.map((b) => createItem(b, languages.CompletionItemKind.Function, b, 'Built-in Function')), ...snippets.map((s) => ({ label: s.label, - kind: 27 /* Snippet */, + kind: languages.CompletionItemKind.Snippet, insertText: s.insertText, - insertTextRules: 4 /* InsertAsSnippet */, + insertTextRules: languages.CompletionItemInsertTextRule.InsertAsSnippet, documentation: s.detail, range, })), ];
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/livecodes/editor/monaco/languages/monaco-lang-minizinc.ts(1 hunks)src/livecodes/languages/prettier.ts(2 hunks)src/livecodes/templates/starter/minizinc-starter.ts(1 hunks)src/livecodes/vendors.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/livecodes/languages/prettier.ts (1)
src/livecodes/vendors.ts (1)
prettierMinizincUrl(347-349)
src/livecodes/editor/monaco/languages/monaco-lang-minizinc.ts (1)
src/livecodes/languages/languages.ts (1)
languages(83-172)
src/livecodes/templates/starter/minizinc-starter.ts (1)
src/sdk/models.ts (1)
Template(1379-1386)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Redirect rules - livecodes
- GitHub Check: Header rules - livecodes
- GitHub Check: Pages changed - livecodes
- GitHub Check: tests (24.x, 4)
- GitHub Check: tests (24.x, 5)
- GitHub Check: tests (24.x, 2)
- GitHub Check: tests (24.x, 1)
- GitHub Check: tests (24.x, 3)
- GitHub Check: build
- GitHub Check: build (24.x)
- GitHub Check: build (24.x)
🔇 Additional comments (6)
src/livecodes/languages/prettier.ts (1)
1-1: LGTM!The MiniZinc parser plugin integration follows the established pattern for other Prettier plugins and is implemented correctly.
Also applies to: 12-12
src/livecodes/templates/starter/minizinc-starter.ts (2)
3-7: LGTM!The template metadata and configuration follow the established pattern for starter templates.
125-169: LGTM!The MiniZinc example code is well-structured and demonstrates key language features effectively. The source attribution to the official tutorial is a nice touch.
src/livecodes/editor/monaco/languages/monaco-lang-minizinc.ts (2)
5-35: LGTM!The language configuration correctly defines MiniZinc's syntax rules for comments, brackets, and folding regions.
37-281: LGTM!The tokenizer comprehensively handles MiniZinc syntax including string interpolation, nested comments, and Unicode operators. The implementation follows Monaco's Monarch tokenizer patterns correctly.
src/livecodes/vendors.ts (1)
301-302: Minizinc package version is current; verify prettier-plugin-minizinc availability.The minizinc package version 4.4.4 is the latest version, so that reference is correct. However, the @live-codes/prettier-plugin-minizinc@0.2.0 package could not be verified in the public npm registry. Confirm this package exists and is accessible in your build environment, or verify the correct package name and version if it's a private or internal package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/livecodes/utils/utils.ts (1)
689-689: Timing behavior differs significantly between code paths.When
readyStateis'interactive', the function executes immediately (DOM ready, but external resources may still be loading). When neither'complete'nor'interactive', it waits for the'load'event (all resources loaded). This creates inconsistent timing that could lead to race conditions depending on whenonLoadis called.Consider using
'DOMContentLoaded'if only DOM-ready semantics are needed, or document the timing expectations clearly.🔎 Alternative implementation using DOMContentLoaded
export const onLoad = /* @__PURE__ */ (fn: (...args: any[]) => any) => { - if (document.readyState === 'complete' || document.readyState === 'interactive') { + if (document.readyState !== 'loading') { fn(); } else { - window.addEventListener('load', fn); + document.addEventListener('DOMContentLoaded', fn, { once: true }); } };This ensures consistent DOM-ready semantics without waiting for all resources.
src/livecodes/languages/minizinc/lang-minizinc-script.ts (2)
40-40: Remove dead code: type guard is unreachable.The parameter
datais typed asMiniZincData = {}, sotypeof data === 'string'can never be true. This check is dead code.🔎 Proposed fix
hasRun = true; - if (typeof data === 'string') data = {}; const { dzn = '', json = '', config = {} } = data;
65-65: Consider API consistency:solvevsrunreturn types.The
solvemethod returns the raw result frommodel.solve()(appears to be an event emitter based on lines 71-76), while the asyncrunmethod wraps it in a Promise with event handling. This creates an inconsistent API surface.Consider whether
solveshould also return a Promise for consistency, or document the difference clearly.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/livecodes/languages/minizinc/lang-minizinc-script.tssrc/livecodes/utils/utils.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Redirect rules - livecodes
- GitHub Check: Header rules - livecodes
- GitHub Check: Pages changed - livecodes
- GitHub Check: tests (24.x, 3)
- GitHub Check: tests (24.x, 5)
- GitHub Check: tests (24.x, 1)
- GitHub Check: tests (24.x, 4)
- GitHub Check: tests (24.x, 2)
- GitHub Check: build (24.x)
- GitHub Check: build
- GitHub Check: build (24.x)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
src/livecodes/languages/minizinc/lang-minizinc-script.ts (6)
33-33: Module import starts eagerly at script evaluation time.The dynamic import begins immediately when the script is evaluated, not lazily on first use. This ensures faster initialization but loads the MiniZinc runtime even if
livecodes.minizincmethods are never called.This appears intentional given the auto-run pattern (lines 126-132), but be aware of the upfront loading cost.
35-57: Good implementation of model assembly and solving.The function correctly:
- Guards against uninitialized state
- Gathers MiniZinc code from all script tags
- Builds the model with optional data files
- Applies sensible defaults (gecode solver, 10s timeout) while allowing config overrides
60-64: LGTM: Clean initialization with proper guards.The init method correctly prevents double initialization and ensures both page load and module import complete before proceeding.
66-119: LGTM: Comprehensive error handling and safe property access.The async
runmethod properly:
- Ensures initialization before execution
- Handles errors via event listeners
- Manages non-zero exit codes
- Uses optional chaining to safely extract solution output (addresses past review comment)
- Logs results and resolves with the raw result object
120-123: LGTM: Direct module access without page load dependency.Unlike
init, this method doesn't wait forpageLoaded, only for the module import. This appears intentional since querying solvers doesn't require DOM access.
126-132: LGTM: Smart auto-execution with duplicate prevention.The auto-initialization pattern correctly waits for page load, initializes the runtime, and only executes if the user hasn't manually triggered
runorsolve. ThehasRunflag provides clean coordination.
| export const onLoad = /* @__PURE__ */ (fn: (...args: any[]) => any) => { | ||
| if (document.readyState === 'complete' || document.readyState === 'interactive') { | ||
| fn(); | ||
| } else { | ||
| window.addEventListener('load', fn); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add event listener cleanup to prevent memory leaks.
If onLoad is invoked multiple times before the page finishes loading, each call registers a new 'load' event listener that is never removed. This can lead to memory leaks and redundant executions.
Consider either:
- Using
{ once: true }option to auto-remove the listener after first execution - Storing and checking a flag to prevent multiple registrations
🔎 Proposed fix using the `once` option
export const onLoad = /* @__PURE__ */ (fn: (...args: any[]) => any) => {
if (document.readyState === 'complete' || document.readyState === 'interactive') {
fn();
} else {
- window.addEventListener('load', fn);
+ window.addEventListener('load', fn, { once: true });
}
};🤖 Prompt for AI Agents
In src/livecodes/utils/utils.ts around lines 688 to 694, onLoad currently adds a
'load' listener for every call which can leak if invoked multiple times before
load; update the implementation to either pass the { once: true } option to
window.addEventListener('load', fn, { once: true }) so the listener auto-removes
after firing, or add a simple module-level boolean (e.g., let loadRegistered =
false) to guard registration and set it true when you add the listener so
subsequent calls skip adding another listener; keep the immediate-ready-state
branch unchanged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/docs/languages/minizinc.mdx (1)
129-130: Consider consistent spelling variant.The text mixes "visualizations" (line 129) and "visualisations" (line 130). While both are correct (American vs. British English), using a consistent variant improves readability.
Suggested fix
-However, using the [helper methods](#usage-from-javascript), and based on [official implementations for visualisations](https://github.com/MiniZinc/libminizinc/tree/master/share/minizinc/std/ide), it should be possible to support them. +However, using the [helper methods](#usage-from-javascript), and based on [official implementations for visualizations](https://github.com/MiniZinc/libminizinc/tree/master/share/minizinc/std/ide), it should be possible to support them.src/livecodes/languages/minizinc/lang-minizinc-script.ts (2)
48-48: Clarify or remove the string type guard.Line 48 checks
if (typeof data === 'string')and converts it to{}, but the TypeScript signature specifiesdata: MiniZincData = {}, which is an object type. This guard may be unnecessary defensive programming or could indicate a type mismatch elsewhere.If this check is not needed, consider removing it for clarity:
- if (typeof data === 'string') data = {};
133-136: Minor: Variable shadowing ingetSolvers().Line 134 declares
const MiniZincwhich shadows the module-levelMiniZincvariable. While this works correctly, it may cause confusion.Consider removing the redundant assignment:
getSolvers: async () => { - const MiniZinc = await modPromise; - return MiniZinc.solvers(); + return (await modPromise).solvers(); },
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/docs/languages/minizinc.mdxdocs/docusaurus.config.tssrc/livecodes/languages/minizinc/lang-minizinc-script.tssrc/livecodes/templates/starter/minizinc-starter.tssrc/livecodes/utils/utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/livecodes/utils/utils.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/livecodes/templates/starter/minizinc-starter.ts (1)
src/sdk/models.ts (1)
Template(1379-1386)
src/livecodes/languages/minizinc/lang-minizinc-script.ts (3)
src/livecodes/main.ts (1)
livecodes(36-252)src/livecodes/utils/utils.ts (1)
onLoad(688-694)src/livecodes/vendors.ts (1)
minizincUrl(301-301)
🪛 LanguageTool
docs/docs/languages/minizinc.mdx
[uncategorized] ~130-~130: Do not mix variants of the same word (‘visualisation’ and ‘visualization’) within a single text.
Context: ... based on [official implementations for visualisations](https://github.com/MiniZinc/libminizin...
(EN_WORD_COHERENCY)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Redirect rules - livecodes
- GitHub Check: Header rules - livecodes
- GitHub Check: Pages changed - livecodes
- GitHub Check: tests (24.x, 1)
- GitHub Check: tests (24.x, 4)
- GitHub Check: tests (24.x, 3)
- GitHub Check: tests (24.x, 2)
- GitHub Check: tests (24.x, 5)
- GitHub Check: build
- GitHub Check: build (24.x)
- GitHub Check: build (24.x)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (10)
src/livecodes/templates/starter/minizinc-starter.ts (4)
3-7: LGTM!Template metadata is correctly structured and follows the expected
Templatetype definition.
29-81: Well-structured JavaScript integration.The inline JavaScript correctly uses the
livecodes.minizincAPI and handles both success and error cases. The optional chaining onresult.solution?.outputprovides safe access to nested properties.
110-129: CSS selectors correctly flattened.The button styles now use standard CSS selectors (
#button:hover,#button:disabled) instead of nested Sass-style syntax. This ensures the hover and disabled states will work correctly.
157-201: LGTM!The MiniZinc example code is well-documented, sourced from the official tutorial, and demonstrates constraint modeling effectively. Using
String.rawprevents escape sequence issues.docs/docs/languages/minizinc.mdx (1)
69-94: LGTM!The API documentation accurately describes the
livecodes.minizincinterface and matches the implementation insrc/livecodes/languages/minizinc/lang-minizinc-script.ts. The distinction betweensolve()(requires manualinit()) andrun()(auto-initializes) is clearly documented.src/livecodes/languages/minizinc/lang-minizinc-script.ts (4)
1-3: LGTM!The dynamic import at module level (line 33) enables preloading the MiniZinc runtime while the page loads, which optimizes initialization time.
Also applies to: 33-33
35-41: LGTM!The
getOutputhelper safely traverses the solution output hierarchy using optional chaining and provides a sensible fallback.
67-132: LGTM!The
livecodes.minizincAPI implementation correctly handles initialization, event-driven solving, and error cases. Therun()method properly wraps the solve progress object and handles errors, solutions, and exit events. The safe access toresult.solutionviagetOutput()addresses previous review concerns.
139-145: LGTM!The auto-run behavior on page load is well-designed: it only executes if the user hasn't manually invoked
run()orsolve(), preventing duplicate execution via thehasRunflag.docs/docusaurus.config.ts (1)
293-293:skipErrorChecking: trueis a pre-existing configuration, not related to the MiniZinc additions.This setting was already present in the docusaurus.config.ts file before the MiniZinc language additions. The MiniZinc changes are minimal string literal additions to existing union types in src/sdk/models.ts, which do not introduce type errors and do not require TypeDoc error checking to be disabled.
Likely an incorrect or invalid review comment.
|
I think this is ready to merge. |
|
i18n ActionsSource PR has been merged into the default branch. Maintainers can comment |
|
.i18n-update-push |
i18n Actions:
|
| Name | Description |
|---|---|
| New Branch for i18n | i18n/live-codes/minizinc |
| Last Commit SHA | 722bba3 |
Maintainers can comment .i18n-update-pull after translation is done to trigger the i18n pull workflow and pull the changes back to Github.
|
.i18n-update-pull |
i18n Actions:
|
| Name | Description |
|---|---|
| i18n Branch | i18n/live-codes/minizinc |
| Last Commit SHA | 44533fa |
| i18n PR | #933 |


What type of PR is this? (check all applicable)
Description
This PR adds support for MiniZinc language
@sharno I would appreciate it if you can have a look (thank you for letting me know about MiniZinc!)
http://minizinc.livecodes.pages.dev/?template=minizinc
Summary by CodeRabbit
New Features
Documentation
Chores
Other
✏️ Tip: You can customize this high-level summary in your review settings.