Skip to content

fix(config): infinite loop on spotify_launch_flags and non-deterministic extension ordering#3847

Open
nghiphaam wants to merge 2 commits into
spicetify:mainfrom
nghiphaam:fix/config-loop-ordering
Open

fix(config): infinite loop on spotify_launch_flags and non-deterministic extension ordering#3847
nghiphaam wants to merge 2 commits into
spicetify:mainfrom
nghiphaam:fix/config-loop-ordering

Conversation

@nghiphaam
Copy link
Copy Markdown
Contributor

@nghiphaam nghiphaam commented May 20, 2026

spotify_launch_flags in EditConfig used continue which skipped args = args[2:], causing an infinite loop when that field is passed. Also sorts the extension list after map iteration to ensure stable ordering.

Summary by CodeRabbit

  • Bug Fixes
    • Resolved non-deterministic ordering in configuration lists, now displaying in consistent order across sessions.
    • Fixed configuration edit flow to ensure all valid setting changes are properly persisted without interruption.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

📝 Walkthrough

Walkthrough

src/cmd/config.go adds the sort import and applies deterministic ordering to config list values. The spotify_launch_flags case in EditConfig no longer skips via continue, allowing the loop to advance normally. List values are sorted before persisting, removing map-iteration nondeterminism.

Changes

Config Determinism Fixes

Layer / File(s) Summary
Sort import, control flow fix, and list sorting
src/cmd/config.go
Import sort package, change spotify_launch_flags case from continue to no-op comment, and sort newList with sort.Strings before joining and persisting array config values for deterministic output.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A sorted config, oh what delight,
No more chaos in the night!
Lists arranged in perfect rows,
Where spotify_launch_flags gracefully flows,
Determinism blooms—a rabbit's delight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the two main issues fixed: an infinite loop bug with spotify_launch_flags and non-deterministic extension ordering, matching the PR's stated objectives.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/cmd/config.go (1)

23-24: 💤 Low value

Correctly fixes the infinite loop.

The removal of continue ensures the loop advances past args = args[2:].

Consider adding a warning message to inform users that this field is not editable via the config command (similar to how toggleType warns on invalid values), but this is optional.

 case "spotify_launch_flags":
-    // not editable via config command
+    unchangeWarning(field, "not editable via config command")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cmd/config.go` around lines 23 - 24, In the switch branch for case
"spotify_launch_flags" (the same branch now falling through to args = args[2:]),
add a user-facing warning message indicating this field is not editable via the
config command (mirroring the behavior of the invalid toggleType warning) before
advancing the args slice so users know why no change is applied; update the code
path that currently just skips editing to call the same logging/printing
mechanism used for toggleType warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/cmd/config.go`:
- Around line 23-24: In the switch branch for case "spotify_launch_flags" (the
same branch now falling through to args = args[2:]), add a user-facing warning
message indicating this field is not editable via the config command (mirroring
the behavior of the invalid toggleType warning) before advancing the args slice
so users know why no change is applied; update the code path that currently just
skips editing to call the same logging/printing mechanism used for toggleType
warnings.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1458ea3d-aec6-4570-95a8-5c6ad728e942

📥 Commits

Reviewing files that changed from the base of the PR and between a1447c8 and 505cf03.

📒 Files selected for processing (1)
  • src/cmd/config.go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant