Fix radix string round trips with literal separators - #723
Open
Shubham-Padkonde wants to merge 2 commits into
Open
Shubham-Padkonde wants to merge 2 commits into
Shubham-Padkonde wants to merge 2 commits into
Conversation
Shubham-Padkonde
marked this pull request as ready for review
September 22, 2026 05:51
mkorbel1
approved these changes
Sep 22, 2026
mkorbel1
left a comment
Contributor
There was a problem hiding this comment.
This looks good to me, thank you for fixing!
Author
|
The latest Run Checks retry passes markdownlint (0 errors), but the Markdown link checker cannot reach https://code.visualstudio.com/Download (status 0). Could a maintainer rerun job 106873448545? The rerun API requires repository admin rights. I have not changed code or disabled the link check for this connectivity failure. Investigated with Codex assistance. |
Contributor
|
Sorry about that, just restarted those runs. I think we'll remove the markdown link checks soon, they are too flakey. |
This branch has not been deployed
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.
Description & Motivation
A radix string formatted with
sepChar: '',']', or a backslash cannot be parsed with the same separator. Accept an empty separator consistently withtoRadixStringand escape nonempty separators before including them in the parser's regular expression. Numeric parsing and default separator behavior are unchanged.Related Issue(s)
Fixes #722.
Testing
dart test test/logic_value_test.dart test/logic_value_width_test.dartpasses all 181 tests. Seven separator cases exercise all five supported radices, plus four-state values for non-decimal radices.dart analyze lib/src/values/logic_value.dart test/logic_value_test.dart: no issues.dart formatrun on both changed Dart files;git diff --checkpasses.dart doc: 0 warnings, 0 errors.dart test --fail-fastwas attempted but is not green: Icarus Verilog is missing locally, and the initial run also exhausted the Windows system temporary directory. Temporary files were redirected to D: for the successful two-module run. A subsequent attempt with portable Icarus 10.1 also failed in simulation tests. Full supported-environment validation is still outstanding; the hosted workflow is waiting for maintainer approval: https://github.com/intel/rohd/actions/runs/35691054318. The code change is ready for review, but full-suite success is not claimed.Backwards-compatibility
No public API changes. Existing separators and invalid radix-character checks remain in place; previously rejected formatter output becomes parseable.
Documentation
The parser's API comment explains literal and empty separators. A changelog entry is included.
Prepared and tested with OpenAI Codex assistance.