Do not check the existence of stdin-path when processing data from STDIN #1489
Conversation
| } | ||
|
|
||
| // Replace backslashes with forwardslashes. | ||
| $path = str_replace('\\', '/', $path); |
| // Replace backslashes with forwardslashes. | ||
| $path = str_replace('\\', '/', $path); | ||
| // Combine multiple slashes into a single slash. | ||
| $path = preg_replace('/\/+/', '/', $path); |
There was a problem hiding this comment.
you could canonicalize backslashes with this as well in one. Combining multiple slashes is much appreceated.
| { | ||
| // Don't prefix absolute paths. | ||
| if (substr($path, 0, 1) !== '/') { | ||
| $path = getcwd().'/'.$path; |
There was a problem hiding this comment.
getcwd will do a realpath (which might not be what you want according to the issue). You perhaps want to optionally inject that value as a parameter (which would allow to process on getenv("PWD") or some other OS-specific strategy to obtain the working directory).
| // Ignore parts that have no value. | ||
| if (empty($part) === true || $part === '.') continue; | ||
| if ($part !== '..') { | ||
| // Cool, we found a new part. |
There was a problem hiding this comment.
It's neither cool nor do we find anything here. Perhaps just $parts[] = $path;
| if ($part !== '..') { | ||
| // Cool, we found a new part. | ||
| array_push($parts, $part); | ||
| } else if (count($parts) > 0) { |
There was a problem hiding this comment.
You mean $part === '..' I guess ....
| // Going back up. | ||
| array_pop($parts); | ||
| } else { | ||
| // Path is outside of the root. |
There was a problem hiding this comment.
No it is not. It actually is root then.
| * | ||
| * @return mixed | ||
| */ | ||
| public static function realpath($path) | ||
| public static function realpath($path, $isStdin=false) |
There was a problem hiding this comment.
Optional helper parameter, consider to instead create a second function and call that second function when intended (functional decomposition instead of conditional code).
|
I don't like the idea of trying to expand references inside a fake file path like this. It also needs to be compatible with Windows paths and it doesn't look like it would be. I'm going to close this PR and attempt another solution, but I'll comment on the main issue. |
#1488
realpath is used to get canonicalized absolute pathname, whether to specify the file directly or to use STDIN with --stdin-path specified, and it will check the existence of the file.
In my opinion, it is not necessary to check the existence of stdin-path when processing data from STDIN. If you want, just specify the file instead of using STDIN.
This PR provides a new function to get canonicalized absolute pathname and not check existence. When processing data from STDIN with --stdin-path specified, use the new function.