Conversation
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 11 minutes and 1 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR adds a compatibility mode selecting either native or GNU Changes
Sequence Diagram(s)sequenceDiagram
participant Main as main()
participant Config as settings::load_startup_config()
participant Env as Environment
participant File as Config File
participant CLI as cli::parse_from_mode()
participant App as app::run_with_flags_and_config()
Main->>Config: load_startup_config()
Config->>Env: check LSP_COMPAT_MODE
alt Env var present
Env-->>Config: compat_mode value
else Env var absent
Config->>File: read config.toml
File-->>Config: compat_mode field
alt Config compat_mode exists
Config->>Config: use from config
else Config compat_mode missing
Config->>Config: default Native
end
end
Config-->>Main: StartupConfig{params, compat_mode}
Main->>CLI: parse_from_mode(compat_mode)
CLI-->>Main: Flags (mode-aware)
Main->>App: run_with_flags_and_config(flags, params)
App->>App: merge flags + params
App-->>Main: Result
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
seapagan
changed the title
seapagan
changed the title
ls compatibility mode
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 81 |
| Duplication | 42 |
TIP This summary will be updated as you push new changes. Give us feedback
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
docs/src/usage.md (1)
114-114: Consider adding a note clarifying the mode context.Line 114 references
-Din native mode context, but with the new compatibility mode documentation above, users might be confused since-Dis reserved in GNU mode. Consider adding a brief clarification.📝 Suggested clarification
-If you add the '-D' option to the command, directories will be sorted first: +If you add the '-D' option to the command (native mode only; use +`--group-directories-first` in gnu mode), directories will be sorted first:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/usage.md` at line 114, Add a brief clarification next to the '-D' example stating that the '-D' option (directories sorted first) applies only in native mode and is reserved/unavailable in GNU (compatibility) mode; reference the new compatibility/GNU mode section so readers aren’t confused about mode-specific option behavior and consider a one-line example or parenthetical note indicating how to enable native mode if needed.docs/src/config.md (1)
126-126: Consider using"native"in the example config.The example configuration sets
compat_mode = "gnu", which may surprise users copying the example verbatim. Sincenativeis the default and most users will start there, consider showing"native"or omitting this line (relying on the default) and adding a comment about switching to"gnu"when needed.📝 Suggested alternative
-compat_mode = "gnu" +# compat_mode = "native" # or "gnu" for GNU ls compatibility🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/config.md` at line 126, Update the example config to avoid surprising users by either setting compat_mode to "native" or removing the compat_mode line and adding a short comment; specifically modify the example line that currently reads compat_mode = "gnu" so it shows compat_mode = "native" (or omit it) and include a one-line note that users can switch to "gnu" if they need GNU compatibility—refer to the compat_mode configuration key in the docs to locate the example to change.tests/integration.rs (1)
702-723: Add explicit rejection tests for the-Iand-Zreserved flags in GNU compatibility mode.The test at lines 702-723 verifies all four reserved flags (
-D,-I,-N,-Z) are omitted from help output, but explicit rejection tests exist only for-D(line 622) and-N(line 642). For consistency and complete coverage, add tests that verify-Iand-Zare rejected when used in GNU mode, similar to the existing tests for-Dand-N.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration.rs` around lines 702 - 723, Add two tests mirroring the existing reserved-flag checks for -D and -N: create test_gnu_mode_rejects_I and test_gnu_mode_rejects_Z that run Command::cargo_bin("lsp") with env("LSP_COMPAT_MODE","gnu") and pass "-I" and "-Z" respectively, assert the command exits non-success and stderr contains the expected reserved-flag rejection message (same pattern used in the existing tests for -D and -N), placing them alongside the other integration tests in tests/integration.rs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/src/config.md`:
- Line 126: Update the example config to avoid surprising users by either
setting compat_mode to "native" or removing the compat_mode line and adding a
short comment; specifically modify the example line that currently reads
compat_mode = "gnu" so it shows compat_mode = "native" (or omit it) and include
a one-line note that users can switch to "gnu" if they need GNU
compatibility—refer to the compat_mode configuration key in the docs to locate
the example to change.
In `@docs/src/usage.md`:
- Line 114: Add a brief clarification next to the '-D' example stating that the
'-D' option (directories sorted first) applies only in native mode and is
reserved/unavailable in GNU (compatibility) mode; reference the new
compatibility/GNU mode section so readers aren’t confused about mode-specific
option behavior and consider a one-line example or parenthetical note indicating
how to enable native mode if needed.
In `@tests/integration.rs`:
- Around line 702-723: Add two tests mirroring the existing reserved-flag checks
for -D and -N: create test_gnu_mode_rejects_I and test_gnu_mode_rejects_Z that
run Command::cargo_bin("lsp") with env("LSP_COMPAT_MODE","gnu") and pass "-I"
and "-Z" respectively, assert the command exits non-success and stderr contains
the expected reserved-flag rejection message (same pattern used in the existing
tests for -D and -N), placing them alongside the other integration tests in
tests/integration.rs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08aef5c4-4c83-4904-ad01-1f1389e4b26c
📒 Files selected for processing (12)
README.mdTODO.mddocs/src/config.mddocs/src/usage.mdsrc/app.rssrc/cli.rssrc/main.rssrc/settings.rssrc/structs.rstests/crate/cli.rstests/crate/settings.rstests/integration.rs
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Signed-off-by: Grant Ramsay <seapagan@gmail.com>
1 participant
Summary
Add a bootstrap GNU compatibility mode without changing native
lsplusbehavior.This introduces startup mode selection from
LSP_COMPAT_MODEorcompat_modein config, reserves the conflicting GNU short flags ingnumode, and keeps the currentlsplus-specific behavior innativemode.What changed
clapbuilder API so shared options stay in one parser path-D,-I,-N, and-Zfor GNU semantics ingnumode and reject them until implemented--group-directories-firstand--indicator-style=slashclaphelp output, with-pand--indicator-style=slashas separate entry points to the same behaviorWhy
This gives
lspa functional GNU-compatible CLI surface for aliasing or scripting, without breaking the existing native command set. It also establishes the parser structure needed to implement the remaining GNU behaviors incrementally.Summary by CodeRabbit
New Features
Documentation
Tests