Skip to content
This repository was archived by the owner on Dec 4, 2025. It is now read-only.

Conversation

@neubaner
Copy link
Collaborator

@neubaner neubaner commented Nov 21, 2025

Hide and make the buffer readonly when we ensure the virtual document is open. This is mosty to prevent the html virtual document to be visible when the html lsp is not available, as rzls works fine without it.

Summary by CodeRabbit

  • Refactor
    • Optimized the timing of buffer initialization for virtual document handling, moving option configuration to an earlier stage in the setup process.

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

Hide and make the buffer readonly when we ensure the virtual document is
open. This is mosty to prevent the html virtual document to be visible
when the html lsp is not available, as rzls works fine without it.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 21, 2025

Walkthrough

Buffer option configuration for C# and HTML virtual documents has been relocated from LSP attach autocmd to the buffer initialization phase, applying buftype=nowrite and buflisted=false earlier during buffer opening.

Changes

Cohort / File(s) Summary
Buffer option initialization refactor
lua/rzls/documentstore.lua
Moved buftype and buflisted option setup for C# and HTML virtual buffers from LspAttach autocmd to ensure_open during buffer initialization; removed the autocmd-based approach

Sequence Diagram(s)

sequenceDiagram
    participant caller as Caller
    participant ensure as ensure_open
    participant lsp as LSP
    
    rect rgb(220, 240, 230)
    Note over ensure,lsp: NEW: Options set during initialization
    caller->>ensure: register_vbufs_by_path()
    ensure->>ensure: Open buffer
    ensure->>ensure: Set buftype=nowrite<br/>buflisted=false
    end
    
    ensure-->>lsp: LSP attach event
    Note over lsp: Autocmd removed<br/>(no longer needed)
    
    rect rgb(240, 220, 220)
    Note over ensure,lsp: OLD: Options set on LSP attach
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Single file with focused behavioral change
  • Option initialization timing shift is straightforward to verify
  • Confirm that removed autocmd logic is completely replicated in the new initialization path
  • Validate no edge cases where LSP attach timing dependency existed

Poem

🐰 A rabbit hops through code with glee,
Options set where they should be,
No autocmd dance delay,
Buffers born the proper way,
Timing's right—the config's clean!

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 title accurately summarizes the main change: hiding virtual documents when their LSP is not installed, which aligns with the code changes and PR objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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.

@neubaner neubaner requested a review from tris203 November 21, 2025 07:00
Copy link
Contributor

@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.

Actionable comments posted: 0

🧹 Nitpick comments (1)
lua/rzls/documentstore.lua (1)

66-75: C# virtual buffer nowrite/unlisted config looks correct and aligns with the PR goal

Moving the buftype=nowrite and buflisted=false settings into register_vbufs_by_path for the C# URI (and scoping them with { buf = buf }) ensures the Razor C# virtual buffer stays hidden/unlisted even when no C#/Roslyn LSP attach event runs. This matches the intent of keeping these internal buffers out of the normal buffer list and non-writable to disk.

If you want to trim duplication with the HTML block below, you could factor these three option calls into a small helper like configure_virtual_vbuf(buf, ft) and reuse it for both C# and HTML.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bbfe0f5 and 803a01b.

📒 Files selected for processing (1)
  • lua/rzls/documentstore.lua (2 hunks)
🔇 Additional comments (1)
lua/rzls/documentstore.lua (1)

90-99: HTML virtual buffer configuration mirrors C# and should prevent visibility without HTML LSP

This block mirrors the C# handling: setting filetype=html, buftype=nowrite, and buflisted=false on the HTML virtual buffer at ensure_open time makes it an internal, unlisted buffer regardless of whether an HTML LSP is installed or attaches. That should address the issue of the HTML virtual document becoming visible when the HTML LSP isn’t present, while keeping behavior consistent with the C# virtual document.

If you introduce a helper for the C# block (e.g., configure_virtual_vbuf(buf, "cs"|"html")), you can reuse it here to keep these paths in lockstep.

@tris203
Copy link
Owner

tris203 commented Nov 21, 2025

IIRC, it used to be like this. But it creates a bit of a race condition, as I think the LSP client wont attach to unlisted buffers. So we need the client to be attached before we unlist.

That might have changed since in which case this is much better

@neubaner
Copy link
Collaborator Author

Yeah, I imagined it that would be the reason, but I could not break it in any way. But race conditions are just like that :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants