Skip to content

Conversation

h0ek
Copy link

@h0ek h0ek commented Aug 3, 2025

User description

Add a new wake plugin to Oh My Bash for easy Wake-on-LAN. The plugin:

  • Reads a MAC address and broadcast IP from ~/.wakeonlan/
  • Sends the magic packet via wakeonlan -i
  • Provides Bash autocompletion of device names
  • This lets users simply run wake to power on machines on their LAN.

PR Type

Enhancement


Description

  • Add wake plugin for Wake-on-LAN functionality

  • Provides device autocompletion from ~/.wakeonlan directory

  • Includes comprehensive documentation and usage examples

  • Updates main plugins README with wake entry


Diagram Walkthrough

flowchart LR
  A["User runs 'wake <device>'"] --> B["Read MAC/broadcast from ~/.wakeonlan/<device>"]
  B --> C["Execute wakeonlan command"]
  D["Tab completion"] --> E["List available devices from ~/.wakeonlan"]
Loading

File Walkthrough

Relevant files
Enhancement
wake.plugin.sh
Core wake plugin implementation                                                   

plugins/wake/wake.plugin.sh

  • Implements wake() function that reads MAC and broadcast IP from config
    files
  • Adds error handling for missing wakeonlan tool and invalid devices
  • Provides bash autocompletion for device names from ~/.wakeonlan
    directory
  • Includes usage instructions when called without arguments
+37/-0   
Documentation
README.md
Update plugins documentation table                                             

plugins/README.md

  • Adds wake plugin entry to the plugins table
  • Describes wake plugin as wakeonlan tool wrapper
+2/-1     
README.md
Complete wake plugin documentation                                             

plugins/wake/README.md

  • Comprehensive documentation for wake plugin usage
  • Configuration file format and directory structure explanation
  • Installation instructions for wakeonlan dependency
  • Usage examples and autocompletion demonstration
+57/-0   

h0ek added 3 commits August 3, 2025 17:51
Add wake plugin: WoL wrapper reading MAC and broadcast IP
Wake plugin readme.
Added Wake plugin to the table.
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Error Handling

The function doesn't validate if the config directory exists before listing devices in the usage message, which could cause an error if ~/.wakeonlan doesn't exist.

echo "Available devices: $(ls "$cfgdir" | tr '\n' ' ')"
return 1
Input Validation

No validation is performed on the MAC address and broadcast IP read from the config file, which could lead to wakeonlan being called with invalid parameters.

read -r mac bcast < "$cfgfile"

# Send magic-packet to the given broadcast address
wakeonlan -i "$bcast" "$mac"

Copy link

qodo-merge-for-open-source bot commented Aug 3, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Validate config file content format
Suggestion Impact:The suggestion was directly implemented with minor modifications - the validation logic was added exactly as suggested, with only a slight change in the error message to include the filename

code diff:

+ if [[ -z "$mac" || -z "$bcast" ]]; then
+   echo "ERROR: Invalid config format in '$cfgfile'. Expected: <MAC> <broadcast-IP>" >&2
+   return 1
+ fi

The read command will silently fail if the config file is empty or malformed,
leading to undefined behavior when calling wakeonlan. Add validation to ensure
both MAC and broadcast IP are properly read from the file.

plugins/wake/wake.plugin.sh [22-24]

 # Read the two fields: MAC and broadcast-IP
 local mac bcast
 read -r mac bcast < "$cfgfile"
+if [[ -z "$mac" || -z "$bcast" ]]; then
+  echo "ERROR: Invalid config file format. Expected: <MAC> <broadcast-IP>" >&2
+  return 1
+fi

[Suggestion processed]

Suggestion importance[1-10]: 8

__

Why: This is a valuable suggestion that adds necessary input validation, preventing the wakeonlan command from being called with empty arguments if the config file is malformed.

Medium
Check directory existence before listing
Suggestion Impact:The suggestion was directly implemented - the commit added the exact directory existence check with conditional ls command and error message as suggested

code diff:

+   if [[ -d "$cfgdir" ]]; then
+     echo "Available devices: $(ls "$cfgdir" | tr '\n' ' ')"
+   else
+     echo "No devices configured. Create $cfgdir directory first."
+   fi

The ls command will fail if the ~/.wakeonlan directory doesn't exist, causing an
error message. Check if the directory exists before listing its contents to
provide a cleaner user experience.

plugins/wake/wake.plugin.sh [11-15]

 if [[ -z "$1" || ! -f "$cfgfile" ]]; then
   echo "Usage: wake <device>"
-  echo "Available devices: $(ls "$cfgdir" | tr '\n' ' ')"
+  if [[ -d "$cfgdir" ]]; then
+    echo "Available devices: $(ls "$cfgdir" | tr '\n' ' ')"
+  else
+    echo "No devices configured. Create $cfgdir directory first."
+  fi
   return 1
 fi

[Suggestion processed]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that ls will error if the config directory doesn't exist, and the proposed change handles this case gracefully, improving user experience.

Medium
Fix typo in documentation
Suggestion Impact:The suggestion was directly implemented - the typo "distirbution" was corrected to "distribution" in the exact same line and context as suggested

code diff:

-wakeonlan man page. If your distirbution does not offer wakeonlan package just install it manually from the GitHub, here are the steps:
+wakeonlan man page. If your distribution does not offer wakeonlan package just install it manually from the GitHub, here are the steps:

There's a typo in "distirbution" which should be "distribution". This affects
the professional appearance of the documentation.

plugins/wake/README.md [49]

-If your distirbution does not offer wakeonlan package just install it manually from the GitHub, here are the steps:
+If your distribution does not offer wakeonlan package just install it manually from the GitHub, here are the steps:

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 3

__

Why: The suggestion correctly identifies and fixes a typo in the documentation, which is a minor but good improvement for professionalism.

Low
  • Update

h0ek added 2 commits August 3, 2025 18:11
Validate config format and directory before listing.
Typo fix.
@h0ek h0ek requested a review from akinomyoga August 5, 2025 20:59
@h0ek h0ek requested a review from akinomyoga August 6, 2025 10:22
h0ek and others added 3 commits August 8, 2025 14:30
h0ek added 2 commits August 8, 2025 15:14
Suggestions and additional features implemented.
Updated readme based on additional features implemented.
@h0ek
Copy link
Author

h0ek commented Aug 8, 2025

I spent some more time and improved the script based on your suggestions and additions that came up while I was using this plugin.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants