Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Expand '~/' in AbsPath #52
Conversation
jameinel
approved these changes
Jun 22, 2017
It would be good to understand why, but the functionality is sound.
| func (ctx *Context) AbsPath(path string) string { | ||
| + if len(path) >= 2 && path[:2] == "~/" { |
jameinel
Jun 22, 2017
Owner
You can use "strings.HasPrefix"
If we're doing this, what about ~user/ and what about Windows, etc?
Typically ~ is expanded by the shell rather than the commands themselves.
Was this for a specific bug. Because it does mean that you can't quote args to not treat ~ as $HOME, because bash will take the quote and then pass it to us and we'll still expand it.
Probably still ok, but it would be good to understand why ~ expansion wasn't working in someone's shell.
axw
Jun 22, 2017
Member
juju/utils.NormalizePath handles both ~/foo and ~user/foo. We should probably use that?
https://github.com/juju/utils/blob/master/file.go#L42
| - c.Assert(ctx.AbsPath("foo/bar"), gc.Equals, filepath.Join(ctx.Dir, "foo/bar")) | ||
| + c.Check(ctx.AbsPath("/foo/bar"), gc.Equals, "/foo/bar") | ||
| + c.Check(ctx.AbsPath("foo/bar"), gc.Equals, filepath.Join(ctx.Dir, "foo/bar")) | ||
| + homeDir := os.Getenv("HOME") |
jameinel
Jun 22, 2017
Owner
I also don't think this is a safe test on Windows, though maybe we're skipping these already? HOME certainly isn't guaranteed to exist there.
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju-cmd |
wupeka commentedJun 22, 2017
No description provided.