Conversation
- remove requirement for hardcoded index.md file - optionally limit the header level reported `auto_toc: 2` will report only level 1 and level 2 headers in the files - scan and report folder contents (not recursively yet).
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR restructures and expands the module development documentation with new sections for connection module basics and advanced topics, reorganizes category structure from generated-index to doc-type links, and enhances the auto-TOC plugin to support hierarchical subdirectory listings. Changes
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (22)
for-developers/module-development/connection-basics/presets.md-8-10 (1)
8-10:⚠️ Potential issue | 🟡 MinorSmall typo: missing word "need" 🙂
Line 10 reads "you will to call" — looks like "need" slipped away!
Suggested fix
-In order to add presets to a module, you will to call another function in your module. The process is similar to how you define actions and feedbacks. +To add presets to a module, you will need to call another function in your module. The process is similar to how you define actions and feedbacks.for-developers/module-development/connection-advanced/setting-custom-variables.md-1-15 (1)
1-15:⚠️ Potential issue | 🟡 MinorMissing YAML front matter — this might cause unexpected behavior in Docusaurus
Every other doc in this PR includes front matter (
title,sidebar_label,sidebar_position,description). Without it, Docusaurus will try to infer the title from the first content line (the blockquote warning), which probably isn't what you want. Would you mind adding front matter here too? Something like:Suggested addition
+--- +title: Setting Custom Variables +sidebar_label: Custom Variables +description: How to output data to user-owned custom variables from an action. +--- + > This is an experimental idea, that may be removed without noticefor-developers/module-development/connection-basics/upgrade-scripts.md-9-19 (1)
9-19:⚠️ Potential issue | 🟡 MinorA few small text and lint nits throughout the file 📝
A handful of minor things the linter caught, plus a grammar one:
- Line 9: "a on/off" → "an on/off"
- Line 13: "practise" → "practice" (more standard in tech documentation)
- Line 19: The fenced code block is missing a language identifier (e.g.,
```js). Flagged by markdownlint (MD040).Suggested fixes
-A common example is changing a on/off option to be an on/off/toggle option. +A common example is changing an on/off option to be an on/off/toggle option.-Each upgrade script will only get run once for each action and feedback, but it is good practise to write the scripts so that they can be executed multiple times. This will help you when testing your script, or if jumping between versions of companion. +Each upgrade script will only get run once for each action and feedback, but it is good practice to write the scripts so that they can be executed multiple times. This will help you when testing your script, or if jumping between versions of companion.-``` +```js const upgradeScripts = require('./upgrades')for-developers/module-development/connection-basics/upgrade-scripts.md-83-85 (1)
83-85:⚠️ Potential issue | 🟡 MinorSmall grammar nit on the last line 🙂
"Anything changes" → "Any changes"
Suggested fix
##### return result -This tells Companion what had changed in your script. Anything changes made to actions or feedbacks not listed in the result object will be discarded. +This tells Companion what has changed in your script. Any changes made to actions or feedbacks not listed in the result object will be discarded.for-developers/module-development/connection-basics/user-configuration.md-5-5 (1)
5-5:⚠️ Potential issue | 🟡 MinorDescription doesn't match the page content — looks copy-pasted from feedback docs 📋
The
descriptionfield says "Module feedback definition details" but this page is about user-config definitions. This text often shows up in search results and meta tags, so it'd be nice to fix it.Suggested fix
-description: Module feedback definition details. +description: Module user-config definition details.for-developers/module-development/connection-advanced/subscribe-unsubscribe-flow.md-44-44 (1)
44-44:⚠️ Potential issue | 🟡 MinorMissing apostrophes: "isnt" → "isn't", "arent" → "aren't"
Suggested fix
-This optimisation isnt necessary for most modules, but if your users end up with hundreds of feedbacks of the same type, this will help ensure there arent performance issues. +This optimisation isn't necessary for most modules, but if your users end up with hundreds of feedbacks of the same type, this will help ensure there aren't performance issues.for-developers/module-development/connection-basics/variables.md-10-10 (1)
10-10:⚠️ Potential issue | 🟡 MinorSmall typo — "change update" looks like a leftover edit 🙂
Looks like there might be a stray word here. Maybe it should read "define variables and update their values"?
Suggested fix
-This section explains how to define variables and change update their values. +This section explains how to define variables and update their values.for-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.md-9-9 (1)
9-9:⚠️ Potential issue | 🟡 MinorTypo: "trigges" → "triggers", and missing period
Suggested fix
-A benefit of this, is that these boolean feedbacks can be used as conditions in the trigges system. We hope to allow for more complex behaviours with these feedbacks, but before that is worth doing we need more modules to be utilising the new type of feedback +A benefit of this is that these boolean feedbacks can be used as conditions in the triggers system. We hope to allow for more complex behaviours with these feedbacks, but before that is worth doing we need more modules to be utilising the new type of feedback.for-developers/module-development/connection-basics/user-configuration.md-25-25 (1)
25-25:⚠️ Potential issue | 🟡 MinorMissing space between sentences
There's a missing space between "...more details." and "The linked documentation...".
Suggested fix
-The fields you can use here are similar to the ones for actions and feedbacks, but with more limitations. See the [list of field types](./input-field-types.md) for more details.The linked documentation states any limitations that apply when used for the configuration, or if they are not allowed. +The fields you can use here are similar to the ones for actions and feedbacks, but with more limitations. See the [list of field types](./input-field-types.md) for more details. The linked documentation states any limitations that apply when used for the configuration, or if they are not allowed.for-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.md-138-140 (1)
138-140:⚠️ Potential issue | 🟡 MinorStep numbering jumps from 2 to 4, and small typo 📋
The steps go
### 1,### 2, then### 4— step 3 is missing! Either there's content that hasn't been added yet, or the numbering just needs adjusting.Also on line 140: "help with self" → "help with this".
Suggested fix
-### 4. Add an upgrade script +### 3. Add an upgrade script -Users will have feedbacks assigned to buttons already, and these will all need updating to the new format. A helper has been added to help with self. +Users will have feedbacks assigned to buttons already, and these will all need updating to the new format. A helper has been added to help with this.for-developers/module-development/connection-advanced/subscribe-unsubscribe-flow.md-37-37 (1)
37-37:⚠️ Potential issue | 🟡 MinorMissing opening backtick breaks inline code formatting
this.unsubscribeActions()is missing its opening backtick, so it'll render as plain text with a stray backtick.Suggested fix
-For actions, the methods `this.subscribeActions()` and this.unsubscribeActions()` can be used instead +For actions, the methods `this.subscribeActions()` and `this.unsubscribeActions()` can be used instead.for-developers/module-development/connection-basics/feedbacks.md-148-148 (1)
148-148:⚠️ Potential issue | 🟡 MinorTODO marker visible to readers
_TODO the linked page doesn't say much?_This note would be visible in the published docs. It might be worth removing it or converting it to an HTML comment (
<!-- TODO ... -->) so it's hidden from readers.for-developers/module-development/connection-basics/actions.md-99-99 (1)
99-99:⚠️ Potential issue | 🟡 MinorBroken link:
../api-changes/companion-4.1.mddoesn't resolve 🔗Hey there! The build pipeline is flagging that this link can't be resolved. It looks like the
api-changes/companion-4.1.mdfile might not exist yet in this PR. Would you mind either adding that file, updating the link to point to the correct location, or perhaps leaving just the version reference without the link for now?for-developers/module-development/connection-basics/input-field-types.md-77-77 (1)
77-77:⚠️ Potential issue | 🟡 MinorTODO marker visible to readers
_TODO: write an advanced page for this? or add to user-config?_Same note as the other files — this will show up in published docs. Consider hiding it in an HTML comment or removing it before merge.
for-developers/module-development/connection-basics/feedbacks.md-110-110 (1)
110-110:⚠️ Potential issue | 🟡 MinorBroken link:
../api-changes/companion-4.1.mddoesn't resolveSame issue as in
actions.md— the pipeline flags this link as unresolvable. Likely the target file hasn't been added to this PR yet.for-developers/module-development/connection-basics/actions.md-105-107 (1)
105-107:⚠️ Potential issue | 🟡 MinorTODO marker left in published documentation 📝
Just a heads-up — this TODO is visible to readers:
TODO: Was the following true for actions or only feedbacks?
Totally fine to have TODOs during development, but it'd be great to resolve this before merging so readers don't see unfinished notes. If you're not sure of the answer yet, maybe wrap the uncertain paragraph in a
:::cautionadmonition or remove it for now?Would you like me to open an issue to track this TODO so it doesn't get lost?
for-developers/module-development/connection-basics/feedbacks.md-65-70 (1)
65-70:⚠️ Potential issue | 🟡 MinorMissing comma in JS example — syntax error 🐛
There's a missing comma after the
labelvalue on line 67. This would be a syntax error if someone copies this example directly.Suggested fix
options: [{ type: 'number', - label: 'Source' + label: 'Source', id: 'source', default: 1 }],for-developers/module-development/connection-basics/feedbacks.md-87-87 (1)
87-87:⚠️ Potential issue | 🟡 MinorTypo: missing space in "documentationlinked"
Small typo — "documentationlinked" should be "documentation linked".
Suggested fix
-There are more properties available in feedback definitions, which are described in full in the documentationlinked in the [previous section](`#feedback-types`). +There are more properties available in feedback definitions, which are described in full in the documentation linked in the [previous section](`#feedback-types`).for-developers/module-development/connection-basics/input-field-types.md-22-24 (1)
22-24:⚠️ Potential issue | 🟡 MinorMissing comma in JS example — syntax error
Same issue as in
feedbacks.md: missing comma after thelabelstring on line 23.Suggested fix
{ type: 'number', - label: 'Source' + label: 'Source', id: 'source',for-developers/module-development/connection-basics/feedbacks.md-22-22 (1)
22-22:⚠️ Potential issue | 🟡 MinorCopy-paste slip: "actions" should be "feedbacks" ✏️
Looks like this sentence was borrowed from the actions page — it says "split the actions definitions" but since we're in the feedbacks doc, it should say "feedbacks definitions" (or "feedback definitions").
Suggested fix
-The [TypeScript module template](https://github.com/bitfocus/companion-module-template-ts) includes a file `src/feedbacks.ts` which is where your feedbacks should be defined. It is not required to use this structure, but it keeps it more readable than having everything in one file. More complex modules will likely want to split the actions definitions into even more files/folders. +The [TypeScript module template](https://github.com/bitfocus/companion-module-template-ts) includes a file `src/feedbacks.ts` which is where your feedbacks should be defined. It is not required to use this structure, but it keeps it more readable than having everything in one file. More complex modules will likely want to split the feedback definitions into even more files/folders.for-developers/module-development/connection-basics/input-field-types.md-13-13 (1)
13-13:⚠️ Potential issue | 🟡 MinorIncomplete sentence: "When defining an ," ✂️
It looks like a word got lost here! The sentence reads "When defining an ," which is missing its noun. Perhaps it should be something like "When defining an action, feedback, or config"?
Suggested fix
-When defining an , the actions, feedbacks and module config definition object includes a property called `options:` that takes a list of input-field definitions. For example, +When defining an action, feedback, or module config, the definition object includes a property called `options:` that takes a list of input-field definitions. For example,for-developers/module-development/connection-advanced/bonjour-device-discovery.md-1-3 (1)
1-3:⚠️ Potential issue | 🟡 MinorMissing YAML front matter 📋
All the other new docs in this PR include front matter (
---block with title, sidebar_label, sidebar_position, description). This file is missing it, which could affect how Docusaurus renders the page title, sidebar entry, and meta description. Would you mind adding something like:+--- +title: Bonjour Device Discovery +sidebar_label: Bonjour Discovery +description: How to use Bonjour (MDNS) for device discovery in Companion modules. +--- + Bonjour is a standardised method of device discovery, utilising MDNS.
🧹 Nitpick comments (10)
for-developers/module-development/index.md (1)
1-7: Small suggestion on the description — totally optional!
"Outline Module Development"reads a bit like a verb phrase rather than a description. Something like"An overview of module development for Companion"might flow more naturally, but no big deal either way. 🙂for-developers/module-development/connection-basics/presets.md (3)
12-12: Heading level jump flagged by markdownlint (MD001)The first heading after the doc title (h1) is
###(h3). Markdown best practice is to increment heading levels one at a time, so this should be##. The same applies to the other###headings (lines 49, 59, 113) — they'd naturally become##as well. Totally up to you whether to address now or later!
55-55: Link text "here" isn't very descriptive (MD059)Screen readers and accessibility tools benefit from meaningful link text. A small tweak would help:
Suggested fix
-You can see the full list of values that can be set and their valid values in the `style` object [here](https://bitfocus.github.io/companion-module-base/interfaces/CompanionButtonStyleProps.html) +You can see the full list of values that can be set and their valid values in the [`style` object documentation](https://bitfocus.github.io/companion-module-base/interfaces/CompanionButtonStyleProps.html)
70-89: Minor: inconsistent indentation in the code exampleLine 75 (
rotate_left) has an extra leading space compared to the surrounding properties (down,up,rotate_right). Just a tiny formatting nit — no rush!src/remark/autoTocPlugin.mjs (2)
96-98:startsWithis slightly too loose for filename exclusion — consider===insteadUsing
startsWith(vfilename)means if your current file is e.g.index.md, it would also exclude a hypotheticalindex.md.bakor similar. A strict equality check would be more precise and avoids surprises.Suggested fix
- const files = readdirSync(dir).filter((f) => f.endsWith('.md') && !f.startsWith(vfilename)) + const files = readdirSync(dir).filter((f) => f.endsWith('.md') && f !== vfilename)
186-200: Subdirectory rendering uses baretextandbreaknodes at the document root — may render unpredictably 🌲The tree-drawing characters (├──, └──) with braille blanks for indentation are a creative approach! Just a heads-up though: bare
textandbreaknodes outside of aparagraphin mdast aren't standard, and different renderers (or future Docusaurus versions) may handle them inconsistently. Screen readers could also announce the braille characters (U+2800) unexpectedly.If you run into rendering quirks, wrapping each sub-file entry in a
paragraphnode (or using a nested list) would be more robust. No action required right now if it's rendering fine for you — just flagging it as something to be aware of!for-developers/module-development/connection-basics/actions.md (1)
91-93: Consider more descriptive link text for accessibilityThe markdownlint tool flags generic link text like "here" (MD059). More descriptive text helps screen readers and makes scanning easier. For example:
Suggested improvement
-There are more properties available, which are described in full in [the autogenerated Actions documentation on GitHub](https://bitfocus.github.io/companion-module-base/interfaces/CompanionActionDefinition.html) +There are more properties available, which are described in full in [the autogenerated Actions documentation on GitHub](https://bitfocus.github.io/companion-module-base/interfaces/CompanionActionDefinition.html). -The `options` property of the action definition is an array of input types, as defined [here](./input-field-types.md) +The `options` property of the action definition is an array of input types, as defined in [Input Field Types](./input-field-types.md).for-developers/module-development/connection-advanced/bonjour-device-discovery.md (1)
38-56: Code block missing language specifierThis fenced code block doesn't have a language tag, so it won't get syntax highlighting. Adding
jswould be consistent with the other code blocks in this file and across the PR.Suggested fix
-``` +```js { type: 'textinput',for-developers/module-development/connection-basics/feedbacks.md (2)
93-93: Heading level skips from h2 to h4The markdownlint tool flags this (MD001): heading levels should increment by one. Since the parent is
##, this should be###instead of####.Suggested fix
-#### Inverting boolean feedbacks +### Inverting boolean feedbacks
99-104: Code block missing language specifierAdding a language tag (e.g.,
js) would enable syntax highlighting and be consistent with the rest of the doc.Suggested fix
-``` +```js CreateUseBuiltinInvertForFeedbacksUpgradeScript({
| --- | ||
| title: Connection Module Basic API | ||
| description: Basic API calls and objects for Companion connection modules | ||
| auto_toc: 2 | ||
| --- | ||
|
|
||
| This section describes advanced elements of the Companion connection-module API. |
There was a problem hiding this comment.
Title and description say "Basic" but this is the Advanced section 🔄
Looks like the frontmatter was copied from the basics index — the title says "Connection Module Basic API" and the description mentions "Basic API calls", but this is the Advanced section (line 7 correctly says "advanced elements"). The sidebar and navigation will show the wrong title.
Suggested fix
---
-title: Connection Module Basic API
-description: Basic API calls and objects for Companion connection modules
+title: Connection Module Advanced API
+description: Advanced API topics for Companion connection modules
auto_toc: 2
---
This section describes advanced elements of the Companion connection-module API.📝 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.
| --- | |
| title: Connection Module Basic API | |
| description: Basic API calls and objects for Companion connection modules | |
| auto_toc: 2 | |
| --- | |
| This section describes advanced elements of the Companion connection-module API. | |
| --- | |
| title: Connection Module Advanced API | |
| description: Advanced API topics for Companion connection modules | |
| auto_toc: 2 | |
| --- | |
| This section describes advanced elements of the Companion connection-module API. |
| To make it easier for users to configure your actions and feedbacks, you can implement the `learn` callback. This is intended to allow for the options of an action or feedback to be updated to match the current state of the device. | ||
|
|
||
| When this is implemented, a new button appears in the action or feedback allowing the user to trigger a learn event. | ||
|
|
||
|  | ||
|
|
||
| You can see the full api specification for [actions](https://bitfocus.github.io/companion-module-base/interfaces/CompanionActionDefinition.html#learn) and [feedbacks](https://bitfocus.github.io/companion-module-base/interfaces/CompanionBooleanFeedbackDefinition.html#learn). |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the directory structure and find the file
fd "learn-action-feedback-values.md" -type fRepository: bitfocus/website
Length of output: 230
🏁 Script executed:
# Also look at the directory to understand structure
ls -la for-developers/module-development/connection-advanced/ 2>/dev/null || echo "Path not found, searching for the directory..."Repository: bitfocus/website
Length of output: 698
🏁 Script executed:
# Search for the file more broadly
find . -name "learn-action-feedback-values.md" -type f 2>/dev/nullRepository: bitfocus/website
Length of output: 147
🏁 Script executed:
# Read the actual file to check for frontmatter
cat -n for-developers/module-development/connection-advanced/learn-action-feedback-values.mdRepository: bitfocus/website
Length of output: 2372
🏁 Script executed:
# Check if the image file exists - need to find where images directory is
find . -name "action-learn-button.png" -type f 2>/dev/nullRepository: bitfocus/website
Length of output: 126
🏁 Script executed:
# Also check what's in the images directory relative to the md file
ls -la for-developers/module-development/connection-advanced/images/ 2>/dev/null || echo "No images directory in connection-advanced"
ls -la for-developers/module-development/images/ 2>/dev/null || echo "No images directory in module-development"
ls -la for-developers/images/ 2>/dev/null || echo "No images directory in for-developers"Repository: bitfocus/website
Length of output: 546
🏁 Script executed:
# Check if other files in the same directory have frontmatter
head -20 for-developers/module-development/connection-advanced/bonjour-device-discovery.md
head -20 for-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.md
head -20 for-developers/module-development/connection-advanced/index.mdRepository: bitfocus/website
Length of output: 1977
🏁 Script executed:
# Check more files to understand the frontmatter pattern across the site
find for-developers/module-development -name "*.md" -type f | head -10 | while read file; do
echo "=== $file ==="
head -5 "$file"
echo ""
doneRepository: bitfocus/website
Length of output: 1740
Thanks for the contribution! Just missing the YAML frontmatter here.
This file needs the frontmatter block at the top, just like other docs in the repo. Something like:
---
title: Learn Action and Feedback Values
description: Implementing the learn callback for actions and feedbacks
sidebar_label: Learn Action and Feedback Values
---Without it, Docusaurus won't display the page correctly in the sidebar. The image reference is fine — that path resolves correctly!
| ## Why | ||
|
|
||
| Most feedbacks should now be defined as 'boolean' feedbacks. The idea is to gradually convert them across over time, as they give the user more flexibility and will end up with a more consistent interface. | ||
|
|
||
| Previously, it was up to the module author to decide what properties a feedback should change. This means that most feedbacks are only able to change both the background and text colour. But what if the user wants to change a png, or the text? They could ask for that to be possible, but that would likely require the module author to duplicate the feedback with different style options. | ||
|
|
There was a problem hiding this comment.
Missing YAML frontmatter — this page might not render correctly in the sidebar 🏷️
All the other docs in this PR include a YAML frontmatter block with title, sidebar_label, sidebar_position, and description. This file jumps straight into the content. Without frontmatter, Docusaurus may not place it correctly in the sidebar or generate proper metadata.
Would you mind adding something like:
---
title: Migrating to Boolean Feedbacks
sidebar_label: Boolean Feedback Migration
sidebar_position: <appropriate number>
description: Guide to migrating legacy feedbacks to boolean feedbacks.
---| ```js | ||
| feedbacks['set_source'] = { | ||
| type: 'boolean', // Change this | ||
| label: 'Brief description of the feedback here', | ||
| description: 'Longer description of the feedback', | ||
| defaultStyle: { | ||
| // Move the values from options to here | ||
| color: combineRgb(0, 0, 0), | ||
| bgcolor: combineRgb(255, 0, 0) | ||
| } | ||
| // remove the old style properties from options | ||
| options: [{ | ||
| type: 'number', | ||
| label: 'Source' | ||
| id: 'source', | ||
| default: 1 | ||
| }] | ||
| callback: (feedback) => { | ||
| // Update this to return the boolean value you used to use to decide to return the style object | ||
| return this.currentSource == feedback.options.source | ||
| }, | ||
| } | ||
| ``` |
There was a problem hiding this comment.
A few syntax issues in the "To" code example 🧩
The "To" JavaScript example has some missing commas that would cause syntax errors if someone copies it:
- Line 63: Missing comma after the closing
}ofdefaultStyle - Line 67: Missing comma after
'Source'(same issue exists in the "From" block at line 37) - Line 70: Missing comma after closing
]ofoptions
Since this is a migration guide, folks will likely copy these examples closely — it'd be great to keep them syntactically valid!
Suggested fix for the "To" block
defaultStyle: {
// Move the values from options to here
color: combineRgb(0, 0, 0),
bgcolor: combineRgb(255, 0, 0)
- }
+ },
// remove the old style properties from options
options: [{
type: 'number',
- label: 'Source'
+ label: 'Source',
id: 'source',
default: 1
- }]
+ }],
callback: (feedback) => {📝 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.
| ```js | |
| feedbacks['set_source'] = { | |
| type: 'boolean', // Change this | |
| label: 'Brief description of the feedback here', | |
| description: 'Longer description of the feedback', | |
| defaultStyle: { | |
| // Move the values from options to here | |
| color: combineRgb(0, 0, 0), | |
| bgcolor: combineRgb(255, 0, 0) | |
| } | |
| // remove the old style properties from options | |
| options: [{ | |
| type: 'number', | |
| label: 'Source' | |
| id: 'source', | |
| default: 1 | |
| }] | |
| callback: (feedback) => { | |
| // Update this to return the boolean value you used to use to decide to return the style object | |
| return this.currentSource == feedback.options.source | |
| }, | |
| } | |
| ``` |
| - [Upgrade Scripts](Upgrade-scripts) | ||
| - [Feedbacks](Feedbacks) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's see the overall structure
tree -L 3 for-developers/module-development/ 2>/dev/null | head -50Repository: bitfocus/website
Length of output: 1239
🏁 Script executed:
#!/bin/bash
# Check if files matching the suggested paths exist
fd "upgrade-scripts" --extension md
fd "feedbacks" --extension mdRepository: bitfocus/website
Length of output: 465
🏁 Script executed:
#!/bin/bash
# Let's look at the actual file content around lines 175-176
cat -n "for-developers/module-development/connection-advanced/migrating-legacy-to-boolean-feedbacks.md" | sed -n '170,180p'Repository: bitfocus/website
Length of output: 283
🏁 Script executed:
#!/bin/bash
# Check the connection-basics directory specifically
ls -la for-developers/module-development/connection-basics/ 2>/dev/nullRepository: bitfocus/website
Length of output: 747
Fix these internal links to make them point to the actual documentation files 📖
The links (Upgrade-scripts) and (Feedbacks) won't resolve—they're missing the directory path and have capitalization that doesn't match the actual files. The files do exist in the connection-basics folder, so here's the fix:
-- [Upgrade Scripts](Upgrade-scripts)
-- [Feedbacks](Feedbacks)
+- [Upgrade Scripts](../connection-basics/upgrade-scripts.md)
+- [Feedbacks](../connection-basics/feedbacks.md)Thanks for working on the docs! This is a quick fix that'll make sure readers can navigate smoothly. 🙏
📝 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.
| - [Upgrade Scripts](Upgrade-scripts) | |
| - [Feedbacks](Feedbacks) | |
| - [Upgrade Scripts](../connection-basics/upgrade-scripts.md) | |
| - [Feedbacks](../connection-basics/feedbacks.md) |
| @@ -0,0 +1,44 @@ | |||
| Sometimes it is useful to know what actions/feedbacks are being used. This is common for devices which have thousands of properties, or if loading and maintining a bit of data has a cost, such as requiring polling to fetch. | |||
There was a problem hiding this comment.
Missing frontmatter, and typo: "maintining" → "maintaining"
Like a couple of the other new advanced docs, this file is missing the YAML frontmatter block. Also a small typo on the first line.
Suggested frontmatter + typo fix
+---
+title: Subscribe / Unsubscribe Flow
+sidebar_label: Subscribe / Unsubscribe
+sidebar_position: <appropriate number>
+description: How to use subscribe and unsubscribe callbacks for actions and feedbacks.
+---
+
-Sometimes it is useful to know what actions/feedbacks are being used. This is common for devices which have thousands of properties, or if loading and maintining a bit of data has a cost, such as requiring polling to fetch.
+Sometimes it is useful to know what actions/feedbacks are being used. This is common for devices which have thousands of properties, or if loading and maintaining a bit of data has a cost, such as requiring polling to fetch.📝 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.
| Sometimes it is useful to know what actions/feedbacks are being used. This is common for devices which have thousands of properties, or if loading and maintining a bit of data has a cost, such as requiring polling to fetch. | |
| --- | |
| title: Subscribe / Unsubscribe Flow | |
| sidebar_label: Subscribe / Unsubscribe | |
| sidebar_position: <appropriate number> | |
| description: How to use subscribe and unsubscribe callbacks for actions and feedbacks. | |
| --- | |
| Sometimes it is useful to know what actions/feedbacks are being used. This is common for devices which have thousands of properties, or if loading and maintaining a bit of data has a cost, such as requiring polling to fetch. |
🧰 Tools
🪛 LanguageTool
[grammar] ~1-~1: Ensure spelling is correct
Context: ...usands of properties, or if loading and maintining a bit of data has a cost, such as requi...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
| ##### feedbacks | ||
|
|
||
| This is an array of the feedbacks that may need upgrading to the new version. | ||
|
|
||
| Note: This may not be all of the actions that exist. Sometimes it will be called with actions that have been imported from a page from an older version of companion | ||
|
|
||
| Any changed feedbacks should be added to the `updatedFeedbacks` array in the result. |
There was a problem hiding this comment.
Copy-paste slip — this section says "actions" but should say "feedbacks" 🔄
Lines 79 mentions "actions" twice, but this is the feedbacks section. Looks like it was copied from the actions section above (lines 71-73) without updating the terminology. This could confuse readers trying to understand the difference.
Suggested fix
##### feedbacks
This is an array of the feedbacks that may need upgrading to the new version.
-Note: This may not be all of the actions that exist. Sometimes it will be called with actions that have been imported from a page from an older version of companion
+Note: This may not be all of the feedbacks that exist. Sometimes it will be called with feedbacks that have been imported from a page from an older version of companion
Any changed feedbacks should be added to the `updatedFeedbacks` array in the result.📝 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.
| ##### feedbacks | |
| This is an array of the feedbacks that may need upgrading to the new version. | |
| Note: This may not be all of the actions that exist. Sometimes it will be called with actions that have been imported from a page from an older version of companion | |
| Any changed feedbacks should be added to the `updatedFeedbacks` array in the result. | |
| ##### feedbacks | |
| This is an array of the feedbacks that may need upgrading to the new version. | |
| Note: This may not be all of the feedbacks that exist. Sometimes it will be called with feedbacks that have been imported from a page from an older version of companion | |
| Any changed feedbacks should be added to the `updatedFeedbacks` array in the result. |
🧰 Tools
🪛 LanguageTool
[style] ~79-~79: Consider removing “of” to be more concise
Context: ...the new version. Note: This may not be all of the actions that exist. Sometimes it will b...
(ALL_OF_THE)
| ```js | ||
| this.setVariableValues({ | ||
| 'variable1': 'new value' | ||
| 'variable2': 99, | ||
| 'old_variable': undefined // This unsets a value | ||
| }) | ||
| ``` |
There was a problem hiding this comment.
Missing comma in the code example — this would be a syntax error ✏️
The object literal on line 36 is missing a trailing comma after 'new value', which would cause a SyntaxError for anyone copying this snippet. Since this is teaching material, it'd be great to keep the examples copy-pasteable!
Suggested fix
this.setVariableValues({
- 'variable1': 'new value'
+ 'variable1': 'new value',
'variable2': 99,
'old_variable': undefined // This unsets a value
})📝 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.
| ```js | |
| this.setVariableValues({ | |
| 'variable1': 'new value' | |
| 'variable2': 99, | |
| 'old_variable': undefined // This unsets a value | |
| }) | |
| ``` |
| const dirs = subdirs.map((d) => { | ||
| const subdir = join(dir, d) | ||
| const raw = readFileSync(join(subdir, '_category_.json')) | ||
| const frontmatter = JSON.parse(raw) | ||
| const indexfile = frontmatter.link?.id | ||
| const subfiles = readdirSync(subdir).filter((f) => f.endsWith('.md') && !f.startsWith(indexfile)) | ||
| const subpages = getPages(subfiles, subdir) | ||
| subpages.sort((a, b) => a.sidebarPosition - b.sidebarPosition) |
There was a problem hiding this comment.
Potential bug when _category_.json has no link.id — and missing encoding on readFileSync
Two things here:
-
indexfilecan beundefined(line 120) if the category JSON has nolinkor nolink.id. When that happens,f.startsWith(undefined)coercesundefinedto the string"undefined", which silently does the wrong thing rather than skipping the filter. -
Missing
'utf-8'encoding on line 118 —readFileSyncwithout encoding returns aBuffer. It works becauseJSON.parseaccepts Buffers, but it's inconsistent with line 102 which properly passes'utf-8'.
Suggested fix
- const raw = readFileSync(join(subdir, '_category_.json'))
+ const raw = readFileSync(join(subdir, '_category_.json'), 'utf-8')
const frontmatter = JSON.parse(raw)
const indexfile = frontmatter.link?.id
- const subfiles = readdirSync(subdir).filter((f) => f.endsWith('.md') && !f.startsWith(indexfile))
+ const subfiles = readdirSync(subdir).filter((f) => f.endsWith('.md') && (!indexfile || f !== indexfile))
|
I'm not keen on that http://localhost:4005/for-developers/module-development page. I feel like this would be better as a handcrafted page. For http://localhost:4005/for-developers/module-development/connection-basics/, I wonder if this auto-toc is beneficial, vs the other category layout. Because the headings only really make sense within the context of the page they are from. |
There seems to be alot of overlap between module-development-101.md, your new overview.md, and user-configuration.md I realized we're missing two big issues: the module class itself, and establishing connections. So I added stuff. Copy-edited: - actions.md - overview.md Added: - connecting.md Updated: - variables.md - presets.md (typos caught something in v2.0.md)
I hear your argument, especially wrt the API log needing the auto-toc more than other sections, but think it's still better than the silly cards. Maybe we can refine later? At worst, we can remove it from this PR if we can't agree too easily.
Not inconsistent! Just to be sure we're on the same page: they indicate whether it's an in-page link vs. a file in a subdir. "├──" is pretty standard notation for "text folder-trees" Put a bit more extravagantly: It's like saying this message is inconsistent because some lines start with a vertical line, some with a bullet and some are flush left! (I suppose you could say I'm inconsistent in not showing headings for files in the subdirs, but that's the point at which I start thinking "its quite a wall of links.") That said, we can certainly debate styling or whether or how much should be there at all. My thought is that the current design provides a little more depth/overview than the sidebar, which starts with all categories closed.
Not sure I understand this concern: isn't the context right there in the auto-toc as well? I think in general, its value is more in giving an overview of the section-structure than providing in-page links, though both can be useful... FWIW, I'd just as soon not do anything hand-crafted because these pages are likely-as-not to be overlooked -- you only get it if you click the name of the group as opposed to clicking on the caret... OK, stepping off for today. All of my local changes have been pushed to here. |
- minor fix to autToc - transfer bonjour.png from a "hidden" asset - fix typos (feedbacks.md)
build now works but you may have to run `yarn docusaurus clear` to clear the build caches (deleting /build is likely not enough).
|
Important note from the commit comment:
done for (my) today. |
2 participants
This is a wip. Two features:
auto_toc: 2will report only level 1 and level 2 headers in the filesThe following files/folders are essentially done in the module-development/ folder:
-- index.md (autogenerated)
-- user-configuration.md
-- actions.md
-- feedbacks.md
-- input-field-types.md
If you look at index.md or the user-config/actions/feedbacks files, you'll see a more-or-less standardized structure that I've currently settled on. Feel free to comment/fix anything.
Please also search for TODO in these files for questions I couldn't resolve on my own.
(Note that the autoToc pages do not update with the incremental builds, you have to restart
yarn startto see the changes.)Summary by CodeRabbit