Skip to content

Conversation

@techouse
Copy link
Owner

@techouse techouse commented Nov 3, 2025

This pull request adds support for omitting null values when encoding lists using the comma-separated format in query strings. It introduces the new commaCompactNulls option, updates the encoding logic to filter out null entries when this option is enabled, and ensures the round-trip marker logic remains correct. The changes are covered by new and updated unit tests.

Feature addition:

  • Added the commaCompactNulls option to EncodeOptions, allowing users to drop null items from lists when using the comma-separated format. [1] [2]

Encoding logic updates:

  • Updated the encoding implementation in encode.dart to filter out null entries from lists when commaCompactNulls is enabled, and to correctly handle the round-trip marker when only non-null items remain. [1] [2]

API propagation:

  • Ensured the new commaCompactNulls option is passed through all relevant constructors, method calls, and internal logic, including copyWith and debug output. [1] [2] [3] [4] [5]

Testing:

  • Added and updated unit tests to verify the correct behavior of commaCompactNulls, including cases with mixed, all-null, and single non-null list entries, as well as integration with the round-trip marker. [1] [2] [3] [4] [5] [6] [7] [8]

@techouse techouse self-assigned this Nov 3, 2025
@techouse techouse added the enhancement New feature or request label Nov 3, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 3, 2025

Walkthrough

A new commaCompactNulls option is introduced to the encoder, allowing null entries to be filtered from comma-formatted lists during encoding. The option is propagated through the encoding pipeline, including the core _encode function and EncodeOptions model, with supporting test coverage added.

Changes

Cohort / File(s) Summary
Core encoding logic
lib/src/extensions/encode.dart
Implements commaCompactNulls parameter in _encode function. Filters null entries before joining comma-separated lists when enabled. Adjusts round-trip marker logic based on filtered list length. Propagates the option through recursive encoding calls.
Options model
lib/src/models/encode_options.dart
Adds commaCompactNulls field (default false) to EncodeOptions. Updates constructor, copyWith method, toString, and equality props.
Encode function
lib/src/qs.dart
Computes and passes commaCompactNulls flag to internal _encode call when encoding with comma list format.
Test coverage
test/unit/encode_test.dart, test/unit/models/encode_options_test.dart
Adds three encode tests exercising null filtering with comma format and round-trip markers. Updates options model tests to cover the new commaCompactNulls field in constructor, copyWith, and serialisation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Logic changes in lib/src/extensions/encode.dart around null filtering and round-trip marker recalculation warrant careful review
  • Verify commaEffectiveLength computation correctly reflects filtered list state for round-trip marker decisions
  • Ensure recursive calls properly propagate commaCompactNulls consistently across nested structures

Possibly related PRs

Suggested labels

test

Poem

🐰 Hop along to compact nulls with flair,
Where commas dance and filter spare,
Round-trip markers guard the way,
Null-free lists shine bright today!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description covers the feature addition, encoding logic updates, API propagation, and testing. However, the template requires a 'Type of change' section and 'How Has This Been Tested?' section which are missing. Add the 'Type of change' checklist (mark 'New feature') and complete the 'How Has This Been Tested?' section with specific test details and reproduction instructions.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately highlights the main feature addition of the commaCompactNulls option to EncodeOptions.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/compact_comma_nulls

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 50202d2 and 63088b6.

📒 Files selected for processing (5)
  • lib/src/extensions/encode.dart (6 hunks)
  • lib/src/models/encode_options.dart (6 hunks)
  • lib/src/qs.dart (1 hunks)
  • test/unit/encode_test.dart (1 hunks)
  • test/unit/models/encode_options_test.dart (7 hunks)
🔇 Additional comments (5)
test/unit/encode_test.dart (1)

5263-5312: Excellent test coverage for the new feature.

The three new tests comprehensively validate the commaCompactNulls behaviour:

  1. Null filtering in mixed lists
  2. Key omission when all entries are null
  3. Round-trip marker preservation after filtering

The test cases cover the critical scenarios and interactions with commaRoundTrip.

lib/src/models/encode_options.dart (1)

51-51: Well-integrated new option.

The commaCompactNulls field is properly wired through the constructor, copyWith, toString, and equality props, following the established pattern. The default value of false ensures backward compatibility.

Also applies to: 121-122, 182-182, 203-203, 226-226, 249-249

lib/src/qs.dart (1)

161-162: Clean integration following established patterns.

The ccn flag computation mirrors the crt pattern, and the parameter is correctly threaded through to the internal encoder. The logic appropriately restricts the flag to comma-format lists.

Also applies to: 170-170

test/unit/models/encode_options_test.dart (1)

26-26: Thorough test updates for the new field.

The commaCompactNulls field is properly tested across all test scenarios, ensuring correct behaviour in constructor, copyWith, and toString operations.

Also applies to: 45-45, 65-65, 83-83, 100-100, 119-119, 139-139

lib/src/extensions/encode.dart (1)

57-57: Robust implementation of null filtering for comma format.

The core encoding logic correctly:

  • Filters null entries when commaCompactNulls is enabled (lines 172–174)
  • Tracks the effective length after filtering (line 176)
  • Uses commaEffectiveLength to determine round-trip marker placement (lines 220–225), ensuring [null, 'foo'] with both flags produces a[]=foo
  • Handles the all-nulls case by marking as Undefined (line 197)
  • Propagates the option through recursive calls (line 301)

The ternary operator at lines 220–225 is somewhat dense but correct: it chooses the filtered length when available, falling back to the original list length otherwise.

Also applies to: 150-150, 167-199, 220-228, 301-301


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codacy-production
Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.19% (target: -1.00%) 90.48%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (b3cd037) 895 878 98.10%
Head commit (63088b6) 911 (+16) 892 (+14) 97.91% (-0.19%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#43) 21 19 90.48%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

@codecov
Copy link

codecov bot commented Nov 3, 2025

Codecov Report

❌ Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.91%. Comparing base (b3cd037) to head (63088b6).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
lib/src/extensions/encode.dart 87.50% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #43      +/-   ##
==========================================
- Coverage   98.10%   97.91%   -0.19%     
==========================================
  Files          14       14              
  Lines         895      910      +15     
==========================================
+ Hits          878      891      +13     
- Misses         17       19       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@techouse techouse merged commit 3c0d55b into main Nov 3, 2025
12 of 14 checks passed
@techouse techouse deleted the feat/compact_comma_nulls branch November 3, 2025 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants