Homepage, readme and FAQ updates #138
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe homepage content has been updated to clarify that the product operates on a subscription-based model with usage limits, rather than offering unlimited token access or no token costs. Text changes affect the hero section, benefit descriptions, and FAQ, providing specific details about subscription tiers and message limits. Additional FAQ entries explain the rationale behind the fileWriteLineLimit setting and compatibility notes for using Desktop Commander with other MCP clients. The README was enhanced with clearer installation, update, uninstall instructions, and detailed explanation of the fileWriteLineLimit configuration. Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/index.html (1)
930-930: Consider centralizing subtitle styling.
The inline style on this<p>tag could be moved into a CSS class (e.g..hero-subtitle) for better maintainability and consistency.Apply this diff as an example:
- <p style="color: #f0f0f0; font-weight: 300;">Work with code and text, run processes, and automate tasks using Claude Desktop's subscription model - no per-token API charges.</p> + <p class="hero-subtitle">Work with code and text, run processes, and automate tasks using Claude Desktop's subscription model - no per-token API charges.</p>And in your CSS:
.hero-subtitle { color: #f0f0f0; font-weight: 300; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/index.html(3 hunks)
🔇 Additional comments (3)
docs/index.html (3)
923-923: Hero title update looks good.
The desktop hero title now clearly indicates the product works with Claude Desktop, aligning with the subscription-based messaging.
928-928: Mobile hero title updated.
The mobile title now matches the desktop version, ensuring consistency across devices.
1002-1003: Benefit card text updated correctly.
The change from "Unlimited token access" to "Subscription-based usage" accurately reflects the subscription model and message limits. The description is clear and concise.
| <div class="accordion-item"> | ||
| <div class="accordion-header"> | ||
| <span>Is usage really "unlimited"?</span> | ||
| <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" class="feather feather-chevron-down"><polyline points="6 9 12 15 18 9"></polyline></svg> | ||
| </div> | ||
| <div class="accordion-body"> | ||
| <div class="accordion-body-inner"> | ||
| <p>Desktop Commander uses Claude Desktop's subscription model, which has usage limits that reset 2-3 times per day based on your plan tier. It's "unlimited" in that you don't pay per token like with API access, but you do have message limits. For most development work, Claude Pro's 45 messages per 5-hour window is quite generous - equivalent to 1,800+ messages per month.</p> | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Accessibility and structured data update needed for new FAQ.
The new accordion item is missing the ARIA attributes (role="button", aria-expanded, aria-controls, tabindex, plus unique id on the header and matching id on the body) that other FAQ headers have—this impacts keyboard navigation and screen-reader support. Additionally, the JSON-LD structured data block (lines 1504–1543) wasn’t extended to include this question, so it won’t be indexed by search engines. Please:
- Add matching ARIA markup and unique IDs to the new header and its content container.
- Update the JSON-LD
mainEntityarray to include the "Is usage really 'unlimited'?" question and answer.
🤖 Prompt for AI Agents
In docs/index.html around lines 1588 to 1598, the new accordion FAQ item lacks
necessary ARIA attributes and unique IDs for accessibility and keyboard
navigation. Add role="button", aria-expanded, aria-controls, and tabindex
attributes to the accordion header, and assign unique IDs to both the header and
its corresponding accordion body with matching aria-controls and id references.
Additionally, update the JSON-LD structured data block between lines 1504 and
1543 to include this new FAQ question and answer in the mainEntity array for
proper search engine indexing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
FAQ.md (1)
300-326: Approve fileWriteLineLimit rationale FAQ with minor grammar nitpick.
This section provides clear context and examples. Consider adding a comma before “and” in the sentence “…multiple chunks have succeeded, and that work is not lost” for improved readability.- ... make AI work in smaller chunks so when you hit that limit, multiple chunks have succeeded and that work is not lost - it just needs to restart ... + ... make AI work in smaller chunks so when you hit that limit, multiple chunks have succeeded, and that work is not lost - it just needs to restart ...🧰 Tools
🪛 LanguageTool
[uncategorized] ~325-~325: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ough, the completed chunks are preserved and you only need to restart from where it ...(COMMA_COMPOUND_SENTENCE)
README.md (1)
132-134: Grammar: Add missing comma before “but.”
To improve readability in “No – uses npx but config might not update automatically,” insert a comma before “but”:- **❌ Auto-Updates:** No - uses npx but config might not update automatically + **❌ Auto-Updates:** No - uses npx, but config might not update automatically🧰 Tools
🪛 LanguageTool
[uncategorized] ~132-~132: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ning. ❌ Auto-Updates: No - uses npx but config might not update automatically ...(COMMA_COMPOUND_SENTENCE_2)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
FAQ.md(2 hunks)README.md(5 hunks)docs/index.html(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/index.html
🧰 Additional context used
🪛 LanguageTool
FAQ.md
[uncategorized] ~325-~325: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ough, the completed chunks are preserved and you only need to restart from where it ...
(COMMA_COMPOUND_SENTENCE)
README.md
[uncategorized] ~132-~132: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ning. ❌ Auto-Updates: No - uses npx but config might not update automatically ...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~342-~342: A comma might be missing here.
Context: ...ying here is to make AI work in smaller chunks so when you hit that limit, multiple ch...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
🔇 Additional comments (8)
FAQ.md (1)
258-279: Approve added MCP client compatibility FAQ.
The new section clearly explains compatibility caveats for non-Claude MCP clients and aligns with related documentation.README.md (7)
69-69: Approve callout for update & uninstall guidance.
This prominent note helps users choose the right installation option with clear update and uninstall details upfront.
71-71: Approve Option 1 auto-update labeling.
The ⭐ and bold labels clearly indicate that this installation supports automatic updates.
83-85: Approve Option 1 update & uninstall commands.
Commands for manual updates and uninstallation are correct and consistent.
94-96: Approve Option 2 manual-update details.
The bash installer option clearly communicates manual update and uninstall steps.
106-108: Approve Option 3 Smithery installation details.
Auto-update instructions and the uninstall command look accurate.
151-153: Approve Option 5 local checkout instructions.
Local installation steps, manual update, and uninstall commands are clear and accurate.
155-160: Approve new Updating & Uninstalling section.
The dedicated section consolidates all update and uninstall instructions, improving discoverability and maintainability.
README.md
Outdated
| **🔄 Manual Update:** Run the setup command again | ||
| **🗑️ Uninstall:** Run `npx @wonderwhy-er/desktop-commander@latest setup --uninstall` | ||
|
|
||
| ### Option 2: Using bash script installer (macOS) ❌ **Manual Updates** |
There was a problem hiding this comment.
it's not manual update! This is the same as Option 1, but with nodejs check.
There was a problem hiding this comment.
we need to create same script for windows
| @@ -78,22 +80,34 @@ npx @wonderwhy-er/desktop-commander@latest setup --debug | |||
| ``` | |||
| Restart Claude if running. | |||
|
|
|||
| ### Option 2: Using bash script installer (macOS) | |||
| **✅ Auto-Updates:** Yes - automatically updates when you restart Claude | |||
There was a problem hiding this comment.
Really like this listboxes, need to add them to the landing page
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
README.md (2)
87-87: Consider Windows installer support or notice.
Option 2 is labeled for macOS only. If Windows isn’t supported yet, please call that out, or consider providing an equivalent Windows script.
106-108: Reminder: mention Smithery prerequisites.
Past feedback noted that Smithery installation requires an account and API key. Please consider adding a brief note about this requirement.
🧹 Nitpick comments (2)
README.md (2)
132-134: Clarify manual update phrasing and punctuation.
The phrase “Usually automatic via npx, but if issues occur…” under “🔄 Manual Update” may confuse users (manual vs automatic). Also, add a comma after “npx” for readability.Could be updated as:
- **🔄 Manual Update:** Usually automatic via npx, but if issues occur, update your config file or re-add the entry + **🔄 Manual Update:** Re-run the npx command or update your config file directly if the entry wasn’t applied🧰 Tools
🪛 LanguageTool
[uncategorized] ~132-~132: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ning. ❌ Auto-Updates: No - uses npx but config might not update automatically ...(COMMA_COMPOUND_SENTENCE_2)
336-359: Enhance configuration docs and hierarchy.
The newfileWriteLineLimitsection is very useful. A few optional refinements:
- Consider matching heading levels (use
###instead of####to align with other subsections under Configuration Management).- Add an entry for “Understanding fileWriteLineLimit” to the Table of Contents for discoverability.
- Refine the sentence at line 342 to improve readability by adding a comma.
Suggested diffs within this block:
- #### Understanding fileWriteLineLimit + ### Understanding fileWriteLineLimit - What we're trying here is to make AI work in smaller chunks so when you hit that limit, multiple chunks have succeeded and that work is not lost - it just needs to restart from the last chunk + What we're trying here is to make AI work in smaller chunks, so when you hit that limit, multiple chunks have succeeded and that work is not lost — it just needs to restart from the last chunk🧰 Tools
🪛 LanguageTool
[uncategorized] ~342-~342: A comma might be missing here.
Context: ...ying here is to make AI work in smaller chunks so when you hit that limit, multiple ch...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(5 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
89-89: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🪛 LanguageTool
README.md
[uncategorized] ~132-~132: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ning. ❌ Auto-Updates: No - uses npx but config might not update automatically ...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~342-~342: A comma might be missing here.
Context: ...ying here is to make AI work in smaller chunks so when you hit that limit, multiple ch...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
🔇 Additional comments (10)
README.md (10)
69-69: Clear summary for update & uninstall instructions.
The new callout effectively alerts users to the differences in update mechanics across installation options.
71-71: Option 1 heading consistency is good.
The “Install through npx ⭐ Auto-Updates” heading is clear and matches the icon usage elsewhere.
83-85: Option 1 instructions are accurate.
The auto-update and manual update commands for Option 1 are correct and easy to follow.
98-98: Option 3 heading is clear.
The “Installing via Smithery ⭐ Auto-Updates” title appropriately highlights the auto-update capability.
110-110: Option 4 heading is correct.
The label clearly indicates manual updates for the config-edit method.
136-136: Option 5 heading is fine.
The checkout option clearly signals manual updates via git.
151-153: Option 5 instructions are accurate.
The manual git pull and setup steps are correctly outlined.
157-158: Automatic updates section is consistent.
This summary correctly reiterates that only Options 1 and 3 auto-update.
160-163: Manual updates section is consistent.
The bullet list accurately describes manual update steps for Options 2, 4, and 5.
165-172: Uninstalling section is clear.
All uninstall commands and steps align with the installation instructions above.
| **✅ Auto-Updates:** Yes - requires manual updates | ||
| **🔄 Manual Update:** Re-run the bash installer command above | ||
| **🗑️ Uninstall:** Remove the MCP server entry from your Claude config file and delete the cloned repository if it exists |
There was a problem hiding this comment.
Fix auto-update label inconsistency for Option 2.
The current line reads “✅ Auto-Updates: Yes – requires manual updates”, which contradicts itself. It should indicate no auto-updates.
Apply this diff:
- **✅ Auto-Updates:** Yes - requires manual updates
+ **❌ Auto-Updates:** No - requires manual updates 📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **✅ Auto-Updates:** Yes - requires manual updates | |
| **🔄 Manual Update:** Re-run the bash installer command above | |
| **🗑️ Uninstall:** Remove the MCP server entry from your Claude config file and delete the cloned repository if it exists | |
| **❌ Auto-Updates:** No - requires manual updates | |
| **🔄 Manual Update:** Re-run the bash installer command above | |
| **🗑️ Uninstall:** Remove the MCP server entry from your Claude config file and delete the cloned repository if it exists |
🤖 Prompt for AI Agents
In README.md around lines 94 to 96, the "Auto-Updates" label for Option 2 is
inconsistent, stating "Yes - requires manual updates," which contradicts itself.
Change the label to indicate "No" for auto-updates to accurately reflect that
manual updates are required. Update the text to "**✅ Auto-Updates:** No -
requires manual updates" to fix this inconsistency.
Summary by CodeRabbit
Summary by CodeRabbit
FAQ issues links;
#140
#121
#117
#99
#84