Version management, platform validation, fallback colors and fix text json#453
Open
felipecastrosales wants to merge 37 commits intoStacDev:devfrom
Open
Version management, platform validation, fallback colors and fix text json#453felipecastrosales wants to merge 37 commits intoStacDev:devfrom
felipecastrosales wants to merge 37 commits intoStacDev:devfrom
Conversation
… app version registration
…sion satisfaction check
Feature/version
…orts in stac_registry.dart and stac.dart
Feature/flutter35
feat: add platform validation for widget support in Stac class
- Introduced a new method to normalize and validate platform identifiers against a predefined list of supported platforms. - Updated platform checking logic to improve validation and logging of unsupported platforms. - Removed unused imports and cleaned up the code for better readability. - Added missing export for layout parsers in foundation.dart.
…d version management - Removed the deprecated platform identifier normalization logic from stac_service.dart. - Updated platform checks to utilize a new utility function for better consistency. - Simplified the StacVersion class constructor and adjusted platform-specific JSON handling to use the current platform string. - Added export for stac_platforms.dart in utils.dart to support the new platform handling logic.
- Changed the test to return true when the app version is null, indicating that the constraint is satisfied in this case. - Renamed a test for clarity, focusing on build number equality handling. - Updated comments for better understanding of platform-specific JSON handling.
Contributor
📝 WalkthroughWalkthroughAdds optional build-number initialization and runtime gating (version + platform) with custom-color parsing, a new StacVersion utility, expanded parser/utility exports, small stac_core text-style/span JSON adjustments, and updates stac_core dependencies to git references. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App
participant StacInit as Stac.initialize()
participant Service as StacService.initialize()
participant Registry as StacRegistry
participant Parser as StacService.fromJson()
participant Version as StacVersion
participant Platform as PlatformCheck
App->>StacInit: initialize(buildNumber: N)
StacInit->>Service: initialize(buildNumber: N)
Service->>Registry: registerBuildNumber(N)
Registry-->>Service: stored
App->>Parser: fromJson(componentJson)
alt version present
Parser->>Version: StacVersion.fromJson(versionData)
Version-->>Parser: versionObj
Parser->>Version: isSatisfied(N)
alt not satisfied
Version-->>Parser: false
Parser-->>App: null (filtered)
else satisfied
Parser->>Platform: validate(componentJson.platform)
alt platform mismatch
Platform-->>Parser: unsupported
Parser-->>App: null (filtered)
else supported
Parser-->>App: component instance
end
end
else no version
Parser->>Platform: validate(componentJson.platform)
alt platform mismatch
Platform-->>Parser: unsupported
Parser-->>App: null (filtered)
else supported
Parser-->>App: component instance
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/stac/pubspec.yaml (1)
26-31:⚠️ Potential issue | 🟠 MajorSame git dependency concerns as in
stac_webview/pubspec.yaml.This package has identical issues with the git-based stac_core dependency:
Organization mismatch: Git URL (
SuaMusica/stac) differs from this package's repository (StacDev/stacat line 4).Non-reproducible builds: Branch ref
devallcan change at any time. Consider pinning to a specific commit SHA if a versioned release isn't available yet:Suggested alternative using commit SHA
# when approved, use stac_core: ^1.3.x stac_core: git: url: https://github.com/SuaMusica/stac.git path: packages/stac_core - ref: devall + ref: <commit-sha> # Pin to specific commit for reproducibility
- Track the revert: Add an issue reference to ensure this temporary configuration is reverted before release.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stac/pubspec.yaml` around lines 26 - 31, The git dependency for stac_core currently points to url: https://github.com/SuaMusica/stac and ref: devall which causes organization mismatch and non-reproducible builds; update the stac_core dependency entry to reference the correct repository/org (replace SuaMusica with StacDev in the git url or point to the canonical repo used by this package), pin the ref to a specific commit SHA instead of the branch name (replace ref: devall with ref: <commit-sha>), and add a TODO/issue reference comment noting this is temporary and must be reverted to a versioned release before publishing.
🧹 Nitpick comments (4)
packages/stac/lib/src/parsers/widgets/widgets.dart (2)
62-62: Minor: Export ordering inconsistency.The
stac_text_parserexport should be grouped with otherstac_text_*exports (lines 84-86:stac_text_button,stac_text_field,stac_text_form_field) for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stac/lib/src/parsers/widgets/widgets.dart` at line 62, The export for stac_text_parser is out of order; move the line exporting 'stac_text_parser' so it sits with the other stac_text_* exports (the lines exporting stac_text_button, stac_text_field, stac_text_form_field) to keep consistent grouping and ordering of related exports in widgets.dart.
23-24: Minor: Export ordering inconsistency.The
stac_doubleexport appears beforestac_default_*entries, breaking alphabetical ordering. Consider placing it afterstac_divider(line 26) for consistency with the file's alphabetical organization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stac/lib/src/parsers/widgets/widgets.dart` around lines 23 - 24, Reorder the export entries in widgets.dart so the export 'package:stac/src/parsers/widgets/stac_double/stac_double.dart' is moved to follow the 'stac_divider' export, restoring alphabetical order among exports (so stac_divider comes before stac_double and then the stac_default_* exports like stac_default_bottom_navigation_controller_parser remain after).packages/stac/lib/src/parsers/foundation/foundation.dart (1)
62-72: Redundant exports: individual layout parsers and barrel file.Lines 62-71 export individual layout parsers, while line 72 exports
layout/parsers.dartwhich re-exports the same files. This creates unnecessary duplication. Consider removing either:
- The individual exports (lines 62-71), keeping only the barrel export, or
- The barrel export (line 72), keeping the individual exports
♻️ Option 1: Keep only the barrel export
// Layout parsers -export 'layout/stac_axis_parser.dart'; -export 'layout/stac_box_fit_parser.dart'; -export 'layout/stac_box_shape_parser.dart'; -export 'layout/stac_clip_parser.dart'; -export 'layout/stac_flex_fit_parser.dart'; -export 'layout/stac_material_tap_target_size_parser.dart'; -export 'layout/stac_stack_fit_parser.dart'; -export 'layout/stac_vertical_direction_parser.dart'; -export 'layout/stac_wrap_alignment_parser.dart'; -export 'layout/stac_wrap_cross_alignment_parser.dart'; export 'layout/parsers.dart';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stac/lib/src/parsers/foundation/foundation.dart` around lines 62 - 72, The file currently exports both individual layout parsers (stac_axis_parser.dart, stac_box_fit_parser.dart, stac_box_shape_parser.dart, stac_clip_parser.dart, stac_flex_fit_parser.dart, stac_material_tap_target_size_parser.dart, stac_stack_fit_parser.dart, stac_vertical_direction_parser.dart, stac_wrap_alignment_parser.dart, stac_wrap_cross_alignment_parser.dart) and the barrel layout/parsers.dart which re-exports the same files; remove the redundant individual exports and keep only the single barrel export layout/parsers.dart (or alternatively remove the barrel and keep the individual exports) so the module exports are not duplicated—update the export list in foundation.dart accordingly.packages/stac/lib/src/framework/stac_service.dart (1)
223-249: Consider failing closed whenplatformis provided but all entries are invalid.Right now, if every provided platform identifier is invalid, the widget still renders. That can silently bypass intended platform restrictions on typo/misconfig.
🔒 Suggested guard
if (invalid.isNotEmpty) { Log.w( 'Unknown platform identifier(s) in "platform": $invalid. Supported: $supportedPlatformStrings', ); } + + if (validatedPlatforms.isEmpty) { + Log.w('No valid platform identifiers found; widget will not render.'); + return null; + } final isPlatformSupported = validatedPlatforms.contains( currentPlatform, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stac/lib/src/framework/stac_service.dart` around lines 223 - 249, When a caller provides platform but none of the entries are valid, currently the widget still renders; update the guard in the platform handling block (around platform, currentPlatformString(), supportedPlatformStrings, validatedPlatforms, invalid, isPlatformSupported) to fail closed: if platform != null and validatedPlatforms.isEmpty then Log.w that all provided platform identifiers are invalid (include invalid list) and return null so the widget does not render; keep the existing behavior for the case where some validatedPlatforms exist and currentPlatform is not among them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/stac_webview/pubspec.yaml`:
- Around line 22-27: Update the stac_core git dependency to a stable, documented
reference: clarify whether the URL "https://github.com/SuaMusica/stac.git" is an
intentional fork (document this in a short comment next to the stac_core entry)
or change it to the canonical "https://github.com/StacDev/stac" if that was
intended; replace the unstable branch ref "devall" with a pinned commit SHA or a
tagged release (or switch to the published version constraint "stac_core:
^1.3.x" once approved); and add a TODO comment with the tracking issue/PR number
to ensure the temporary git override is addressed before merge (also keep the
note that pubspec_overrides.yaml is used for local development).
In `@packages/stac/lib/src/utils/version/stac_version.dart`:
- Around line 151-157: StacVersion.fromJson currently calls toPlatformJson(json,
'buildNumber').toInt() which can throw if toPlatformJson returns null; update
the buildNumber extraction to safely handle null and non-int types by reading
the platform value as num/String?, converting with a null-aware cast and
toInt(), and providing a sensible fallback (e.g. 0) or throwing a clear
ArgumentError; specifically modify the StacVersion.fromJson factory so
buildNumber is obtained like: read = toPlatformJson(json, 'buildNumber') as num?
or String?, then compute buildNumber = read != null ? (read is num ?
read.toInt() : int.tryParse(read) ?? fallback) : fallback; keep the condition
line using toPlatformJson(json, 'condition') as String? and call
.toStacConditionVersion() as before.
In `@packages/stac/test/src/utils/version/stac_version_test.dart`:
- Around line 11-14: The current setUp uses
StacRegistry.instance.registerBuildNumber(null) which is ignored by
registerBuildNumber; replace this non-resetting pattern by either (A) removing
the global setUp and explicitly calling
StacRegistry.instance.registerBuildNumber(<explicit-value>) in each test that
needs a build number, or (B) add a test-only reset API on StacRegistry (e.g.,
resetForTests() or clearBuildNumber()) and call that from setUp; update tests to
use the explicit registerBuildNumber or the new resetForTests method instead of
passing null so test state is deterministic.
- Around line 220-226: The tests use hardcoded platform-specific keys (e.g.,
'condition_windows', 'buildNumber_ios', 'condition_android') which can match the
host runtime and make tests flaky; update the tests such as the one named "falls
back to generic condition when platform-specific not available" to compute a
non-host platform key at runtime (use dart:io Platform to detect current OS and
choose the opposite suffix or otherwise pick a platform that is guaranteed not
to match the current host) and build the JSON map with that dynamic key; apply
the same fix to the other affected tests around the other cases (the ones
referencing buildNumber_ios and condition_android) so they all construct
platform-specific keys programmatically instead of hardcoding them.
---
Duplicate comments:
In `@packages/stac/pubspec.yaml`:
- Around line 26-31: The git dependency for stac_core currently points to url:
https://github.com/SuaMusica/stac and ref: devall which causes organization
mismatch and non-reproducible builds; update the stac_core dependency entry to
reference the correct repository/org (replace SuaMusica with StacDev in the git
url or point to the canonical repo used by this package), pin the ref to a
specific commit SHA instead of the branch name (replace ref: devall with ref:
<commit-sha>), and add a TODO/issue reference comment noting this is temporary
and must be reverted to a versioned release before publishing.
---
Nitpick comments:
In `@packages/stac/lib/src/framework/stac_service.dart`:
- Around line 223-249: When a caller provides platform but none of the entries
are valid, currently the widget still renders; update the guard in the platform
handling block (around platform, currentPlatformString(),
supportedPlatformStrings, validatedPlatforms, invalid, isPlatformSupported) to
fail closed: if platform != null and validatedPlatforms.isEmpty then Log.w that
all provided platform identifiers are invalid (include invalid list) and return
null so the widget does not render; keep the existing behavior for the case
where some validatedPlatforms exist and currentPlatform is not among them.
In `@packages/stac/lib/src/parsers/foundation/foundation.dart`:
- Around line 62-72: The file currently exports both individual layout parsers
(stac_axis_parser.dart, stac_box_fit_parser.dart, stac_box_shape_parser.dart,
stac_clip_parser.dart, stac_flex_fit_parser.dart,
stac_material_tap_target_size_parser.dart, stac_stack_fit_parser.dart,
stac_vertical_direction_parser.dart, stac_wrap_alignment_parser.dart,
stac_wrap_cross_alignment_parser.dart) and the barrel layout/parsers.dart which
re-exports the same files; remove the redundant individual exports and keep only
the single barrel export layout/parsers.dart (or alternatively remove the barrel
and keep the individual exports) so the module exports are not duplicated—update
the export list in foundation.dart accordingly.
In `@packages/stac/lib/src/parsers/widgets/widgets.dart`:
- Line 62: The export for stac_text_parser is out of order; move the line
exporting 'stac_text_parser' so it sits with the other stac_text_* exports (the
lines exporting stac_text_button, stac_text_field, stac_text_form_field) to keep
consistent grouping and ordering of related exports in widgets.dart.
- Around line 23-24: Reorder the export entries in widgets.dart so the export
'package:stac/src/parsers/widgets/stac_double/stac_double.dart' is moved to
follow the 'stac_divider' export, restoring alphabetical order among exports (so
stac_divider comes before stac_double and then the stac_default_* exports like
stac_default_bottom_navigation_controller_parser remain after).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
packages/stac/lib/src/framework/stac.dartpackages/stac/lib/src/framework/stac_registry.dartpackages/stac/lib/src/framework/stac_service.dartpackages/stac/lib/src/parsers/foundation/foundation.dartpackages/stac/lib/src/parsers/foundation/layout/parsers.dartpackages/stac/lib/src/parsers/widgets/widgets.dartpackages/stac/lib/src/utils/color_utils.dartpackages/stac/lib/src/utils/stac_platforms.dartpackages/stac/lib/src/utils/utils.dartpackages/stac/lib/src/utils/version/stac_version.dartpackages/stac/lib/stac.dartpackages/stac/pubspec.yamlpackages/stac/test/src/utils/version/stac_version_test.dartpackages/stac_core/lib/foundation/text/stac_text_style/stac_text_style.dartpackages/stac_webview/pubspec.yamlpubspec.yaml
💤 Files with no reviewable changes (1)
- pubspec.yaml
Comment on lines
+22
to
+27
| # when approved, use stac_core: ^1.3.x | ||
| stac_core: | ||
| git: | ||
| url: https://github.com/SuaMusica/stac.git | ||
| path: packages/stac_core | ||
| ref: devall |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Check if the git branch 'devall' exists in the referenced repository
echo "=== Checking git branch 'devall' ==="
git ls-remote --heads https://github.com/SuaMusica/stac.git devall
# 2. Get more context about the repository
echo -e "\n=== All branches and tags in the repository ==="
git ls-remote https://github.com/SuaMusica/stac.git | head -20Repository: StacDev/stac
Length of output: 1464
🏁 Script executed:
#!/bin/bash
# 3. Read the full pubspec.yaml to verify the context
echo "=== Full pubspec.yaml content ==="
cat -n packages/stac_webview/pubspec.yamlRepository: StacDev/stac
Length of output: 1141
🏁 Script executed:
#!/bin/bash
# 4. Search for any TODO or issue references related to this dependency change
echo "=== Searching for TODO/FIXME comments related to stac_core or this change ==="
rg -i "todo|fixme|issue|approved|stac_core|devall" packages/stac_webview/ --type yaml --type dart
# 5. Check git history to understand when this was changed
echo -e "\n=== Recent changes to pubspec.yaml ==="
git log --oneline -10 packages/stac_webview/pubspec.yaml 2>/dev/null || echo "Git history not available"Repository: StacDev/stac
Length of output: 799
Git dependency points to different organization with unstable branch reference.
The configuration has several issues that should be resolved:
-
Organization mismatch: The dependency uses
https://github.com/SuaMusica/stac.gitwhile the package's homepage ishttps://github.com/StacDev/stac. This needs clarification—document whether this is intentional (e.g., a fork with specific features) or a configuration error. -
Unstable branch reference: Using branch ref
devallis non-reproducible since branches can be force-pushed. Once approved, replace with a specific commit SHA or tagged release version (e.g.,^1.3.xas noted in the comment). -
Missing issue tracking: The comment "when approved, use stac_core: ^1.3.x" lacks an issue or PR reference. Add a TODO comment with a tracking issue number to ensure this temporary state doesn't remain unresolved.
Note: pubspec_overrides.yaml provides a local path override for development, which is appropriate for local workflows but ensure the main pubspec.yaml is resolved before merging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/stac_webview/pubspec.yaml` around lines 22 - 27, Update the
stac_core git dependency to a stable, documented reference: clarify whether the
URL "https://github.com/SuaMusica/stac.git" is an intentional fork (document
this in a short comment next to the stac_core entry) or change it to the
canonical "https://github.com/StacDev/stac" if that was intended; replace the
unstable branch ref "devall" with a pinned commit SHA or a tagged release (or
switch to the published version constraint "stac_core: ^1.3.x" once approved);
and add a TODO comment with the tracking issue/PR number to ensure the temporary
git override is addressed before merge (also keep the note that
pubspec_overrides.yaml is used for local development).
Comment on lines
+151
to
+157
| factory StacVersion.fromJson(Map<String, dynamic> json) { | ||
| return StacVersion( | ||
| buildNumber: toPlatformJson(json, 'buildNumber').toInt(), | ||
| condition: (toPlatformJson(json, 'condition') as String?) | ||
| .toStacConditionVersion(), | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Potential null dereference in fromJson.
If buildNumber (and platform-specific variant) is missing from the JSON, toPlatformJson returns null, and calling .toInt() on a null value will throw a NoSuchMethodError at runtime.
Consider adding null handling:
🐛 Proposed fix with null safety
factory StacVersion.fromJson(Map<String, dynamic> json) {
+ final buildNumberValue = toPlatformJson(json, 'buildNumber');
+ if (buildNumberValue == null) {
+ throw ArgumentError('buildNumber is required in StacVersion JSON');
+ }
return StacVersion(
- buildNumber: toPlatformJson(json, 'buildNumber').toInt(),
+ buildNumber: (buildNumberValue as num).toInt(),
condition: (toPlatformJson(json, 'condition') as String?)
.toStacConditionVersion(),
);
}Alternatively, if a default build number is acceptable:
♻️ Alternative: provide a default
factory StacVersion.fromJson(Map<String, dynamic> json) {
return StacVersion(
- buildNumber: toPlatformJson(json, 'buildNumber').toInt(),
+ buildNumber: (toPlatformJson(json, 'buildNumber') as num?)?.toInt() ?? 0,
condition: (toPlatformJson(json, 'condition') as String?)
.toStacConditionVersion(),
);
}📝 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.
Suggested change
| factory StacVersion.fromJson(Map<String, dynamic> json) { | |
| return StacVersion( | |
| buildNumber: toPlatformJson(json, 'buildNumber').toInt(), | |
| condition: (toPlatformJson(json, 'condition') as String?) | |
| .toStacConditionVersion(), | |
| ); | |
| } | |
| factory StacVersion.fromJson(Map<String, dynamic> json) { | |
| final buildNumberValue = toPlatformJson(json, 'buildNumber'); | |
| if (buildNumberValue == null) { | |
| throw ArgumentError('buildNumber is required in StacVersion JSON'); | |
| } | |
| return StacVersion( | |
| buildNumber: (buildNumberValue as num).toInt(), | |
| condition: (toPlatformJson(json, 'condition') as String?) | |
| .toStacConditionVersion(), | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/stac/lib/src/utils/version/stac_version.dart` around lines 151 -
157, StacVersion.fromJson currently calls toPlatformJson(json,
'buildNumber').toInt() which can throw if toPlatformJson returns null; update
the buildNumber extraction to safely handle null and non-int types by reading
the platform value as num/String?, converting with a null-aware cast and
toInt(), and providing a sensible fallback (e.g. 0) or throwing a clear
ArgumentError; specifically modify the StacVersion.fromJson factory so
buildNumber is obtained like: read = toPlatformJson(json, 'buildNumber') as num?
or String?, then compute buildNumber = read != null ? (read is num ?
read.toInt() : int.tryParse(read) ?? fallback) : fallback; keep the condition
line using toPlatformJson(json, 'condition') as String? and call
.toStacConditionVersion() as before.
Comment on lines
+11
to
+14
| setUp(() { | ||
| // Reset the app version before each test | ||
| StacRegistry.instance.registerBuildNumber(null); | ||
| }); |
Contributor
There was a problem hiding this comment.
setUp is not actually resetting build-number state.
registerBuildNumber(null) is intentionally guarded, so this setup can leave prior state in place and make tests order-dependent. Prefer setting explicit build numbers only in tests that need them (or introducing a dedicated test-only reset path).
Based on learnings: In packages/stac/lib/src/framework/stac_registry.dart, registerBuildNumber intentionally ignores null and resetting build number is not part of the design.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/stac/test/src/utils/version/stac_version_test.dart` around lines 11
- 14, The current setUp uses StacRegistry.instance.registerBuildNumber(null)
which is ignored by registerBuildNumber; replace this non-resetting pattern by
either (A) removing the global setUp and explicitly calling
StacRegistry.instance.registerBuildNumber(<explicit-value>) in each test that
needs a build number, or (B) add a test-only reset API on StacRegistry (e.g.,
resetForTests() or clearBuildNumber()) and call that from setUp; update tests to
use the explicit registerBuildNumber or the new resetForTests method instead of
passing null so test state is deterministic.
Comment on lines
+220
to
+226
| test('falls back to generic condition when platform-specific not available', | ||
| () { | ||
| final json = { | ||
| 'buildNumber': 1000, | ||
| 'condition': '<', | ||
| 'condition_windows': '!=', // Different platform | ||
| }; |
Contributor
There was a problem hiding this comment.
Platform-negative tests are host-dependent due hardcoded platform keys.
Using fixed keys like condition_windows, buildNumber_ios, and condition_android can accidentally match the current runtime platform and make these tests flaky across CI environments.
💡 Suggested deterministic test update
+ final nonCurrentPlatforms =
+ supportedPlatformStrings.where((p) => p != platform).toList();
final json = {
'buildNumber': 1000,
'condition': '<',
- 'condition_windows': '!=', // Different platform
+ 'condition_${nonCurrentPlatforms.first}': '!=',
};+ final nonCurrentPlatforms =
+ supportedPlatformStrings.where((p) => p != platform).toList();
final json = {
- 'buildNumber_ios':
- 2000, // Only platform-specific for different platform
- 'condition_android':
- '>=', // Only platform-specific for different platform
+ 'buildNumber_${nonCurrentPlatforms.first}': 2000,
+ 'condition_${nonCurrentPlatforms.length > 1 ? nonCurrentPlatforms[1] : nonCurrentPlatforms.first}': '>=',
};Also applies to: 250-256
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/stac/test/src/utils/version/stac_version_test.dart` around lines 220
- 226, The tests use hardcoded platform-specific keys (e.g.,
'condition_windows', 'buildNumber_ios', 'condition_android') which can match the
host runtime and make tests flaky; update the tests such as the one named "falls
back to generic condition when platform-specific not available" to compute a
non-host platform key at runtime (use dart:io Platform to detect current OS and
choose the opposite suffix or otherwise pick a platform that is guaranteed not
to match the current host) and build the JSON map with that dynamic key; apply
the same fix to the other affected tests around the other cases (the ones
referencing buildNumber_ios and condition_android) so they all construct
platform-specific keys programmatically instead of hardcoding them.
- Introduced a utility function to read span content from either text or data keys in JSON. - Updated the StacTextSpan class to accept text from both keys, improving flexibility in JSON handling. - Enhanced documentation to clarify the new behavior for JSON input.update ref
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/stac_core/lib/foundation/text/stac_text_span/stac_text_span.dart (1)
7-9: Consider clarifying the unusedkeyparameter.The
keyparameter is intentionally unused since the function always reads from'text'with a fallback to'data'. This is correct for the use case, but could be clearer. Consider either:
- Adding a brief comment explaining why
keyis ignored, or- Using an underscore prefix (
_key) to signal intentional disuse💡 Optional: clarify unused parameter
/// Used by [StacTextSpan] to read span content from either `"text"` or `"data"`. +/// The [key] parameter is required by [JsonKey.readValue] but unused here +/// since we always check both keys for backwards compatibility. String? _readTextOrData(Map<dynamic, dynamic> map, String key) => map['text'] ?? map['data'];Or alternatively:
-String? _readTextOrData(Map<dynamic, dynamic> map, String key) => +String? _readTextOrData(Map<dynamic, dynamic> map, String _) => map['text'] ?? map['data'];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stac_core/lib/foundation/text/stac_text_span/stac_text_span.dart` around lines 7 - 9, The helper function _readTextOrData currently takes an unused parameter named key which is confusing; update the function signature to mark the parameter as intentionally unused (rename it to _key) or add a brief inline comment above _readTextOrData explaining that the function always reads 'text' with a fallback to 'data' and therefore ignores the passed key, so future readers understand the intent (refer to function _readTextOrData and its parameter key).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/stac_core/lib/foundation/text/stac_text_span/stac_text_span.dart`:
- Around line 7-9: The helper function _readTextOrData currently takes an unused
parameter named key which is confusing; update the function signature to mark
the parameter as intentionally unused (rename it to _key) or add a brief inline
comment above _readTextOrData explaining that the function always reads 'text'
with a fallback to 'data' and therefore ignores the passed key, so future
readers understand the intent (refer to function _readTextOrData and its
parameter key).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/stac_core/lib/foundation/text/stac_text_span/stac_text_span.dartpackages/stac_core/lib/foundation/text/stac_text_span/stac_text_span.g.dart
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
3 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.
This PR introduces version management, platform validation, and several improvements to the STAC framework.
✨ Features
Version Management System
childrenitems ontextwidget. Previously, the text with "children" was expected to include the date, but the latest update changed it to "text." I kept both working to maintain compatibility with older and newer versions.Build Number Support
buildNumberparameter toStac.initialize()StacRegistryPlatform Validation
Custom Color Parsing
parseCustomColorhook inStacRegistryStacService.setParseCustomColor()🔧 Improvements
StacDoubleandStacTextparsersstac_coreto local path dependency for development✅ Tests
StacVersion(277 lines)🧹 Chores
🔄 Migration Notes
If you're using
Stac.initialize(), you can now optionally pass abuildNumber:To use custom color parsing:
Summary by CodeRabbit
New Features
Tests
Chores