Skip to content

Add asdf provider detection - #27

Merged
akriaueno merged 1 commit into
developfrom
feature/asdf-provider-detection
Dec 20, 2025
Merged

akriaueno merged 1 commit into
developfrom
feature/asdf-provider-detection

Conversation

@akriaueno

@akriaueno akriaueno commented Dec 20, 2025

Copy link
Copy Markdown
Owner

Summary

  • add asdf provider detection rule (shims/installs)
  • ensure rule precedence before system
  • add test for asdf shim precedence

Testing

  • nimble test

Summary by CodeRabbit

  • New Features

    • Added detection support for asdf as a provider option, expanding the system's ability to recognize asdf installations in your environment.
  • Tests

    • Added test case to verify asdf detection works correctly and maintains proper precedence handling.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Dec 20, 2025

Copy link
Copy Markdown

Walkthrough

This change adds asdf provider detection to the default provider rules by introducing pattern matching for asdf-specific paths (.asdf/shims and .asdf/installs), alongside a corresponding test case to verify correct provider precedence detection for asdf installations.

Changes

Cohort / File(s) Summary
Provider detection rule addition
src/why_core.nim
Introduces a new provider detection rule using mkContains to match asdf paths (".asdf/shims" and ".asdf/installs"), inserted after the Mise rule in default provider rules.
Test coverage
tests/test_why_core.nim
Adds a test case validating that asdf shim paths take precedence over system paths when detecting the provider via detectProviderByPath.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Straightforward pattern addition with minimal logic changes
  • Test case is straightforward precedence verification
  • Both changes follow established conventions in the codebase

Possibly related issues

  • Support asdf provider detection #18: Both this PR and the issue add asdf detection to default provider rules using the same approach (pattern matching for asdf-specific paths), directly addressing the asdf provider recognition objective.

Poem

🐰 A shim so fine in .asdf's keep,
Now Nim detects it, swift and deep,
With patterns placed, precedence clear,
The provider's path is drawing near!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Add asdf provider detection' directly and specifically describes the main change: adding asdf provider detection functionality, which is the primary focus of the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/asdf-provider-detection

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
tests/test_why_core.nim (1)

88-95: Test correctly verifies asdf precedence.

The test accurately verifies that asdf shims are detected before system paths, confirming the intended rule precedence.

Optional: Consider adding test coverage for the .asdf/installs pattern

The current test only verifies the .asdf/shims pattern. You could add another test case for the .asdf/installs pattern to ensure comprehensive coverage:

test "asdf installs path is detected as asdf provider":
  let rules = defaultRules("/home/test")
  let provider = detectProviderByPath(
    "/home/test/.asdf/installs/nodejs/20.0.0/bin/node",
    "/home/test/.asdf/installs/nodejs/20.0.0/bin/node",
    rules
  )
  check provider == "asdf"
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 121bc5e and 5339dc3.

📒 Files selected for processing (2)
  • src/why_core.nim (1 hunks)
  • tests/test_why_core.nim (1 hunks)
⏰ 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). (1)
  • GitHub Check: test (macos-latest)
🔇 Additional comments (1)
src/why_core.nim (1)

60-62: LGTM! asdf provider rule correctly implemented.

The asdf provider rule is well-positioned and uses appropriate patterns. The placement after Mise and before Snap ensures proper precedence, particularly before the System provider as intended by the PR objectives.

@akriaueno
akriaueno merged commit 3800853 into develop Dec 20, 2025
5 checks passed
@akriaueno
akriaueno deleted the feature/asdf-provider-detection branch May 30, 2026 13:25
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