-
Notifications
You must be signed in to change notification settings - Fork 13
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
Unify asy conditionals and variables data structure #2568
base: version/0-40-0-RC1
Are you sure you want to change the base?
Changes from all commits
74c9db1
5aa013b
647cb7f
23bea1b
25ad33d
f377c1e
9fce069
5678f64
9b2ac3b
35fbde4
d105f38
b31ecb2
54c2876
45056f9
7f9addc
d51d1e2
b6f7f6c
8afd893
2918fd8
0dcce06
25a74f9
18438c3
3f2bb37
685a93e
09269dd
0c8f91c
ece577f
1b955de
7809313
cfd2ae6
a8891b5
0b9ae89
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ package constraints | |
import ( | ||
"bytes" | ||
"fmt" | ||
"path/filepath" | ||
"reflect" | ||
"regexp" | ||
"sort" | ||
"strings" | ||
|
@@ -12,27 +12,10 @@ import ( | |
"github.com/ActiveState/cli/internal/locale" | ||
"github.com/ActiveState/cli/internal/logging" | ||
"github.com/ActiveState/cli/internal/multilog" | ||
"github.com/ActiveState/cli/internal/rtutils/ptr" | ||
"github.com/ActiveState/cli/pkg/platform/authentication" | ||
"github.com/ActiveState/cli/pkg/projectfile" | ||
"github.com/ActiveState/cli/pkg/sysinfo" | ||
"github.com/thoas/go-funk" | ||
) | ||
|
||
var cache = make(map[string]interface{}) | ||
|
||
func getCache(key string, getter func() (interface{}, error)) (interface{}, error) { | ||
if v, ok := cache[key]; ok { | ||
return v, nil | ||
} | ||
v, err := getter() | ||
if err != nil { | ||
return nil, err | ||
} | ||
cache[key] = v | ||
return v, err | ||
} | ||
|
||
// For testing. | ||
var osOverride, osVersionOverride, archOverride, libcOverride, compilerOverride string | ||
|
||
|
@@ -41,22 +24,9 @@ type Conditional struct { | |
funcs template.FuncMap | ||
} | ||
|
||
func NewConditional(a *authentication.Auth) *Conditional { | ||
func NewConditional() *Conditional { | ||
c := &Conditional{map[string]interface{}{}, map[string]interface{}{}} | ||
|
||
c.RegisterFunc("Mixin", func() map[string]interface{} { | ||
res := map[string]string{ | ||
"Name": "", | ||
"Email": "", | ||
} | ||
if a.Authenticated() { | ||
res["Name"] = a.WhoAmI() | ||
res["Email"] = a.Email() | ||
} | ||
return map[string]interface{}{ | ||
"User": res, | ||
} | ||
}) | ||
c.RegisterFunc("Contains", funk.Contains) | ||
c.RegisterFunc("HasPrefix", strings.HasPrefix) | ||
c.RegisterFunc("HasSuffix", strings.HasSuffix) | ||
|
@@ -72,62 +42,36 @@ func NewConditional(a *authentication.Auth) *Conditional { | |
return c | ||
} | ||
|
||
type projectable interface { | ||
Owner() string | ||
Name() string | ||
NamespaceString() string | ||
CommitID() string | ||
BranchName() string | ||
Path() string | ||
URL() string | ||
} | ||
func NewPrimeConditional(structure interface{}) *Conditional { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is old code written 2 years ago that I simplified. |
||
c := NewConditional() | ||
|
||
func NewPrimeConditional(auth *authentication.Auth, pj projectable, subshellName string) *Conditional { | ||
var ( | ||
pjOwner string | ||
pjName string | ||
pjNamespace string | ||
pjURL string | ||
pjCommit string | ||
pjBranch string | ||
pjPath string | ||
) | ||
if !ptr.IsNil(pj) { | ||
pjOwner = pj.Owner() | ||
pjName = pj.Name() | ||
pjNamespace = pj.NamespaceString() | ||
pjURL = pj.URL() | ||
pjCommit = pj.CommitID() | ||
pjBranch = pj.BranchName() | ||
pjPath = pj.Path() | ||
if pjPath != "" { | ||
pjPath = filepath.Dir(pjPath) | ||
} | ||
v := reflect.ValueOf(structure) | ||
// deref if needed | ||
if v.Kind() == reflect.Ptr { | ||
v = v.Elem() | ||
} | ||
|
||
c := NewConditional(auth) | ||
c.RegisterParam("Project", map[string]string{ | ||
"Owner": pjOwner, | ||
"Name": pjName, | ||
"Namespace": pjNamespace, | ||
"Url": pjURL, | ||
"Commit": pjCommit, | ||
"Branch": pjBranch, | ||
"Path": pjPath, | ||
|
||
// Legacy | ||
"NamespacePrefix": pjNamespace, | ||
}) | ||
osVersion, err := sysinfo.OSVersion() | ||
if err != nil { | ||
multilog.Error("Could not detect OSVersion: %v", err) | ||
fields := reflect.VisibleFields(v.Type()) | ||
|
||
// Work at depth 1: Vars.[Struct].Struct.Simple | ||
for _, f := range fields { | ||
d1Val := v.FieldByIndex(f.Index) | ||
if d1Val.Kind() == reflect.Ptr { | ||
d1Val = d1Val.Elem() | ||
} | ||
|
||
// Only nodes at depth 1 need to be registered since the generic type | ||
// handling within the templating package will do the rest. If function | ||
// registration is needed at greater depths, this will need to be | ||
// reworked (and may not be possible without expansive refactoring). | ||
switch d1Val.Type().Kind() { | ||
case reflect.Func: | ||
c.RegisterFunc(f.Name, d1Val.Interface()) | ||
|
||
default: | ||
c.RegisterParam(f.Name, d1Val.Interface()) | ||
} | ||
} | ||
c.RegisterParam("OS", map[string]interface{}{ | ||
"Name": sysinfo.OS().String(), | ||
"Version": osVersion, | ||
"Architecture": sysinfo.Architecture().String(), | ||
}) | ||
c.RegisterParam("Shell", subshellName) | ||
|
||
return c | ||
} | ||
|
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.
Why do we need a separate constructor for vars? Is this notably slower or something? This feels very error prone as we will have instances of project initialized with vars and instances without, and we have to just know which is which.
I'd prefer we implement this in a way where this is always the case.
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.
This is done to not uproot existing usages that do not require Vars for this diff. I imagined a new story to cover making all project usages (more?) equal.