Skip to content
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

Wifi segment #1262

Closed
wants to merge 5 commits into from
Closed

Wifi segment #1262

wants to merge 5 commits into from

Conversation

mikesigs
Copy link
Contributor

Prerequisites

  • I have read and understood the CONTRIBUTING guide
  • The commit message follows the conventional commits guidelines
  • Tests for the changes have been added (for bug fixes/features)
  • Docs have been added / updated (for bug fixes/features)

Description

[Description of the change]

@mikesigs mikesigs closed this Nov 20, 2021
if !w.env.hasCommand(cmd) {
return false
}
cmdResult, err := w.env.runCommand(cmd, "wlan", "show", "interfaces")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this won't work cross platform so if you want to contribute this, you need to find a way that works everywhere ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didn't see these comments. I meant to push the PR to my own repo so I could run the GitHub Actions.

I have a new PR that I just submitted. But it will have this same limitation. Testing this feature is impossible without a physical Linux machine with an actual WiFi network interface. Virtual Linux environments don't have that.

What other things could I do? I noticed the spotify segment has some compilation hints that allows them to have Windows, Linux, and WSL specific implementations. I was thinking of doing something like that, and just excluding Linux/Darwin for now.

return false
}

regex := regexp.MustCompile(`(.+) : (.+)`)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we also have regex helpers for this (we cache the compilations), see regex.go for more information.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice. I will look into that and update my other PR.

}

func (w *wifi) string() string {
segmentTemplate := w.props.getString(SegmentTemplate, "{{.SSID}} {{.SignalStrength}} {{.ReceiveRate}}Mbps")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well done, that's 👍🏻

@mikesigs
Copy link
Contributor Author

mikesigs commented Nov 21, 2021

Here is the actual PR with a bunch more work on it!

#1270

Or should we close the other one and re-open this one?? I don't have the ability to Reopen. The button is greyed out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants