Enabling implicit appbase (dnx . run -> dnx run)
#2263
Conversation
| bool DirectoryExists(const char* path) | ||
| { | ||
| struct stat s; | ||
| return stat(path, &s) == 0 && S_ISDIR(s.st_mode); |
There was a problem hiding this comment.
Need to verify this manually.
|
Where are the mono changes? |
|
Mono will be done separately. This is already too big... |
|
I thought we weren't going to do the folder check and only support filenames (replacing the command, effectively). Selecting a different folder would only be supported by |
| } | ||
|
|
||
| bool LastPathSeparatorIndex(LPCTSTR pszPath, size_t* pIndex) | ||
| int split_path(const dnx::char_t* path) |
There was a problem hiding this comment.
Not a fan of this function name
There was a problem hiding this comment.
Couldn't come up with a better name but if you have a proposal let me know... I am also still considering changing it to return two std::strings - directory and filename - these buffer operations are so fragile.
I thought that was the plan? VS uses the full --appbase expansion. The only thing we need to support is |
|
Even better - I did not like going to the disk to check if the folder exists. |
|
Updated the PR so that we no longer check if an argument is a folder except for |
|
(Travis fails because the fix for Mono is separate - #2269 - I will push them together or can squash once reviewed) |
|
⌚ |
| ppszExpandedArgv[i] = new TCHAR[MAX_PATH]; | ||
| // "dnx /path/App.dll arg1" --> "dnx --appbase /path/ /path/App.dll arg1" | ||
| // "dnx /path/App.exe arg1" --> "dnx --appbase /path/ /path/App.exe arg1" | ||
| // "dnx App.exe arg1" --> "dnx --appbase --appbase . App.exe arg1" |
There was a problem hiding this comment.
Yeah. I will fix this.
|
|
- I don't know what cc57457 to
40e0620
Compare
|
@anurse Wasn't that part of the suggestion? |
Before providing appbase folder was necessary. Now we don't expect a folder anymore but assume that the current folder is the appbase folder. The only exception is where for backwards compatibility we continue to recognize `.` (so `dnx . run` continues to work). If the user really wants to provide a folder that is not the current folder they need to use the --appbase parameter. Fixes #1403
The user no longer has to provide folder (or `.`) when running dnx commands. Note that there might be an ambiguity between a folder and command which is resolved by checking if a folder with the provided name physically exists and if it does the parameter is treated as folder path otherwise it is treated as a command. Fixes #1403
|
Yes but |
|
@anurse Can't be. If |
|
My original proposal was to always use |
|
If we always expanded as you propose how would you do |
Before providing appbase folder was necessary. Now if the appbase folder is not provided we will use the current folder (
.) automatically.Note that there might be an ambiguity between a folder and command which is resolved by checking if a folder with the provided name physically exists and if it does the parameter is treated as folder path otherwise it is treated as a command.
Fixes #1403