Skip to content

Conversation

@techouse
Copy link
Owner

@techouse techouse commented Nov 3, 2025

This pull request adds support for more flexible handling of null values in comma-separated lists during query string encoding. It introduces a new commaCompactNulls option to the EncodeOptions model, allowing users to drop null entries from lists when using the comma format. The implementation ensures that single-element lists can still round-trip correctly and that the new option is thoroughly tested.

Feature enhancements: Comma-separated list encoding

  • Added the commaCompactNulls option to EncodeOptions, enabling the encoder to drop null entries from lists when using ListFormat.COMMA. This results in more compact query strings and allows omitting keys when all values are stripped. [1] [2] [3] [4] [5]
  • Updated the encoder logic in Encoder.kt to respect the commaCompactNulls flag, filter out null values from lists, and adjust key generation and round-trip marker handling accordingly. [1] [2] [3] [4] [5] [6] [7]

Testing and documentation

  • Added and expanded unit tests in QsParserSpec.kt to cover the new commaCompactNulls behavior, including edge cases like omitting keys and preserving round-trip markers.
  • Updated tests in EncodeOptionsSpec.kt to verify correct copying and builder behavior for the new option. [1] [2] [3] [4] [5]
  • Improved documentation in README.md to describe the new options and their effects on encoding behavior.

Summary by CodeRabbit

  • New Features

    • Added commaCompactNulls option: drops null entries from comma-separated lists before encoding.
    • Added commaRoundTrip option: enables comma-separated list round-tripping support.
  • Documentation

    • Updated README with clarification on comma format options and encoding behavior.
  • Tests

    • Added test coverage for comma-compacting and round-trip encoding scenarios.

@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 encoding option is introduced to handle null values in comma-delimited lists. When enabled, null entries are filtered from lists before joining with commas, allowing compact representation of comma-separated values.

Changes

Cohort / File(s) Summary
Documentation
README.md
Added clarification notes for ListFormat.COMMA behavior, documenting commaRoundTrip and commaCompactNulls options in the Encoding section.
New Configuration Option
qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/models/EncodeOptions.kt
Added commaCompactNulls: Boolean property to EncodeOptions data class with default false. Updated builder with field, setter method, and propagation in build().
Option Wiring
qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/qs.kt
Integrated commaCompactNulls parameter into encoder call, computing its value based on list format generator and options.
Core Implementation
qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/internal/Encoder.kt
Enhanced encode(...) signature with commaCompactNulls: Boolean parameter. Implemented logic to materialize list items, filter nulls when compact mode is enabled, and adjust join behavior with effectiveCommaLength calculation.
Test Coverage
qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/QsParserSpec.kt
Added three test cases: null-entry filtering with commaCompactNulls, key omission when all values are null, and interaction with commaRoundTrip.
Options Testing
qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/EncodeOptionsSpec.kt
Updated tests to verify commaCompactNulls propagation through constructors and copy() method in EncodeOptions.

Sequence Diagram

sequenceDiagram
    participant caller as Caller
    participant qs as qs.kt
    participant encoder as Encoder
    participant list as List Processing
    
    caller->>qs: encode(data, options{commaCompactNulls=true})
    qs->>qs: Check if listFormat == COMMA
    qs->>encoder: encode(..., commaCompactNulls=true)
    
    encoder->>encoder: isCommaGenerator = true
    encoder->>list: Materialize list items
    
    alt commaCompactNulls enabled
        list->>list: Filter null entries
        list->>encoder: Filtered items
    else
        list->>encoder: All items (including nulls)
    end
    
    encoder->>encoder: Join with commas
    
    alt joined result is non-empty
        encoder->>encoder: Emit single "value" entry
    else
        encoder->>encoder: Emit Undefined
    end
    
    encoder-->>qs: Encoded result
    qs-->>caller: Return encoded string
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Encoder.kt logic: Review the materialization, null-filtering, and effectiveCommaLength calculation logic to ensure correctness in edge cases (single-element lists, all-null lists).
  • Option propagation: Verify commaCompactNulls is consistently passed through recursive encode() calls and builder chains.
  • Test coverage: Examine the three new test cases in QsParserSpec for adequate coverage of null-compaction scenarios with and without round-trip markers.

Poem

🐰 Commas dance through encoded text,
Nulls fade away, when we compact.
Round-trip markers guard the rest,
Options joined, encoding blessed! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description provides comprehensive context about the feature, implementation details, and testing, but lacks explicit issue reference and test procedure checklist items required by the template. Add 'Fixes #(issue)' reference, explicitly check test configuration boxes, and detail specific test procedures in the 'How Has This Been Tested?' section.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title ':sparkles: add EncodeOptions.commaCompactNulls' clearly describes the main feature being added and matches the primary change across all modified files.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/add-comma-compact-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 6ffd2cb and 465efe6.

📒 Files selected for processing (6)
  • README.md (1 hunks)
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/internal/Encoder.kt (6 hunks)
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/models/EncodeOptions.kt (4 hunks)
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/qs.kt (1 hunks)
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/QsParserSpec.kt (1 hunks)
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/EncodeOptionsSpec.kt (5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
README.md

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Update README sections when option semantics or defaults shift

Files:

  • README.md
qs-kotlin/src/main/kotlin/**/*.kt

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

qs-kotlin/src/main/kotlin/**/*.kt: Implement all encode/decode logic in the core module (package io.github.techouse.qskotlin) and prefer adding functionality only in qs-kotlin
Preserve key option semantics in QS.encode/QS.decode: depth limit (default 5), parameterLimit, list formats (indices/brackets/repeat/comma), duplicates (COMBINE/FIRST/LAST), allowDots with encodeDotInKeys/decodeDotInKeys, charset + sentinel, Undefined omission, filters, custom encoder/dateSerializer
Mixed list/map inputs or large numeric indices should coerce to maps in encode/decode
Maintain Undefined omission semantics: only omit keys whose value is exactly Undefined() / Undefined.INSTANCE
Keep public API names stable; do not remove or rename QS, EncodeOptions, or DecodeOptions without a deprecation path
Do not change default option values silently (depth=5, parameterLimit, listFormat, duplicates behavior)

Files:

  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/models/EncodeOptions.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/qs.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/internal/Encoder.kt
**/*.kt

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Kotlin code must be formatted by ktfmt (run ./gradlew ktfmtFormat or ktfmtCheck)

Files:

  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/models/EncodeOptions.kt
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/QsParserSpec.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/qs.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/internal/Encoder.kt
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/EncodeOptionsSpec.kt
qs-kotlin/src/test/kotlin/**/*Spec.kt

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Place new unit tests under qs-kotlin/src/test/kotlin and name them *Spec.kt (e.g., DecodeSpec, EncodeSpec)

Files:

  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/QsParserSpec.kt
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/EncodeOptionsSpec.kt
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: techouse/qs-kotlin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-08T06:50:18.291Z
Learning: Applies to qs-kotlin/src/main/kotlin/**/*.kt : Preserve key option semantics in QS.encode/QS.decode: depth limit (default 5), parameterLimit, list formats (indices/brackets/repeat/comma), duplicates (COMBINE/FIRST/LAST), allowDots with encodeDotInKeys/decodeDotInKeys, charset + sentinel, Undefined omission, filters, custom encoder/dateSerializer
Learnt from: CR
Repo: techouse/qs-kotlin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-08T06:50:18.291Z
Learning: Applies to qs-kotlin/src/main/kotlin/**/*.kt : Implement all encode/decode logic in the core module (package io.github.techouse.qskotlin) and prefer adding functionality only in qs-kotlin
Learnt from: CR
Repo: techouse/qs-kotlin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-08T06:50:18.291Z
Learning: Applies to qs-kotlin/src/main/kotlin/**/*.kt : Keep public API names stable; do not remove or rename QS, EncodeOptions, or DecodeOptions without a deprecation path
Learnt from: techouse
Repo: techouse/qs-kotlin PR: 39
File: qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/EncodeSpec.kt:2155-2158
Timestamp: 2025-10-11T11:03:27.899Z
Learning: In qs-kotlin/src/test/kotlin/**/*Spec.kt: When encodeDotInKeys is true, dots in top-level keys with scalar values are NOT encoded (e.g., {"a.b": "v"} → "a.b=v"). The encodeDotInKeys option only encodes dots when there are nested structures (e.g., {"name.obj": {"first": "John"}} → "name%252Eobj.first=John").
Learnt from: CR
Repo: techouse/qs-kotlin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-08T06:50:18.291Z
Learning: Applies to qs-kotlin/src/main/kotlin/**/*.kt : Do not change default option values silently (depth=5, parameterLimit, listFormat, duplicates behavior)
Learnt from: CR
Repo: techouse/qs-kotlin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-08T06:50:18.291Z
Learning: Follow commit/PR conventions: emoji + present-tense summary and include test output and parity diff notes if behavior changed
📚 Learning: 2025-10-08T06:50:18.291Z
Learnt from: CR
Repo: techouse/qs-kotlin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-08T06:50:18.291Z
Learning: Follow commit/PR conventions: emoji + present-tense summary and include test output and parity diff notes if behavior changed

Applied to files:

  • README.md
📚 Learning: 2025-10-08T06:50:18.291Z
Learnt from: CR
Repo: techouse/qs-kotlin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-08T06:50:18.291Z
Learning: Applies to README.md : Update README sections when option semantics or defaults shift

Applied to files:

  • README.md
📚 Learning: 2025-10-08T06:50:18.291Z
Learnt from: CR
Repo: techouse/qs-kotlin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-08T06:50:18.291Z
Learning: Applies to qs-kotlin/src/main/kotlin/**/*.kt : Preserve key option semantics in QS.encode/QS.decode: depth limit (default 5), parameterLimit, list formats (indices/brackets/repeat/comma), duplicates (COMBINE/FIRST/LAST), allowDots with encodeDotInKeys/decodeDotInKeys, charset + sentinel, Undefined omission, filters, custom encoder/dateSerializer

Applied to files:

  • README.md
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/models/EncodeOptions.kt
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/QsParserSpec.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/qs.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/internal/Encoder.kt
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/EncodeOptionsSpec.kt
📚 Learning: 2025-10-11T11:03:27.899Z
Learnt from: techouse
Repo: techouse/qs-kotlin PR: 39
File: qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/EncodeSpec.kt:2155-2158
Timestamp: 2025-10-11T11:03:27.899Z
Learning: In qs-kotlin/src/test/kotlin/**/*Spec.kt: When encodeDotInKeys is true, dots in top-level keys with scalar values are NOT encoded (e.g., {"a.b": "v"} → "a.b=v"). The encodeDotInKeys option only encodes dots when there are nested structures (e.g., {"name.obj": {"first": "John"}} → "name%252Eobj.first=John").

Applied to files:

  • README.md
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/models/EncodeOptions.kt
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/QsParserSpec.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/qs.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/internal/Encoder.kt
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/EncodeOptionsSpec.kt
📚 Learning: 2025-10-08T06:50:18.291Z
Learnt from: CR
Repo: techouse/qs-kotlin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-08T06:50:18.291Z
Learning: Applies to qs-kotlin/src/main/kotlin/**/*.kt : Mixed list/map inputs or large numeric indices should coerce to maps in encode/decode

Applied to files:

  • README.md
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/QsParserSpec.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/qs.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/internal/Encoder.kt
📚 Learning: 2025-10-08T06:50:18.291Z
Learnt from: CR
Repo: techouse/qs-kotlin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-08T06:50:18.291Z
Learning: Applies to qs-kotlin/src/main/kotlin/**/*.kt : Keep public API names stable; do not remove or rename QS, EncodeOptions, or DecodeOptions without a deprecation path

Applied to files:

  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/models/EncodeOptions.kt
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/QsParserSpec.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/qs.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/internal/Encoder.kt
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/EncodeOptionsSpec.kt
📚 Learning: 2025-10-08T06:50:18.291Z
Learnt from: CR
Repo: techouse/qs-kotlin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-08T06:50:18.291Z
Learning: Applies to qs-kotlin/src/main/kotlin/**/*.kt : Do not change default option values silently (depth=5, parameterLimit, listFormat, duplicates behavior)

Applied to files:

  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/models/EncodeOptions.kt
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/QsParserSpec.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/qs.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/internal/Encoder.kt
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/EncodeOptionsSpec.kt
📚 Learning: 2025-10-08T06:50:18.291Z
Learnt from: CR
Repo: techouse/qs-kotlin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-08T06:50:18.291Z
Learning: Applies to qs-kotlin/src/main/kotlin/**/*.kt : Implement all encode/decode logic in the core module (package io.github.techouse.qskotlin) and prefer adding functionality only in qs-kotlin

Applied to files:

  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/models/EncodeOptions.kt
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/QsParserSpec.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/qs.kt
  • qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/internal/Encoder.kt
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/EncodeOptionsSpec.kt
📚 Learning: 2025-10-08T06:50:18.291Z
Learnt from: CR
Repo: techouse/qs-kotlin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-08T06:50:18.291Z
Learning: Applies to qs-kotlin/src/test/kotlin/**/*Spec.kt : Place new unit tests under qs-kotlin/src/test/kotlin and name them *Spec.kt (e.g., DecodeSpec, EncodeSpec)

Applied to files:

  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/QsParserSpec.kt
  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/EncodeOptionsSpec.kt
📚 Learning: 2025-10-08T06:50:18.291Z
Learnt from: CR
Repo: techouse/qs-kotlin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-08T06:50:18.291Z
Learning: When changing behavior, update/extend focused specs mirroring the upstream qs case and update README and Dokka (./gradlew docs) if semantics or defaults change

Applied to files:

  • qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/QsParserSpec.kt
🧬 Code graph analysis (2)
qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/QsParserSpec.kt (2)
qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/internal/Encoder.kt (1)
  • encode (44-311)
qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/qs.kt (1)
  • encode (83-208)
qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/internal/Encoder.kt (1)
qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/models/EncodeOptions.kt (3)
  • encoder (205-343)
  • encoder (233-233)
  • encoder (241-243)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Android AAR (AGP) + unit tests
  • GitHub Check: JVM tests (Java 17)
  • GitHub Check: Ensure compatibility with qs
  • GitHub Check: JVM tests (Java 21)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (13)
qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/models/EncodeOptions.kt (2)

135-139: LGTM! Clean addition of the new option.

The commaCompactNulls property is correctly added with a sensible default of false, ensuring no breaking changes. The documentation clearly explains its purpose.


229-229: Builder implementation is correct.

The builder pattern is properly implemented:

  • Private field initialized to false
  • Fluent setter method returns this for chaining
  • Property is correctly passed to the EncodeOptions constructor in build()

Also applies to: 310-311, 340-340

qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/qs.kt (1)

157-159: Correct integration of commaCompactNulls.

The logic properly computes commaCompactNulls as true only when both conditions are met:

  1. The list format generator is COMMA
  2. The option is explicitly enabled

This mirrors the pattern used for commaRoundTrip (lines 154-156) and ensures the feature only activates for the intended format.

README.md (1)

798-802: Documentation is clear and helpful.

The note effectively explains both commaRoundTrip and commaCompactNulls options with a concrete example showing how nulls are dropped when commaCompactNulls is enabled.

qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/models/EncodeOptionsSpec.kt (2)

17-55: Test coverage for copy operations is thorough.

The test correctly verifies that commaCompactNulls:

  • Is preserved in copy operations with no modifications (line 34, 53)
  • Has the expected value when accessed via the getter (line 53)
  • Maintains equality when all properties match (line 54)

57-112: Copy with modifications test is complete.

The test properly exercises:

  • Setting commaCompactNulls = true in the original options (line 74)
  • Overriding to commaCompactNulls = false in the copy (line 93)
  • Asserting the modified value (line 111)
qs-kotlin/src/test/kotlin/io/github/techouse/qskotlin/unit/QsParserSpec.kt (3)

951-965: Excellent test for null dropping behavior.

This test clearly demonstrates the core functionality:

  • Without commaCompactNulls: nulls become empty slots (,)
  • With commaCompactNulls: nulls are removed entirely from the output

The nested structure (a.b) also confirms the option works correctly for nested lists.


967-981: Critical edge case properly tested.

This test verifies the important behavior that when all values are null and commaCompactNulls is enabled, the key is omitted entirely (empty string result). This prevents generating malformed query strings like a=.


983-1004: Round-trip interaction test is well-designed.

This test demonstrates the correct interaction between commaRoundTrip and commaCompactNulls:

  • After compacting nulls, the list has one element
  • commaRoundTrip correctly adds [] suffix for single-element lists
  • This ensures the value can round-trip through decoding

The test confirms that effectiveCommaLength (post-compaction) is used for round-trip logic, not the original length.

qs-kotlin/src/main/kotlin/io/github/techouse/qskotlin/internal/Encoder.kt (4)

70-72: Clean refactoring with correct null-compaction logic.

The three derived flags are well-structured:

  • isCommaGenerator: Extracts the format check for reusability
  • commaRoundTrip: Sensibly defaults to true for comma format (preserving compatibility with line 71's original intent)
  • compactNulls: Only activates when both comma format AND the option are enabled

This ensures the feature is scoped correctly and won't affect other list formats.


150-173: Null-compaction implementation is correct.

The logic properly handles all scenarios:

  1. Materialize once (line 152): Converts iterable to list for reuse
  2. Conditional filtering (line 153): filterNotNull() only when compactNulls is enabled
  3. Track effective length (line 155): Used later for round-trip logic
  4. Build join source (lines 157-164): Applies encoder if needed, converts nulls to empty strings when not compacted
  5. Handle empty case (line 171): Returns Undefined when all values filtered out, causing key omission

The el?.toString() ?: "" pattern on line 163 correctly handles nulls when compactNulls is false, converting them to empty strings that appear as empty slots in comma-separated output (e.g., a,,b).


198-208: Round-trip logic correctly uses effectiveCommaLength.

The conditional logic for appending [] now considers:

  • When comma generator: uses effectiveCommaLength (post-compaction size)
  • When other generators: uses obj.count() (original size)

This ensures that a list like [null, "foo"] with commaCompactNulls enabled:

  • Has effectiveCommaLength = 1 after filtering
  • Triggers the [] suffix when commaRoundTrip is true
  • Produces a[]=foo for proper round-tripping

280-280: Recursive propagation ensures consistency.

The commaCompactNulls parameter is correctly passed to nested encode() calls, ensuring that deeply nested structures with comma-delimited lists maintain consistent null-compaction behavior throughout the encoding process.


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.

@codecov
Copy link

codecov bot commented Nov 3, 2025

Codecov Report

❌ Patch coverage is 70.37037% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.05%. Comparing base (6ffd2cb) to head (465efe6).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...in/io/github/techouse/qskotlin/internal/Encoder.kt 70.83% 1 Missing and 6 partials ⚠️
...o/github/techouse/qskotlin/models/EncodeOptions.kt 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main      #49      +/-   ##
============================================
- Coverage     95.39%   95.05%   -0.34%     
- Complexity      441      444       +3     
============================================
  Files            15       15              
  Lines          1237     1255      +18     
  Branches        265      272       +7     
============================================
+ Hits           1180     1193      +13     
- Misses            9       11       +2     
- Partials         48       51       +3     
Flag Coverage Δ
java-17 95.05% <70.37%> (-0.34%) ⬇️
java-21 95.05% <70.37%> (-0.34%) ⬇️
jvm 95.05% <70.37%> (-0.34%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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 1107d04 into main Nov 3, 2025
11 of 14 checks passed
@techouse techouse deleted the feat/add-comma-compact-nulls branch November 3, 2025 22:58
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