Repository: JanDeDobbeleer/oh-my-posh
Pull request #2143 (2022-04-24 11:52:58)

battery fixes
### Prerequisites

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

### Description

- refactor(battery): move logic to environment
- feat(darwin): manually parse battery info


<!---

Tips:

If you're not comfortable with working with Git, we're working a guide (https://ohmyposh.dev/docs/contributing_git) to help you out.
Oh My Posh advises GitKraken (https://www.gitkraken.com/invite/nQmDPR9D) as your preferred cross platform Git GUI power tool.

-->

[cc]: https://www.conventionalcommits.org/en/v1.0.0/#summary



Patch:
diff --git a/src/cli/root.go b/src/cli/root.go
index 140f3077..83cbf026 100644
--- a/src/cli/root.go
+++ b/src/cli/root.go
@@ -7,11 +7,12 @@ import (
 	"github.com/spf13/cobra"
 )
 
-// Version number of oh-my-posh
 var (
 	config         string
 	displayVersion bool
-	cliVersion     string
+
+	// Version number of oh-my-posh
+	cliVersion string
 )
 
 var rootCmd = &cobra.Command{
diff --git a/src/engine/config.go b/src/engine/config.go
index 520840d4..bb382c11 100644
--- a/src/engine/config.go
+++ b/src/engine/config.go
@@ -234,7 +234,7 @@ func escapeGlyphs(s string) string {
 
 func defaultConfig() *Config {
 	cfg := &Config{
-		Version:    1,
+		Version:    2,
 		FinalSpace: true,
 		Blocks: []*Block{
 			{
diff --git a/src/environment/darwin.go b/src/environment/darwin.go
new file mode 100644
index 00000000..71893e0d
--- /dev/null
+++ b/src/environment/darwin.go
@@ -0,0 +1,64 @@
+//go:build darwin
+
+package environment
+
+import (
+	"errors"
+	"oh-my-posh/regex"
+	"strconv"
+	"strings"
+	"time"
+
+	"github.com/distatus/battery"
+)
+
+func mapMostLogicalState(state string) battery.State {
+	switch state {
+	case "charging":
+		return battery.Charging
+	case "discharging":
+		return battery.Discharging
+	case "AC attached":
+		return battery.NotCharging
+	case "full":
+		return battery.Full
+	case "empty":
+		return battery.Empty
+	case "charged":
+		return battery.Full
+	default:
+		return battery.Unknown
+	}
+}
+
+func (env *ShellEnvironment) parseBatteryOutput(output string) (*BatteryInfo, error) {
+	matches := regex.FindNamedRegexMatch(`(?P<PERCENTAGE>[0-9]{1,3})%; (?P<STATE>[a-zA-Z\s]+);`, output)
+	if len(matches) != 2 {
+		msg := "Unable to find battery state based on output"
+		env.log(Error, "BatteryInfo", msg)
+		return nil, errors.New(msg)
+	}
+	var percentage int
+	var err error
+	if percentage, err = strconv.Atoi(matches["PERCENTAGE"]); err != nil {
+		env.log(Error, "BatteryInfo", err.Error())
+		return nil, errors.New("Unable to parse battery percentage")
+	}
+	return &BatteryInfo{
+		Percentage: percentage,
+		State:      mapMostLogicalState(matches["STATE"]),
+	}, nil
+}
+
+func (env *ShellEnvironment) BatteryState() (*BatteryInfo, error) {
+	defer env.trace(time.Now(), "BatteryInfo")
+	output, err := env.RunCommand("pmset", "-g", "batt")
+	if err != nil {
+		env.log(Error, "BatteryInfo", err.Error())
+		return nil, err
+	}
+	if !strings.Contains(output, "Battery") {
+		return nil, errors.New("No battery found")
+	}
+	return env.parseBatteryOutput(output)
+}
diff --git a/src/environment/shell.go b/src/environment/shell.go
index ac6ca0f2..6dbb459e 100644
--- a/src/environment/shell.go
+++ b/src/environment/shell.go
@@ -122,6 +122,11 @@ type TemplateCache struct {
 	WSL          bool
 }
 
+type BatteryInfo struct {
+	Percentage int
+	State      battery.State
+}
+
 type Environment interface {
 	Getenv(key string) string
 	Pwd() string
@@ -149,7 +154,7 @@ type Environment interface {
 	RunShellCommand(shell, command string) string
 	ExecutionTime() float64
 	Flags() *Flags
-	BatteryInfo() ([]*battery.Battery, error)
+	BatteryState() (*BatteryInfo, error)
 	QueryWindowTitles(processName, windowTitleRegex string) (string, error)
 	WindowsRegistryKeyValue(path string) (*WindowsRegistryValue, error)
 	HTTPRequest(url string, timeout int, requestModifiers ...HTTPRequestModifier) ([]byte, error)
@@ -560,66 +565,6 @@ func (env *ShellEnvironment) Flags() *Flags {
 	return env.CmdFlags
 }
 
-func (env *ShellEnvironment) BatteryInfo() ([]*battery.Battery, error) {
-	defer env.trace(time.Now(), "BatteryInfo")
-	batteries, err := battery.GetAll()
-	// actual error, return it
-	if err != nil && len(batteries) == 0 {
-		env.log(Error, "BatteryInfo", err.Error())
-		return nil, err
-	}
-	// there are no batteries found
-	if len(batteries) == 0 {
-		return nil, &NoBatteryError{}
-	}
-	// some batteries fail to get retrieved, filter them out if present
-	validBatteries := []*battery.Battery{}
-	for _, batt := range batteries {
-		if batt != nil {
-			validBatteries = append(validBatteries, batt)
-		}
-	}
-	// clean minor errors
-	unableToRetrieveBatteryInfo := "A device which does not exist was specified."
-	unknownChargeRate := "Unknown value received"
-	var fatalErr battery.Errors
-	ignoreErr := func(err error) bool {
-		if e, ok := err.(battery.ErrPartial); ok {
-			// ignore unknown charge rate value error
-			if e.Current == nil &&
-				e.Design == nil &&
-				e.DesignVoltage == nil &&
-				e.Full == nil &&
-				e.State == nil &&
-				e.Voltage == nil &&
-				e.ChargeRate != nil &&
-				e.ChargeRate.Error() == unknownChargeRate {
-				return true
-			}
-		}
-		return false
-	}
-	if batErr, ok := err.(battery.Errors); ok {
-		for _, err := range batErr {
-			if !ignoreErr(err) {
-				fatalErr = append(fatalErr, err)
-			}
-		}
-	}
-
-	// when battery info fails to get retrieved but there is at least one valid battery, return it without error
-	if len(validBatteries) > 0 && fatalErr != nil && strings.Contains(fatalErr.Error(), unableToRetrieveBatteryInfo) {
-		return validBatteries, nil
-	}
-	// another error occurred (possibly unmapped use-case), return it
-	if fatalErr != nil {
-		env.log(Error, "BatteryInfo", fatalErr.Error())
-		return nil, fatalErr
-	}
-	// everything is fine
-	return validBatteries, nil
-}
-
 func (env *ShellEnvironment) Shell() string {
 	defer env.trace(time.Now(), "Shell")
 	if env.CmdFlags.Shell != "" {
@@ -661,6 +606,13 @@ func (env *ShellEnvironment) HTTPRequest(targetURL string, timeout int, requestM
 		env.log(Error, "HTTPRequest", err.Error())
 		return nil, err
 	}
+	// anything inside the range [200, 299] is considered a success
+	if response.StatusCode >= 200 && response.StatusCode < 300 {
+		message := "HTTP status code " + strconv.Itoa(response.StatusCode)
+		err := errors.New(message)
+		env.log(Error, "HTTPRequest", message)
+		return nil, err
+	}
 	defer response.Body.Close()
 	body, err := ioutil.ReadAll(response.Body)
 	if err != nil {
diff --git a/src/environment/windows_unix.go b/src/environment/windows_unix.go
new file mode 100644
index 00000000..3f8e5aeb
--- /dev/null
+++ b/src/environment/windows_unix.go
@@ -0,0 +1,106 @@
+//go:build !darwin
+
+package environment
+
+import (
+	"math"
+	"strings"
+	"time"
+
+	"github.com/distatus/battery"
+)
+
+func mapMostLogicalState(currentState, newState battery.State) battery.State {
+	switch currentState {
+	case battery.Discharging, battery.NotCharging:
+		return battery.Discharging
+	case battery.Empty:
+		return newState
+	case battery.Charging:
+		if newState == battery.Discharging {
+			return battery.Discharging
+		}
+		return battery.Charging
+	case battery.Unknown:
+		return newState
+	case battery.Full:
+		return newState
+	}
+	return newState
+}
+
+func (env *ShellEnvironment) BatteryState() (*BatteryInfo, error) {
+	defer env.trace(time.Now(), "BatteryInfo")
+
+	parseBatteryInfo := func(batteries []*battery.Battery) *BatteryInfo {
+		var info BatteryInfo
+		var current, total float64
+		var state battery.State
+		for _, bt := range batteries {
+			current += bt.Current
+			total += bt.Full
+			state = mapMostLogicalState(state, bt.State)
+		}
+		batteryPercentage := current / total * 100
+		info.Percentage = int(math.Min(100, batteryPercentage))
+		info.State = state
+		return &info
+	}
+
+	batteries, err := battery.GetAll()
+	// actual error, return it
+	if err != nil && len(batteries) == 0 {
+		env.log(Error, "BatteryInfo", err.Error())
+		return nil, err
+	}
+	// there are no batteries found
+	if len(batteries) == 0 {
+		return nil, &NoBatteryError{}
+	}
+	// some batteries fail to get retrieved, filter them out if present
+	validBatteries := []*battery.Battery{}
+	for _, batt := range batteries {
+		if batt != nil {
+			validBatteries = append(validBatteries, batt)
+		}
+	}
+	// clean minor errors
+	unableToRetrieveBatteryInfo := "A device which does not exist was specified."
+	unknownChargeRate := "Unknown value received"
+	var fatalErr battery.Errors
+	ignoreErr := func(err error) bool {
+		if e, ok := err.(battery.ErrPartial); ok {
+			// ignore unknown charge rate value error
+			if e.Current == nil &&
+				e.Design == nil &&
+				e.DesignVoltage == nil &&
+				e.Full == nil &&
+				e.State == nil &&
+				e.Voltage == nil &&
+				e.ChargeRate != nil &&
+				e.ChargeRate.Error() == unknownChargeRate {
+				return true
+			}
+		}
+		return false
+	}
+	if batErr, ok := err.(battery.Errors); ok {
+		for _, err := range batErr {
+			if !ignoreErr(err) {
+				fatalErr = append(fatalErr, err)
+			}
+		}
+	}
+
+	// when battery info fails to get retrieved but there is at least one valid battery, return it without error
+	if len(validBatteries) > 0 && fatalErr != nil && strings.Contains(fatalErr.Error(), unableToRetrieveBatteryInfo) {
+		return parseBatteryInfo(validBatteries), nil
+	}
+	// another error occurred (possibly unmapped use-case), return it
+	if fatalErr != nil {
+		env.log(Error, "BatteryInfo", fatalErr.Error())
+		return nil, fatalErr
+	}
+	// everything is fine
+	return parseBatteryInfo(validBatteries), nil
+}
diff --git a/src/mock/environment.go b/src/mock/environment.go
index 2aa15c52..f95119fa 100644
--- a/src/mock/environment.go
+++ b/src/mock/environment.go
@@ -4,7 +4,6 @@ import (
 	"io/fs"
 	"oh-my-posh/environment"
 
-	"github.com/distatus/battery"
 	mock "github.com/stretchr/testify/mock"
 )
 
@@ -122,9 +121,9 @@ func (env *MockedEnvironment) Flags() *environment.Flags {
 	return arguments.Get(0).(*environment.Flags)
 }
 
-func (env *MockedEnvironment) BatteryInfo() ([]*battery.Battery, error) {
+func (env *MockedEnvironment) BatteryState() (*environment.BatteryInfo, error) {
 	args := env.Called()
-	return args.Get(0).([]*battery.Battery), args.Error(1)
+	return args.Get(0).(*environment.BatteryInfo), args.Error(1)
 }
 
 func (env *MockedEnvironment) Shell() string {
diff --git a/src/segments/battery.go b/src/segments/battery.go
index 39bbe356..9326cdb9 100644
--- a/src/segments/battery.go
+++ b/src/segments/battery.go
@@ -1,7 +1,6 @@
 package segments
 
 import (
-	"math"
 	"oh-my-posh/environment"
 	"oh-my-posh/properties"
 
@@ -12,10 +11,9 @@ type Battery struct {
 	props properties.Properties
 	env   environment.Environment
 
-	battery.Battery
-	Percentage int
-	Error      string
-	Icon       string
+	*environment.BatteryInfo
+	Error string
+	Icon  string
 }
 
 const (
@@ -37,26 +35,19 @@ func (b *Battery) Enabled() bool {
 		return false
 	}
 
-	batteries, err := b.env.BatteryInfo()
+	var err error
+	b.BatteryInfo, err = b.env.BatteryState()
 
 	if !b.enabledWhileError(err) {
 		return false
 	}
 
 	// case on computer without batteries(no error, empty array)
-	if err == nil && len(batteries) == 0 {
+	if err == nil && b.BatteryInfo == nil {
 		return false
 	}
 
-	for _, bt := range batteries {
-		b.Battery.Current += bt.Current
-		b.Battery.Full += bt.Full
-		b.Battery.State = b.mapMostLogicalState(b.Battery.State, bt.State)
-	}
-	batteryPercentage := b.Battery.Current / b.Battery.Full * 100
-	b.Percentage = int(math.Min(100, batteryPercentage))
-
-	switch b.Battery.State {
+	switch b.BatteryInfo.State {
 	case battery.Discharging, battery.NotCharging:
 		b.Icon = b.props.GetString(DischargingIcon, "")
 	case battery.Charging:
@@ -85,31 +76,11 @@ func (b *Battery) enabledWhileError(err error) bool {
 	// This hack ensures we display a fully charged battery, even if
 	// that state can be incorrect. It's better to "ignore" the error
 	// than to not display the segment at all as that will confuse users.
-	b.Battery.Current = 100
-	b.Battery.Full = 10
-	b.Battery.State = battery.Full
+	b.Percentage = 100
+	b.State = battery.Full
 	return true
 }
 
-func (b *Battery) mapMostLogicalState(currentState, newState battery.State) battery.State {
-	switch currentState {
-	case battery.Discharging, battery.NotCharging:
-		return battery.Discharging
-	case battery.Empty:
-		return newState
-	case battery.Charging:
-		if newState == battery.Discharging {
-			return battery.Discharging
-		}
-		return battery.Charging
-	case battery.Unknown:
-		return newState
-	case battery.Full:
-		return newState
-	}
-	return newState
-}
-
 func (b *Battery) Init(props properties.Properties, env environment.Environment) {
 	b.props = props
 	b.env = env


Test patch:
diff --git a/src/environment/darwin_test.go b/src/environment/darwin_test.go
new file mode 100644
index 00000000..ca3ddd60
--- /dev/null
+++ b/src/environment/darwin_test.go
@@ -0,0 +1,61 @@
+//go:build darwin
+
+package environment
+
+import (
+	"testing"
+
+	"github.com/distatus/battery"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestParseBatteryOutput(t *testing.T) {
+	cases := []struct {
+		Case               string
+		Output             string
+		ExpectedState      battery.State
+		ExpectedPercentage int
+		ExpectError        bool
+	}{
+		{
+			Case:               "charging",
+			Output:             "99%; charging;",
+			ExpectedState:      battery.Charging,
+			ExpectedPercentage: 99,
+		},
+		{
+			Case:               "charging 1%",
+			Output:             "1%; charging;",
+			ExpectedState:      battery.Charging,
+			ExpectedPercentage: 1,
+		},
+		{
+			Case:               "not charging 80%",
+			Output:             "81%; AC attached;",
+			ExpectedState:      battery.NotCharging,
+			ExpectedPercentage: 81,
+		},
+		{
+			Case:               "charged",
+			Output:             "100%; charged;",
+			ExpectedState:      battery.Full,
+			ExpectedPercentage: 100,
+		},
+		{
+			Case:               "discharging",
+			Output:             "100%; discharging;",
+			ExpectedState:      battery.Discharging,
+			ExpectedPercentage: 100,
+		},
+	}
+	for _, tc := range cases {
+		env := ShellEnvironment{}
+		info, err := env.parseBatteryOutput(tc.Output)
+		if tc.ExpectError {
+			assert.Error(t, err, tc.Case)
+			return
+		}
+		assert.Equal(t, tc.ExpectedState, info.State, tc.Case)
+		assert.Equal(t, tc.ExpectedPercentage, info.Percentage, tc.Case)
+	}
+}
diff --git a/src/segments/battery_test.go b/src/environment/windows_unix_test.go
similarity index 90%
rename from src/segments/battery_test.go
rename to src/environment/windows_unix_test.go
index 59719340..1b278cf1 100644
--- a/src/segments/battery_test.go
+++ b/src/environment/windows_unix_test.go
@@ -1,4 +1,6 @@
-package segments
+//go:build !darwin
+
+package environment
 
 import (
 	"testing"
@@ -24,7 +26,6 @@ func TestMapBatteriesState(t *testing.T) {
 		{Case: "discharging > empty", ExpectedState: battery.Discharging, CurrentState: battery.Empty, NewState: battery.Discharging},
 	}
 	for _, tc := range cases {
-		batt := &Battery{}
-		assert.Equal(t, tc.ExpectedState, batt.mapMostLogicalState(tc.CurrentState, tc.NewState), tc.Case)
+		assert.Equal(t, tc.ExpectedState, mapMostLogicalState(tc.CurrentState, tc.NewState), tc.Case)
 	}
 }