-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update shell_command.markdown #35906
base: current
Are you sure you want to change the base?
Conversation
Provide a clearer distinction between code for automation and code for configuration in the examples.
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not understanding this one:
Provide a clearer distinction between code for automation and code for configuration in the examples.
They are both part of the same configuration.
Your addition therefore makes it more confusing IMHO. Could you clarify what your confusion is about?
../Frenck
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
📝 WalkthroughWalkthroughThe documentation for the Shell Command integration in Home Assistant has been enhanced with additional examples and clarifications. Two new commands, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HomeAssistant
participant ShellCommand
User->>HomeAssistant: Trigger automation
HomeAssistant->>ShellCommand: Execute get_file_contents
ShellCommand-->>HomeAssistant: Return stdout, stderr, returncode
HomeAssistant->>User: Notify based on command output
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
source/_integrations/shell_command.markdown (3)
98-99
: Add blank lines around code blocks for better readabilityAccording to markdown best practices, fenced code blocks should be surrounded by blank lines. This improves readability and ensures proper rendering.
Add blank lines before and after these code blocks:
step: 1 + ```yaml # configuration.yaml entry
{% endraw %}
Also applies to: 133-134 <details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 98-98: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- 99-99: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) </details> </details> --- `98-100`: **Enhance the AC control example with additional context** While the example is good, it would be more helpful to add: 1. A brief explanation of what the example accomplishes 2. Note about the `irsend` command requirements 3. Expected outcome of the automation Consider adding a description like: ```diff +# This example demonstrates how to control an AC unit using IR commands +# based on a slider value. Requires an IR transmitter and the `irsend` command. +# When the slider value changes, it sends the corresponding IR command +# to set the AC temperature. # configuration.yaml entry shell_command: set_ac_to_slider: 'irsend SEND_ONCE DELONGHI AC_{{ states("input_number.ac_temperature") }}_AUTO'
🧰 Tools
🪛 Markdownlint (0.35.0)
98-98: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
99-99: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
133-135
: Add security considerations for file reading exampleThe file reading example should include important security notes:
- File path validation
- File permissions requirements
- Potential security implications
Consider adding a warning note:
+# IMPORTANT: Ensure proper validation of the filename parameter and +# restrict access to only necessary files. The Home Assistant user +# must have appropriate permissions to read the specified files. # configuration.yaml entry shell_command: get_file_contents: "cat {{ filename }}"Additionally, consider using
cat {{ filename | quote }}
to properly handle filenames with spaces or special characters.🧰 Tools
🪛 Markdownlint (0.35.0)
133-133: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
134-134: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
source/_integrations/shell_command.markdown
(2 hunks)
🧰 Additional context used
🪛 Markdownlint (0.35.0)
source/_integrations/shell_command.markdown
98-98: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
99-99: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
133-133: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
134-134: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
📝 WalkthroughWalkthroughThe documentation for the Shell Command integration in Home Assistant has been updated to improve clarity and usability. New YAML configuration examples have been added for the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HomeAssistant
participant ShellCommand
User->>HomeAssistant: Trigger shell command
HomeAssistant->>ShellCommand: Execute command (e.g., set_ac_to_slider)
ShellCommand-->>HomeAssistant: Return output and returncode
HomeAssistant->>User: Notify success or error based on returncode
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
source/_integrations/shell_command.markdown (2)
98-99
: Add blank lines around code blocks for better readabilityAccording to markdown best practices, fenced code blocks should be surrounded by blank lines to ensure proper rendering and readability.
Add blank lines before and after the YAML code blocks at lines 98-99 and 133-134.
Also applies to: 133-134
🧰 Tools
🪛 Markdownlint (0.35.0)
98-98: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
99-99: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
100-102
: Consider adding prerequisite information for irsendThe example effectively demonstrates template usage, but the
irsend
command might need additional context for users.Consider adding a note about:
- Prerequisites for using
irsend
(e.g., LIRC installation)- Link to LIRC documentation or setup instructions
- Explanation of the DELONGHI AC command format
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
source/_integrations/shell_command.markdown
(2 hunks)
🧰 Additional context used
🪛 Markdownlint (0.35.0)
source/_integrations/shell_command.markdown
98-98: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
99-99: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
133-133: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
134-134: null
Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
# configuration.yaml entry | ||
shell_command: | ||
get_file_contents: "cat {{ filename }}" | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add security warning about file access
The example effectively demonstrates response handling, but should include security considerations.
Add a warning note about:
- Ensuring proper file permissions
- Restricting access to sensitive files
- Using absolute paths within the Home Assistant configuration directory
- Input validation for the filename parameter
Example addition:
{% note title="Security" %}
Ensure that:
- The file is within the Home Assistant configuration directory
- The file has appropriate read permissions
- The filename parameter is properly validated
- Access to sensitive files is restricted
{% endnote %}
Provide a clearer distinction between code for automation and code for configuration in the examples.
Proposed change
Type of change
current
branch).current
branch).next
branch).next
branch).Additional information
Checklist
current
branch.next
branch.Summary by CodeRabbit
set_ac_to_slider
andget_file_contents
.