fix: pass inputs as uninterpretted strings#324
Conversation
Cpp-Linter Report
|
WalkthroughUpdates Changes
Sequence Diagram(s)sequenceDiagram
participant GHA as GitHub Actions Runner
participant UV as uv (Nu)
participant LINT as cpp-linter
GHA->>UV: sync --project <action_path> --group action --no-dev
Note right of UV #f8f0e6: install deps without dev packages
GHA->>UV: run cpp-linter [ 'style=...', 'extensions=...', 'tidy-checks=...', ... ]
Note right of GHA #eef6f2: args normalized as single-string tokens
UV->>LINT: spawn cpp-linter with provided args
LINT-->>GHA: produce analysis results, annotations, summaries
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (10)
✨ Finishing Touches🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
action.yml (2)
396-396: Align --extensions with the “single token” pattern.Not critical for commas, but this prevents any stray whitespace/newline surprises and matches the PR’s goal.
- --extensions '${{ inputs.extensions }}' + '--extensions=${{ inputs.extensions }}'
395-415: Avoid embedding GitHub-rendered values inside single-quoted literals; prefer Nu interpolation to dodge quote-escape pitfalls.If a user ever supplies a single quote character, it breaks the Nu literal. Build tokens via variables and Nu’s $"--flag=($val)" interpolation so Nu handles quoting.
Example pattern (illustrative; feel free to apply selectively to the “equals” args):
- let args = [ - '--style=${{ inputs.style }}' - '--tidy-checks=${{ inputs.tidy-checks }}' - '--ignore=${{ inputs.ignore }}' - '--ignore-tidy=${{ inputs.ignore-tidy }}' - '--ignore-format=${{ inputs.ignore-format }}' - '--extra-arg=${{ inputs.extra-args }}' - ] + let style = '${{ inputs.style }}' + let tidy_checks = '${{ inputs.tidy-checks }}' + let ignore = '${{ inputs.ignore }}' + let ignore_tidy = '${{ inputs.ignore-tidy }}' + let ignore_format = '${{ inputs.ignore-format }}' + let extra_args = '${{ inputs.extra-args }}' + let args = [ + $"--style=($style)" + $"--tidy-checks=($tidy_checks)" + $"--ignore=($ignore)" + $"--ignore-tidy=($ignore_tidy)" + $"--ignore-format=($ignore_format)" + $"--extra-arg=($extra_args)" + ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
action.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: 2bndy5
PR: cpp-linter/cpp-linter-action#314
File: action.yml:394-416
Timestamp: 2025-08-17T12:34:25.463Z
Learning: The cpp-linter/cpp-linter-action repository manages the downstream parsing of command-line arguments through their own cpp-linter/cpp-linter project, so passing empty string values for flags like --extra-arg="" is acceptable and known to work in their implementation.
⏰ 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). (18)
- GitHub Check: test (windows-latest, 14)
- GitHub Check: test (windows-latest, 18)
- GitHub Check: test (windows-latest, 12)
- GitHub Check: test (windows-latest, 17)
- GitHub Check: test (windows-latest, 10)
- GitHub Check: test (windows-latest, 16)
- GitHub Check: test (macos-latest, 16)
- GitHub Check: test (windows-latest, 8)
- GitHub Check: test (ubuntu-latest, 20)
- GitHub Check: test (ubuntu-latest, 19)
- GitHub Check: test (ubuntu-latest, 17)
- GitHub Check: test (windows-latest, 16)
- GitHub Check: test (windows-latest, 11)
- GitHub Check: test (windows-latest, 20)
- GitHub Check: test (macos-latest, 20)
- GitHub Check: test (macos-latest, 16)
- GitHub Check: test (macos-latest, 12)
- GitHub Check: test (macos-latest, 11)
🔇 Additional comments (2)
action.yml (2)
372-376: Good call adding --no-dev to uv sync.Cuts install time and avoids pulling dev deps into the runner.
411-411: Nice: single-token --extra-arg form matches Nu parsing and cpp-linter’s internal splitting.This resolves the spaces/leading-hyphen issues from the linked bugs.
BTW, this makes nushell syntax feel more like a programming language in which each specified arg (passed to an external binary executable) is not reinterpreted by the shell. 👍🏼 |
|
I've tested this pretty diligently. I'm confident enough to merge and release ASAP. |
This comment has been minimized.
This comment has been minimized.
2 participants
resolves #321
resolves #323
In nushell, quotes are not required for strings. Quotes were previously used around
inputs.*values if they might-This resulted in unnecessary quotes around the arg value passed to cpp-linter (
--ignore="build") which was causing the undesirable behavior in both #323 and #323.After testing this in the test repo, it looks like the best way is
Nushell will now pass the entire string (
'--extra-arg=${{ inputs.extra-arg }}') as 1 arg to cpp-linter. Any space(s) are included as part of the arg value (as intended). For example:--extra-arg=-std=c++14 -Wallinvokesclang-tidy --extra-arg=-std=c++14 --extra-arg=-WallNote
In the cpp-linter source, the
extra-argvalue is stripped of surrounding quotes because bash and pwsh have very different behavior in each.This pattern also supports values that might begin with
-or be an empty string. For example:--style=disables clang-format (as intended)--tidy-checks=-*disables clang-tidy (as intended)--tidy-checks=uses .clang-tidy file (as instended) if presentFor the
ignore*inputs, the quotes were causing paths to not match the discovered paths/files in a user's project. In cpp-linter source, there is some stripping of space(s), but there is no stripping of quotes. This is intended to allow users to specify patterns like so:which is behaviorally equivalent to
Summary by CodeRabbit
Bug Fixes
Chores