Variable handling #6
Conversation
task.go
Outdated
| if output, err = runCommand(ReplaceVariables(c, vars), ReplaceVariables(t.Dir, vars)); err != nil { | ||
| return &taskRunError{name, err} | ||
| } | ||
| fmt.Println(output) |
There was a problem hiding this comment.
Build error by fmt not being imported.
There was a problem hiding this comment.
Also, maybe should not print output if command set a variable? Not sure, just an idea.
if t.Set != "" {
os.Setenv(t.Set, output)
} else {
fmt.Println(output)
}There was a problem hiding this comment.
Thought about a quiet switch, but that would imply more, I will change code accoring to your suggestion
There was a problem hiding this comment.
You have already...
| @@ -0,0 +1,82 @@ | |||
| package task | |||
There was a problem hiding this comment.
Convention for filenames in Go is using underscores.
variableHandling.go -> variable_handling.go
variableHandling.go
Outdated
| "strings" | ||
|
|
||
| "github.com/BurntSushi/toml" | ||
| yaml "gopkg.in/yaml.v2" |
There was a problem hiding this comment.
No need to alias package name.
task.go
Outdated
| } | ||
| } | ||
|
|
||
| if !Force && isTaskUpToDate(t) { |
There was a problem hiding this comment.
This should run after running deps. See c655f23
There was a problem hiding this comment.
@andreynering Why running dependencies for a task when the task itself would not run? That was at least my assumption, as you would create side-effects that possible could change what the task would generate itself. I create an issue for discussion and commit a change running deps first.
| func ReplaceVariables(initial string, variables map[string]string) string { | ||
| replaced := initial | ||
| for name, val := range variables { | ||
| replaced = strings.Replace(replaced, fmt.Sprintf("{{%s}}", name), val, -1) |
There was a problem hiding this comment.
I think I'll replace this with Go's template engine after merge.
So we can have some variables assigned by default (like OS, Architecture, etc). And will allow users to program if's and range's. Example:
acmd:
cmds:
- {{if eq .GOOS "windows"}}windows-command{{else}}unix-command{{end}}But don't change that in this PR, otherwise it'll become too big.
variableHandling.go
Outdated
|
|
||
| var ( | ||
| // VariableFilePath file containing additional variables | ||
| VariableFilePath = "Variables" |
| build: | ||
| deps: [setvar] | ||
| cmds: | ||
| - echo "{{prefix}} '{{THEVAR}}'" |
There was a problem hiding this comment.
I think prefix should be PREFIX. Uppercase should be a convention for variables, just like environment variables.
There was a problem hiding this comment.
Shouldn't this be a choice of the user instead of the tool?
There was a problem hiding this comment.
It is, user can call it the name they want. But I changed to uppercase as a documentation convention.
| setvar: | ||
| cmds: | ||
| - echo "{{PATH}}" | ||
| set: THEVAR |
There was a problem hiding this comment.
set is the property with the variable name. I searched for a valid name of the most simplistic task to set a variable.
| build: | ||
| deps: [setvar] | ||
| cmds: | ||
| - echo "{{prefix}} '{{THEVAR}}'" |
There was a problem hiding this comment.
It is, user can call it the name they want. But I changed to uppercase as a documentation convention.
| Sources []string | ||
| Generates []string | ||
| Dir string | ||
| Variables map[string]string |
There was a problem hiding this comment.
I changed this to Vars for brevity.
3 participants
Use of variables ( task local, variable file and environment ) in Taskfiles