Properly Expand Appdata Directory #1508
Conversation
ukane-philemon
force-pushed
the
expandAppdataparameter
branch
from
d305913
to
270511b
Compare
There was a problem hiding this comment.
Have confirmed on Windows that at least %% is expanded to $HOME/AppData/Local. Using Windows 10 (I think) and powershell. I'm not sure how windows users use this but I assume %PLACE% makes shortcuts to various places. Changes look good to me except for removing the clean for the config file location, unless there is a reason I do not see?
| // Expand initial ~ to the current user's home directory, or ~otheruser | ||
| // to otheruser's home directory. On Windows, both forward and backward | ||
| // slashes can be used. | ||
| path = path[1:] | ||
|
|
||
| var pathSeparators string | ||
| if runtime.GOOS == "windows" { | ||
| pathSeparators = string(os.PathSeparator) + "/" | ||
| } else { | ||
| pathSeparators = string(os.PathSeparator) | ||
| } | ||
|
|
||
| userName := "" | ||
| if i := strings.IndexAny(path, pathSeparators); i != -1 { | ||
| userName = path[:i] | ||
| path = path[i:] | ||
| } | ||
|
|
||
| homeDir := "" | ||
| var u *user.User | ||
| var err error | ||
| if userName == "" { | ||
| u, err = user.Current() | ||
| } else { | ||
| u, err = user.Lookup(userName) | ||
| } | ||
| if err == nil { | ||
| homeDir = u.HomeDir | ||
| } | ||
| // Fallback to CWD if user lookup fails or user has no home directory. | ||
| if homeDir == "" { | ||
| homeDir = "." | ||
| dirName, err := os.UserHomeDir() | ||
| if err != nil { | ||
| // Fallback to CWD if retrieving user home directory fails. | ||
| dirName = "." | ||
| } |
There was a problem hiding this comment.
This does look like a better option than parsing the file name looking for a user name.
I guess we lose the ability to expand to ~otheruser? I can't imagine when that would be used though. Actually seems like something we should not encourage, so I like this change.
ukane-philemon
force-pushed
the
expandAppdataparameter
branch
from
270511b
to
90b22ad
Compare
| dirName := "" | ||
|
|
||
| // This supports Windows cmd.exe-style %VARIABLE%. | ||
| if strings.HasPrefix(path, "%") { |
There was a problem hiding this comment.
Only if os is windows.
Also, why prefix? What if user is doing something like C:\dexdata\%USERNAME%?
There was a problem hiding this comment.
This was intended to expand %variable%\path
%USERNAME% will eventually expand to a full path C:\users\username. Currently, C:\dexdata\%USERNAME% will be handled by os.ExpandEnv. Will provide support for such cases.
| // This supports Windows cmd.exe-style %VARIABLE%. | ||
| if strings.HasPrefix(path, "%") { | ||
| // Split path into %VARIABLE% and path | ||
| pathArray := strings.SplitAfterN(path, "/", 2) |
There was a problem hiding this comment.
What about "\" as a path separator?
The filepath package has methods and consts for this purpose.
|
Hard to justify this change without any previous complaints or requests for it. Little changes like this can end up breaking stuff. |
|
Considering the helpful comments from @chappjc, @buck54321, @JoeGruffins, I would mark this PR closed pending request to support expanding windows %VARIBLE% in paths. #1525 is open to fixing expanding dcrwallet rpc cert path. |
This diff properly expands the
~to user home directory, provides support for windows%VARIABLE%style when expanding appdata dir and expands dcr rpcwallet cert path. Closes #436