fix(config): apply remote parser review fixes
Signed-off-by: brice <b@bnema.dev>
- commit
- a8061954f7691425fcdd0aecceb8a0b69d0b365e
- author
- brice · · 2026-07-30 08:18 UTC
- parent
- db35bc9
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +39 -15
M internal/adapters/config/config_test.go +13 -1
| @@ -29,6 +29,7 @@ func TestParse(t *testing.T) { | |||
| 29 | 29 | input string | |
| 30 | 30 | want domain.Config | |
| 31 | 31 | wantWarnings []domain.Warning | |
| 32 | + | checkRemote bool | |
| 32 | 33 | }{ | |
| 33 | 34 | { | |
| 34 | 35 | name: "comments and inline comments", | |
| @@ -51,6 +52,17 @@ func TestParse(t *testing.T) { | |||
| 51 | 52 | Codes: map[string]string{"detach": "dt"}, | |
| 52 | 53 | }, | |
| 53 | 54 | }, | |
| 55 | + | { | |
| 56 | + | name: "remote can disable enabled and remember", | |
| 57 | + | input: "[remote]\nenabled = false\nremember = false\n", | |
| 58 | + | want: domain.Config{ | |
| 59 | + | Theme: domain.ThemeAuto, | |
| 60 | + | BindingEntries: []domain.ConfigEntry{}, | |
| 61 | + | Codes: map[string]string{}, | |
| 62 | + | Remote: domain.RemoteConfig{Enabled: false, Remember: false}, | |
| 63 | + | }, | |
| 64 | + | checkRemote: true, | |
| 65 | + | }, | |
| 54 | 66 | { | |
| 55 | 67 | name: "invalid theme warns and keeps default", | |
| 56 | 68 | input: "theme = blue\n", | |
| @@ -358,7 +370,7 @@ func TestParse(t *testing.T) { | |||
| 358 | 370 | if !tt.want.Palette.AnchorSet { | |
| 359 | 371 | tt.want.Palette = domain.Defaults().Palette | |
| 360 | 372 | } | |
| 361 | - | if tt.want.Remote == (domain.RemoteConfig{}) { | |
| 373 | + | if !tt.checkRemote { | |
| 362 | 374 | tt.want.Remote = domain.Defaults().Remote | |
| 363 | 375 | } | |
| 364 | 376 | got, warnings, err := Parse(strings.NewReader(tt.input)) | |
| @@ -29,6 +29,7 @@ func TestParse(t *testing.T) { | |||||
| 29 | input string | 29 | input string | ||
| 30 | want domain.Config | 30 | want domain.Config | ||
| 31 | wantWarnings []domain.Warning | 31 | wantWarnings []domain.Warning | ||
| 32 | + | checkRemote bool | |||
| 32 | }{ | 33 | }{ | ||
| 33 | { | 34 | { | ||
| 34 | name: "comments and inline comments", | 35 | name: "comments and inline comments", | ||
| @@ -51,6 +52,17 @@ func TestParse(t *testing.T) { | |||||
| 51 | Codes: map[string]string{"detach": "dt"}, | 52 | Codes: map[string]string{"detach": "dt"}, | ||
| 52 | }, | 53 | }, | ||
| 53 | }, | 54 | }, | ||
| 55 | + | { | |||
| 56 | + | name: "remote can disable enabled and remember", | |||
| 57 | + | input: "[remote]\nenabled = false\nremember = false\n", | |||
| 58 | + | want: domain.Config{ | |||
| 59 | + | Theme: domain.ThemeAuto, | |||
| 60 | + | BindingEntries: []domain.ConfigEntry{}, | |||
| 61 | + | Codes: map[string]string{}, | |||
| 62 | + | Remote: domain.RemoteConfig{Enabled: false, Remember: false}, | |||
| 63 | + | }, | |||
| 64 | + | checkRemote: true, | |||
| 65 | + | }, | |||
| 54 | { | 66 | { | ||
| 55 | name: "invalid theme warns and keeps default", | 67 | name: "invalid theme warns and keeps default", | ||
| 56 | input: "theme = blue\n", | 68 | input: "theme = blue\n", | ||
| @@ -358,7 +370,7 @@ func TestParse(t *testing.T) { | |||||
| 358 | if !tt.want.Palette.AnchorSet { | 370 | if !tt.want.Palette.AnchorSet { | ||
| 359 | tt.want.Palette = domain.Defaults().Palette | 371 | tt.want.Palette = domain.Defaults().Palette | ||
| 360 | } | 372 | } | ||
| 361 | - | if tt.want.Remote == (domain.RemoteConfig{}) { | 373 | + | if !tt.checkRemote { |
| 362 | tt.want.Remote = domain.Defaults().Remote | 374 | tt.want.Remote = domain.Defaults().Remote | ||
| 363 | } | 375 | } | ||
| 364 | got, warnings, err := Parse(strings.NewReader(tt.input)) | 376 | got, warnings, err := Parse(strings.NewReader(tt.input)) | ||
M internal/adapters/config/remote.go +10 -10
| @@ -1,26 +1,21 @@ | |||
| 1 | 1 | package config | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "errors" | |
| 4 | 5 | "fmt" | |
| 5 | 6 | "strings" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/bnema/vev/internal/domain" | |
| 8 | 9 | ) | |
| 9 | 10 | ||
| 11 | + | // ErrUnsupportedRemoteHosts indicates a legacy remote hosts assignment. | |
| 12 | + | var ErrUnsupportedRemoteHosts = errors.New("unsupported remote hosts assignment") | |
| 13 | + | ||
| 10 | 14 | func parseRemoteKey(remote domain.RemoteConfig, seen map[string]bool, key, value string, lineNo int) (domain.RemoteConfig, []domain.Warning, error) { | |
| 11 | 15 | var warnings []domain.Warning | |
| 12 | - | switch key { | |
| 13 | - | case "enabled", "remember": | |
| 14 | - | warnings = warnDuplicateKey(warnings, seen, key, lineNo) | |
| 15 | - | case "hosts": | |
| 16 | - | return remote, warnings, fmt.Errorf("line %d: unsupported remote hosts assignment", lineNo) | |
| 17 | - | default: | |
| 18 | - | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("unknown key %q", key)}) | |
| 19 | - | return remote, warnings, nil | |
| 20 | - | } | |
| 21 | - | ||
| 22 | 16 | switch key { | |
| 23 | 17 | case "enabled": | |
| 18 | + | warnings = warnDuplicateKey(warnings, seen, key, lineNo) | |
| 24 | 19 | on, ok := parseTrueFalse(value) | |
| 25 | 20 | if !ok { | |
| 26 | 21 | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("invalid enabled %q", value)}) | |
| @@ -28,12 +23,17 @@ func parseRemoteKey(remote domain.RemoteConfig, seen map[string]bool, key, value | |||
| 28 | 23 | } | |
| 29 | 24 | remote.Enabled = on | |
| 30 | 25 | case "remember": | |
| 26 | + | warnings = warnDuplicateKey(warnings, seen, key, lineNo) | |
| 31 | 27 | on, ok := parseTrueFalse(value) | |
| 32 | 28 | if !ok { | |
| 33 | 29 | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("invalid remember %q", value)}) | |
| 34 | 30 | return remote, warnings, nil | |
| 35 | 31 | } | |
| 36 | 32 | remote.Remember = on | |
| 33 | + | case "hosts": | |
| 34 | + | return remote, warnings, fmt.Errorf("line %d: %w", lineNo, ErrUnsupportedRemoteHosts) | |
| 35 | + | default: | |
| 36 | + | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("unknown key %q", key)}) | |
| 37 | 37 | } | |
| 38 | 38 | return remote, warnings, nil | |
| 39 | 39 | } | |
| @@ -1,26 +1,21 @@ | |||||
| 1 | package config | 1 | package config | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "errors" | |||
| 4 | "fmt" | 5 | "fmt" | ||
| 5 | "strings" | 6 | "strings" | ||
| 6 | 7 | ||||
| 7 | "github.com/bnema/vev/internal/domain" | 8 | "github.com/bnema/vev/internal/domain" | ||
| 8 | ) | 9 | ) | ||
| 9 | 10 | ||||
| 11 | + | // ErrUnsupportedRemoteHosts indicates a legacy remote hosts assignment. | |||
| 12 | + | var ErrUnsupportedRemoteHosts = errors.New("unsupported remote hosts assignment") | |||
| 13 | + | ||||
| 10 | func parseRemoteKey(remote domain.RemoteConfig, seen map[string]bool, key, value string, lineNo int) (domain.RemoteConfig, []domain.Warning, error) { | 14 | func parseRemoteKey(remote domain.RemoteConfig, seen map[string]bool, key, value string, lineNo int) (domain.RemoteConfig, []domain.Warning, error) { | ||
| 11 | var warnings []domain.Warning | 15 | var warnings []domain.Warning | ||
| 12 | - | switch key { | |||
| 13 | - | case "enabled", "remember": | |||
| 14 | - | warnings = warnDuplicateKey(warnings, seen, key, lineNo) | |||
| 15 | - | case "hosts": | |||
| 16 | - | return remote, warnings, fmt.Errorf("line %d: unsupported remote hosts assignment", lineNo) | |||
| 17 | - | default: | |||
| 18 | - | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("unknown key %q", key)}) | |||
| 19 | - | return remote, warnings, nil | |||
| 20 | - | } | |||
| 21 | - | ||||
| 22 | switch key { | 16 | switch key { | ||
| 23 | case "enabled": | 17 | case "enabled": | ||
| 18 | + | warnings = warnDuplicateKey(warnings, seen, key, lineNo) | |||
| 24 | on, ok := parseTrueFalse(value) | 19 | on, ok := parseTrueFalse(value) | ||
| 25 | if !ok { | 20 | if !ok { | ||
| 26 | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("invalid enabled %q", value)}) | 21 | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("invalid enabled %q", value)}) | ||
| @@ -28,12 +23,17 @@ func parseRemoteKey(remote domain.RemoteConfig, seen map[string]bool, key, value | |||||
| 28 | } | 23 | } | ||
| 29 | remote.Enabled = on | 24 | remote.Enabled = on | ||
| 30 | case "remember": | 25 | case "remember": | ||
| 26 | + | warnings = warnDuplicateKey(warnings, seen, key, lineNo) | |||
| 31 | on, ok := parseTrueFalse(value) | 27 | on, ok := parseTrueFalse(value) | ||
| 32 | if !ok { | 28 | if !ok { | ||
| 33 | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("invalid remember %q", value)}) | 29 | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("invalid remember %q", value)}) | ||
| 34 | return remote, warnings, nil | 30 | return remote, warnings, nil | ||
| 35 | } | 31 | } | ||
| 36 | remote.Remember = on | 32 | remote.Remember = on | ||
| 33 | + | case "hosts": | |||
| 34 | + | return remote, warnings, fmt.Errorf("line %d: %w", lineNo, ErrUnsupportedRemoteHosts) | |||
| 35 | + | default: | |||
| 36 | + | warnings = append(warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("unknown key %q", key)}) | |||
| 37 | } | 37 | } | ||
| 38 | return remote, warnings, nil | 38 | return remote, warnings, nil | ||
| 39 | } | 39 | } | ||
M internal/adapters/config/remote_test.go +16 -4
| @@ -16,6 +16,7 @@ func TestParseRemote(t *testing.T) { | |||
| 16 | 16 | input string | |
| 17 | 17 | want domain.RemoteConfig | |
| 18 | 18 | wantErr string | |
| 19 | + | wantErrIs error | |
| 19 | 20 | wantWarnings []domain.Warning | |
| 20 | 21 | wantTheme domain.ThemeMode | |
| 21 | 22 | checkTheme bool | |
| @@ -94,21 +95,27 @@ remember = True | |||
| 94 | 95 | input: `[remote] | |
| 95 | 96 | hosts = ["arch", "build@mule"] | |
| 96 | 97 | `, | |
| 97 | - | wantErr: `line 2: unsupported remote hosts assignment`, | |
| 98 | + | wantErr: `line 2: unsupported remote hosts assignment`, | |
| 99 | + | wantErrIs: ErrUnsupportedRemoteHosts, | |
| 100 | + | wantWarnings: nil, | |
| 98 | 101 | }, | |
| 99 | 102 | { | |
| 100 | 103 | name: "empty hosts array fails closed", | |
| 101 | 104 | input: `[remote] | |
| 102 | 105 | hosts = [] | |
| 103 | 106 | `, | |
| 104 | - | wantErr: `line 2: unsupported remote hosts assignment`, | |
| 107 | + | wantErr: `line 2: unsupported remote hosts assignment`, | |
| 108 | + | wantErrIs: ErrUnsupportedRemoteHosts, | |
| 109 | + | wantWarnings: nil, | |
| 105 | 110 | }, | |
| 106 | 111 | { | |
| 107 | 112 | name: "malformed hosts assignment fails closed", | |
| 108 | 113 | input: `[remote] | |
| 109 | 114 | hosts = not-an-array | |
| 110 | 115 | `, | |
| 111 | - | wantErr: `line 2: unsupported remote hosts assignment`, | |
| 116 | + | wantErr: `line 2: unsupported remote hosts assignment`, | |
| 117 | + | wantErrIs: ErrUnsupportedRemoteHosts, | |
| 118 | + | wantWarnings: nil, | |
| 112 | 119 | }, | |
| 113 | 120 | { | |
| 114 | 121 | name: "hosts cannot fall through as a binding", | |
| @@ -117,7 +124,9 @@ hosts = not-an-array | |||
| 117 | 124 | hosts = ["arch"] | |
| 118 | 125 | new-tab = alt+t | |
| 119 | 126 | `, | |
| 120 | - | wantErr: `line 3: unsupported remote hosts assignment`, | |
| 127 | + | wantErr: `line 3: unsupported remote hosts assignment`, | |
| 128 | + | wantErrIs: ErrUnsupportedRemoteHosts, | |
| 129 | + | wantWarnings: nil, | |
| 121 | 130 | }, | |
| 122 | 131 | { | |
| 123 | 132 | name: "unknown remote key warns", | |
| @@ -167,6 +176,9 @@ enabled = true | |||
| 167 | 176 | cfg, warnings, err := Parse(strings.NewReader(tt.input)) | |
| 168 | 177 | if tt.wantErr != "" { | |
| 169 | 178 | require.EqualError(t, err, tt.wantErr) | |
| 179 | + | require.ErrorIs(t, err, tt.wantErrIs) | |
| 180 | + | require.Equal(t, tt.wantWarnings, warnings) | |
| 181 | + | require.Empty(t, cfg.BindingEntries) | |
| 170 | 182 | return | |
| 171 | 183 | } | |
| 172 | 184 | require.NoError(t, err) | |
| @@ -16,6 +16,7 @@ func TestParseRemote(t *testing.T) { | |||||
| 16 | input string | 16 | input string | ||
| 17 | want domain.RemoteConfig | 17 | want domain.RemoteConfig | ||
| 18 | wantErr string | 18 | wantErr string | ||
| 19 | + | wantErrIs error | |||
| 19 | wantWarnings []domain.Warning | 20 | wantWarnings []domain.Warning | ||
| 20 | wantTheme domain.ThemeMode | 21 | wantTheme domain.ThemeMode | ||
| 21 | checkTheme bool | 22 | checkTheme bool | ||
| @@ -94,21 +95,27 @@ remember = True | |||||
| 94 | input: `[remote] | 95 | input: `[remote] | ||
| 95 | hosts = ["arch", "build@mule"] | 96 | hosts = ["arch", "build@mule"] | ||
| 96 | `, | 97 | `, | ||
| 97 | - | wantErr: `line 2: unsupported remote hosts assignment`, | 98 | + | wantErr: `line 2: unsupported remote hosts assignment`, |
| 99 | + | wantErrIs: ErrUnsupportedRemoteHosts, | |||
| 100 | + | wantWarnings: nil, | |||
| 98 | }, | 101 | }, | ||
| 99 | { | 102 | { | ||
| 100 | name: "empty hosts array fails closed", | 103 | name: "empty hosts array fails closed", | ||
| 101 | input: `[remote] | 104 | input: `[remote] | ||
| 102 | hosts = [] | 105 | hosts = [] | ||
| 103 | `, | 106 | `, | ||
| 104 | - | wantErr: `line 2: unsupported remote hosts assignment`, | 107 | + | wantErr: `line 2: unsupported remote hosts assignment`, |
| 108 | + | wantErrIs: ErrUnsupportedRemoteHosts, | |||
| 109 | + | wantWarnings: nil, | |||
| 105 | }, | 110 | }, | ||
| 106 | { | 111 | { | ||
| 107 | name: "malformed hosts assignment fails closed", | 112 | name: "malformed hosts assignment fails closed", | ||
| 108 | input: `[remote] | 113 | input: `[remote] | ||
| 109 | hosts = not-an-array | 114 | hosts = not-an-array | ||
| 110 | `, | 115 | `, | ||
| 111 | - | wantErr: `line 2: unsupported remote hosts assignment`, | 116 | + | wantErr: `line 2: unsupported remote hosts assignment`, |
| 117 | + | wantErrIs: ErrUnsupportedRemoteHosts, | |||
| 118 | + | wantWarnings: nil, | |||
| 112 | }, | 119 | }, | ||
| 113 | { | 120 | { | ||
| 114 | name: "hosts cannot fall through as a binding", | 121 | name: "hosts cannot fall through as a binding", | ||
| @@ -117,7 +124,9 @@ hosts = not-an-array | |||||
| 117 | hosts = ["arch"] | 124 | hosts = ["arch"] | ||
| 118 | new-tab = alt+t | 125 | new-tab = alt+t | ||
| 119 | `, | 126 | `, | ||
| 120 | - | wantErr: `line 3: unsupported remote hosts assignment`, | 127 | + | wantErr: `line 3: unsupported remote hosts assignment`, |
| 128 | + | wantErrIs: ErrUnsupportedRemoteHosts, | |||
| 129 | + | wantWarnings: nil, | |||
| 121 | }, | 130 | }, | ||
| 122 | { | 131 | { | ||
| 123 | name: "unknown remote key warns", | 132 | name: "unknown remote key warns", | ||
| @@ -167,6 +176,9 @@ enabled = true | |||||
| 167 | cfg, warnings, err := Parse(strings.NewReader(tt.input)) | 176 | cfg, warnings, err := Parse(strings.NewReader(tt.input)) | ||
| 168 | if tt.wantErr != "" { | 177 | if tt.wantErr != "" { | ||
| 169 | require.EqualError(t, err, tt.wantErr) | 178 | require.EqualError(t, err, tt.wantErr) | ||
| 179 | + | require.ErrorIs(t, err, tt.wantErrIs) | |||
| 180 | + | require.Equal(t, tt.wantWarnings, warnings) | |||
| 181 | + | require.Empty(t, cfg.BindingEntries) | |||
| 170 | return | 182 | return | ||
| 171 | } | 183 | } | ||
| 172 | require.NoError(t, err) | 184 | require.NoError(t, err) | ||