-
Notifications
You must be signed in to change notification settings - Fork 356
Add documentation for network-public-ip plugin #374
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
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis pull request adds documentation for a new network-public-ip option and updates the network-public-ip script to default the label to "IP" and emit the label and ip separated by a space. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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. Comment |
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
🧹 Nitpick comments (1)
docs/CONFIG.md (1)
649-658: Documentation looks good; consider adding trailing space for style consistency.The documentation is clear and follows the established plugin pattern. The functionality is correct as-is.
For consistency with other plugin examples in this file (e.g.,
battery-labelat line 205,network-offline-labelat line 633), consider adding a trailing space in the example:-set -g @dracula-network-public-ip-label "" +set -g @dracula-network-public-ip-label " "Both approaches work since the script adds a space programmatically; this is purely for documentation style consistency.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/CONFIG.md(2 hunks)scripts/network-public-ip.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/network-public-ip.sh (1)
scripts/utils.sh (1)
get_tmux_option(3-12)
🔇 Additional comments (2)
docs/CONFIG.md (1)
28-28: LGTM!The TOC entry is correctly positioned alphabetically and follows the established format.
scripts/network-public-ip.sh (1)
22-23: LGTM!The implementation correctly:
- Reads the label option using
get_tmux_optionwith a sensible default of "IP"- Adds the missing space between the label and IP value in the output
- Follows the established pattern used by other plugins in this repository
7f46e71 to
412cd20
Compare
|
Thank you very much for your contribution! |
|
|
||
| ### network-public-ip - [up](#table-of-contents) | ||
|
|
||
| This widget displays the public IP address you're using. |
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'd suggest adding info on how the public ip is aquired, as this is an external request to a third party service.
| This widget displays the public IP address you're using. | |
| This widget displays the public IP address you're using, by querying the public service `ifconfig.me`. |
I've found out that the documentation for the
network-public-ipplugin was missing.This PR adds the docs of it, and add a missing space between the label and the IP value of it's output.
Thank you!