Fix doubleinputvalidator for large values (10⁴ and more)#33087
Merged
igorkorsukov merged 3 commits intomusescore:masterfrom Apr 24, 2026
Merged
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThe validator now computes the maximum integer-digit count from configured 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp`:
- Around line 88-93: Add a unit/vtest for the new large-value handling in
doubleinputvalidator by testing maxIntegerDigits behavior and boundary overflow:
create tests that instantiate the validator (or call maxIntegerDigits) with
m_top=99999 (and appropriate m_bottom) and assert that input "12345" is accepted
while values exceeding the top are rejected; also include a test for the edge
case when maxIntDigits == 1 (e.g., very small range) to ensure intPart
normalization to "0" or "-0" still behaves correctly. Reference the
maxIntegerDigits(m_top, m_bottom) logic and the code paths that normalize
intPart (the branches using QRegularExpression and QString("0")/QString("-0"))
when writing the tests so future regressions are caught.
- Line 145: Fix the uncrustify style violation by running the project's
codestyle formatter and applying its suggested formatting to the
DoubleInputValidator implementation (the method containing the assignment "state
= Acceptable;"). Run the uncrustify script used by CI, stage the modified
doubleinputvalidator.cpp, and amend the commit so the formatter-corrected
whitespace/indentation around the validate() logic (or the function that sets
state = Acceptable) matches the project's codestyle.
- Line 133: The quantifier in invalidZeroRegex can become invalid when
maxIntDigits == 1 (producing {2,1}); change the upper bound passed into the
regex to be at least 2 by clamping maxIntDigits (e.g., use qMax(2, maxIntDigits)
or similar) before constructing QRegularExpression in doubleinputvalidator.cpp
where invalidZeroRegex is defined so the pattern becomes valid for small ranges;
keep the rest of the regex logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1943edee-d3b3-45ce-8fc0-14c1574b6a64
📒 Files selected for processing (1)
src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp
saintmatthieu
force-pushed
the
double-input-validator-fix-for-large-values
branch
from
d37453c to
1a61b4e
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp (1)
145-145:⚠️ Potential issue | 🟡 MinorFix the remaining Uncrustify violation.
Check: Codestylestill fails for this file at Line 145. Please runtools/codestyle/uncrustify_run_file.sh src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cppand amend the commit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp` at line 145, There is a remaining Uncrustify codestyle violation at the assignment `state = Acceptable;` in doubleinputvalidator.cpp; run the formatter script (tools/codestyle/uncrustify_run_file.sh src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp), accept the produced changes (which will adjust spacing/indentation around the `state` assignment and nearby lines), then amend the commit so Codestyle passes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp`:
- Line 145: There is a remaining Uncrustify codestyle violation at the
assignment `state = Acceptable;` in doubleinputvalidator.cpp; run the formatter
script (tools/codestyle/uncrustify_run_file.sh
src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp),
accept the produced changes (which will adjust spacing/indentation around the
`state` assignment and nearby lines), then amend the commit so Codestyle passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9e69f397-39e6-4ee7-aa65-c62008494756
📒 Files selected for processing (1)
src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp
saintmatthieu
force-pushed
the
double-input-validator-fix-for-large-values
branch
from
1a61b4e to
23b7cca
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp`:
- Around line 29-38: The maxIntegerDigits function currently uses std::log10
which can misround for exact powers of ten; replace that with an integer-only
digit count: if maxAbs < 1.0 return 1, otherwise take uint64_t n =
static_cast<uint64_t>(std::floor(maxAbs)); count digits by repeatedly dividing n
by 10 (incrementing a counter) until n == 0 and return the counter; ensure
negatives are handled via std::llabs or std::fabs before casting and use the
function name maxIntegerDigits to locate and update the implementation
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ec6ed8ee-6b05-4d63-8463-582d0d9ff163
📒 Files selected for processing (2)
src/framework/uicomponents/qml/Muse/UiComponents/tests/doubleinputvalidator_tests.cppsrc/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp
saintmatthieu
force-pushed
the
double-input-validator-fix-for-large-values
branch
from
23b7cca to
2355789
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp (1)
94-99:⚠️ Potential issue | 🟡 MinorPreserve leading-zero handling for small ranges.
For
maxIntDigits == 1, inputs such as00.5are now rejected beforeinvalidZeroRegexcan classify them, andfixup()no longer normalizes the00integer part. Consider using a separate zero-normalization limit, at least the old fixed width, and checking leading-zero cases before the strict digit-count regex rejects them.Possible direction
const int maxIntDigits = maxIntegerDigits(m_top, m_bottom); - if (intPart.contains(QRegularExpression(QString("^0{1,%1}$").arg(maxIntDigits)))) { + const int maxZeroDigits = std::max(maxIntDigits, 3); + if (intPart.contains(QRegularExpression(QString("^0{1,%1}$").arg(maxZeroDigits)))) { intPart = QString("0"); - } else if (intPart.contains(QRegularExpression(QString("^\\-0{0,%1}$").arg(maxIntDigits)))) { + } else if (intPart.contains(QRegularExpression(QString("^\\-0{0,%1}$").arg(maxZeroDigits)))) { intPart = QString("-0"); }Then mirror that separate zero-specific allowance in
validate()before the strict\d{1,maxIntDigits}path.Also applies to: 135-155
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp` around lines 94 - 99, The current integer-part normalization uses maxIntDigits (from maxIntegerDigits(m_top, m_bottom)) which can be 1 and causes inputs like "00.5" to be rejected before zero-normalization runs; change the logic to use a separate zero-normalization limit (e.g., zeroNormWidth) that preserves the previous fixed-width allowance for leading-zero normalization, perform the leading-zero checks (the two QRegularExpression branches that set intPart to "0" or "-0") using that zeroNormWidth before any strict max-digit regex is applied, and mirror the same zero-specific allowance in the validate() path (the corresponding validation code around lines 135-155) so leading-zero cases are accepted for normalization even when maxIntDigits == 1; reference maxIntegerDigits(m_top,m_bottom), intPart variable, the QRegularExpression checks, and validate() to locate the spots to update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp`:
- Around line 25-32: The code uses std::max in the helper function
maxIntegerDigits (and elsewhere in the validator regex logic) but relies on a
transitive include; add a direct include of <algorithm> at the top of
doubleinputvalidator.cpp so std::max is guaranteed available (update the include
list near the existing <cmath> and other headers and rebuild to ensure no
transitive dependency remains).
---
Duplicate comments:
In
`@src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp`:
- Around line 94-99: The current integer-part normalization uses maxIntDigits
(from maxIntegerDigits(m_top, m_bottom)) which can be 1 and causes inputs like
"00.5" to be rejected before zero-normalization runs; change the logic to use a
separate zero-normalization limit (e.g., zeroNormWidth) that preserves the
previous fixed-width allowance for leading-zero normalization, perform the
leading-zero checks (the two QRegularExpression branches that set intPart to "0"
or "-0") using that zeroNormWidth before any strict max-digit regex is applied,
and mirror the same zero-specific allowance in the validate() path (the
corresponding validation code around lines 135-155) so leading-zero cases are
accepted for normalization even when maxIntDigits == 1; reference
maxIntegerDigits(m_top,m_bottom), intPart variable, the QRegularExpression
checks, and validate() to locate the spots to update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ff3f4aac-f48a-4ff6-ad24-53b9348fb724
📒 Files selected for processing (1)
src/framework/uicomponents/qml/Muse/UiComponents/validators/doubleinputvalidator.cpp
igorkorsukov
approved these changes
Apr 22, 2026
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
2 participants
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.
Needed for audacity/audacity#10745
Generalizes the double input validator (used e.g. by
IncrementalPropertyControl) to handle arbitrarily large numbers. At the moment it correctly handles up to three integers.