Fix SQL parsing and HTML review findings#15
Merged
Conversation
● Bring over the legacy HTML CSS and JavaScript asset folder. ● Add companion HTML documentation and migration notes.
● Exclude double-at SQL Server system variables from parameter parsing ● Keep real bind parameters discoverable beside @@rowcount expressions ● Add regression coverage for SQL Server system variable parsing
There was a problem hiding this comment.
Pull request overview
Updates DBAccess SQL parameter parsing to avoid treating SQL Server @@... system variables as bind parameters, and adds a new set of static HTML/CSS/JavaScript UI framework assets with documentation.
Changes:
- Refines DBAccess parameter parsing regexes to exclude SQL Server
@@system variables (e.g.,@@ROWCOUNT) while still capturing normal@Parambinds. - Adds a regression unit test covering
@@ROWCOUNTmixed with regular bind parameters. - Introduces a repo-level
HTML/UI framework (CSS + JS components) and accompanying migration/docs updates.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| Toolkit.Modern/Data/Database/DBAccess.Parameters.cs | Regex refactor to avoid parsing @@... system variables as bind parameters. |
| Toolkit.Modern.Tests/Unit/Data/Database/DBAccessParameterParsingSimpleTests.cs | Adds regression coverage ensuring @@ROWCOUNT doesn’t produce @ROWCOUNT. |
| MIGRATION-legacy-toolkit.md | Documents new repo-level HTML/ asset folder and doc location. |
| HTML/README.md | Adds UI framework overview and usage notes. |
| HTML/JavaScript/README.md | Documents the browser-side helper scripts and integration guidance. |
| HTML/JavaScript/ByteForge-utilities.js | Adds general client-side utilities (validation, clipboard, security hardening). |
| HTML/JavaScript/ByteForge-tri-state-toggle.js | Adds a tri-state toggle UI component. |
| HTML/JavaScript/ByteForge-multi-selector.js | Adds a two-list selector helper component. |
| HTML/JavaScript/ByteForge-modal.js | Adds a modal/notification system component. |
| HTML/JavaScript/ByteForge-calendar.js | Adds a custom date picker widget component. |
| HTML/JavaScript/ByteForge-bulk-processor.js | Adds a preview-then-apply bulk workflow controller. |
| HTML/CSS/ByteForge-ui.css | Adds styling for the new UI framework components (modal, calendar, toggles, etc.). |
| Docs/HTML.md | Adds a documentation copy for the UI framework. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
● Correct validation and calendar behavior flagged during review ● Harden modal, selector, and bulk summary rendering against unsafe HTML ● Fix case-sensitive documentation links
PaulStSmith
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification