chore: simplify context class #315
Conversation
| $this->verbosityLevel, | ||
| $this->name, | ||
| ); | ||
| return $this->with(currentDirectory: str_starts_with($path, '/') ? $path : PathHelper::realpath($this->currentDirectory . '/' . $path)); |
There was a problem hiding this comment.
I notice something I don't like.
withPath but the property is currentDirectory.
What about deprecating withPath to withCurrentDirectory?
(if okay, I'll open a new PR)
There was a problem hiding this comment.
Ok for renaming it, it bugged me a few times 馃憤
There was a problem hiding this comment.
The current name of the run method for defining the path is also path. Shouldn't we also make sure they're identical?
Why not something like workingPath or workingDirectory to indicate the working directory?
| $this->verbosityLevel, | ||
| $this->name, | ||
| ); | ||
| return $this->with(currentDirectory: str_starts_with($path, '/') ? $path : PathHelper::realpath($this->currentDirectory . '/' . $path)); |
There was a problem hiding this comment.
Ok for renaming it, it bugged me a few times 馃憤
| @@ -276,6 +142,23 @@ public function offsetUnset(mixed $offset): void | |||
| throw new \LogicException('Context is immutable.'); | |||
| } | |||
|
|
|||
| private function with(mixed ...$args): self | |||
There was a problem hiding this comment.
Did you consider putting all the parameters from the constructor here, with their type, but then nullable, with default null value?
It would still allow you to only use named parameters, but now you have type safety.
Instead of an array you reference the parameter directly and fallback to the original value using ??.
There was a problem hiding this comment.
Yes but did not succeed to do that. There is not way in PHP to check is null is passed or if it's the default value
There was a problem hiding this comment.
Ah, it doesn't work when a property is nullable. Forgot about that.
Too bad!
No description provided.