New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: simplify context class #315
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,198 +58,64 @@ public function withData(array $data, bool $keepExisting = true, bool $recursive | |
|
||
if ($keepExisting) { | ||
if ($recursive) { | ||
/* @var array<(int|string), mixed> */ | ||
$data = $this->arrayMergeRecursiveDistinct($this->data, $data); | ||
} else { | ||
/* @var array<(int|string), mixed> */ | ||
$data = array_merge($this->data, $data); | ||
} | ||
} | ||
|
||
return new self( | ||
$data, | ||
$this->environment, | ||
$this->currentDirectory, | ||
$this->tty, | ||
$this->pty, | ||
$this->timeout, | ||
$this->quiet, | ||
$this->allowFailure, | ||
$this->notify, | ||
$this->verbosityLevel, | ||
$this->name, | ||
); | ||
return $this->with(data: $data); | ||
} | ||
|
||
/** @param array<string, string|\Stringable|int> $environment */ | ||
public function withEnvironment(array $environment, bool $keepExisting = true): self | ||
{ | ||
return new self( | ||
$this->data, | ||
$keepExisting ? [...$this->environment, ...$environment] : $environment, | ||
$this->currentDirectory, | ||
$this->tty, | ||
$this->pty, | ||
$this->timeout, | ||
$this->quiet, | ||
$this->allowFailure, | ||
$this->notify, | ||
$this->verbosityLevel, | ||
$this->name, | ||
); | ||
return $this->with(environment: $keepExisting ? [...$this->environment, ...$environment] : $environment); | ||
} | ||
|
||
public function withPath(string $path): self | ||
{ | ||
return new self( | ||
$this->data, | ||
$this->environment, | ||
str_starts_with($path, '/') ? $path : PathHelper::realpath($this->currentDirectory . '/' . $path), | ||
$this->tty, | ||
$this->pty, | ||
$this->timeout, | ||
$this->quiet, | ||
$this->allowFailure, | ||
$this->notify, | ||
$this->verbosityLevel, | ||
$this->name, | ||
); | ||
return $this->with(currentDirectory: str_starts_with($path, '/') ? $path : PathHelper::realpath($this->currentDirectory . '/' . $path)); | ||
} | ||
|
||
public function withTty(bool $tty = true): self | ||
{ | ||
return new self( | ||
$this->data, | ||
$this->environment, | ||
$this->currentDirectory, | ||
$tty, | ||
$this->pty, | ||
$this->timeout, | ||
$this->quiet, | ||
$this->allowFailure, | ||
$this->notify, | ||
$this->verbosityLevel, | ||
$this->name, | ||
); | ||
return $this->with(tty: $tty); | ||
} | ||
|
||
public function withPty(bool $pty = true): self | ||
{ | ||
return new self( | ||
$this->data, | ||
$this->environment, | ||
$this->currentDirectory, | ||
$this->tty, | ||
$pty, | ||
$this->timeout, | ||
$this->quiet, | ||
$this->allowFailure, | ||
$this->notify, | ||
$this->verbosityLevel, | ||
$this->name, | ||
); | ||
return $this->with(pty: $pty); | ||
} | ||
|
||
public function withTimeout(?float $timeout): self | ||
{ | ||
return new self( | ||
$this->data, | ||
$this->environment, | ||
$this->currentDirectory, | ||
$this->tty, | ||
$this->pty, | ||
$timeout, | ||
$this->quiet, | ||
$this->allowFailure, | ||
$this->notify, | ||
$this->verbosityLevel, | ||
$this->name, | ||
); | ||
return $this->with(timeout: $timeout); | ||
} | ||
|
||
public function withQuiet(bool $quiet = true): self | ||
{ | ||
return new self( | ||
$this->data, | ||
$this->environment, | ||
$this->currentDirectory, | ||
$this->tty, | ||
$this->pty, | ||
$this->timeout, | ||
$quiet, | ||
$this->allowFailure, | ||
$this->notify, | ||
$this->verbosityLevel, | ||
$this->name, | ||
); | ||
return $this->with(quiet: $quiet); | ||
} | ||
|
||
public function withAllowFailure(bool $allowFailure = true): self | ||
{ | ||
return new self( | ||
$this->data, | ||
$this->environment, | ||
$this->currentDirectory, | ||
$this->tty, | ||
$this->pty, | ||
$this->timeout, | ||
$this->quiet, | ||
$allowFailure, | ||
$this->notify, | ||
$this->verbosityLevel, | ||
$this->name, | ||
); | ||
return $this->with(allowFailure: $allowFailure); | ||
} | ||
|
||
public function withNotify(bool $notify = true): self | ||
{ | ||
return new self( | ||
$this->data, | ||
$this->environment, | ||
$this->currentDirectory, | ||
$this->tty, | ||
$this->pty, | ||
$this->timeout, | ||
$this->quiet, | ||
$this->allowFailure, | ||
$notify, | ||
$this->verbosityLevel, | ||
$this->name, | ||
); | ||
return $this->with(notify: $notify); | ||
} | ||
|
||
public function withVerbosityLevel(VerbosityLevel $verbosityLevel): self | ||
{ | ||
return new self( | ||
$this->data, | ||
$this->environment, | ||
$this->currentDirectory, | ||
$this->tty, | ||
$this->pty, | ||
$this->timeout, | ||
$this->quiet, | ||
$this->allowFailure, | ||
$this->notify, | ||
$verbosityLevel, | ||
$this->name, | ||
); | ||
return $this->with(verbosityLevel: $verbosityLevel); | ||
} | ||
|
||
public function withName(string $name): self | ||
{ | ||
return new self( | ||
$this->data, | ||
$this->environment, | ||
$this->currentDirectory, | ||
$this->tty, | ||
$this->pty, | ||
$this->timeout, | ||
$this->quiet, | ||
$this->allowFailure, | ||
$this->notify, | ||
$this->verbosityLevel, | ||
$name, | ||
); | ||
return $this->with(name: $name); | ||
} | ||
|
||
public function offsetExists(mixed $offset): bool | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, it doesn't work when a property is nullable. Forgot about that. Too bad! |
||
{ | ||
return new self( | ||
$args['data'] ?? $this->data, | ||
$args['environment'] ?? $this->environment, | ||
$args['currentDirectory'] ?? $this->currentDirectory, | ||
$args['tty'] ?? $this->tty, | ||
$args['pty'] ?? $this->pty, | ||
$args['timeout'] ?? $this->timeout, | ||
$args['quiet'] ?? $this->quiet, | ||
$args['allowFailure'] ?? $this->allowFailure, | ||
$args['notify'] ?? $this->notify, | ||
$args['verbosityLevel'] ?? $this->verbosityLevel, | ||
$args['name'] ?? $this->name, | ||
); | ||
} | ||
|
||
/** | ||
* @param array<(int|string), mixed> $array1 | ||
* @param array<(int|string), mixed> $array2 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice something I don't like.
withPath
but the property iscurrentDirectory
.What about deprecating
withPath
towithCurrentDirectory
?(if okay, I'll open a new PR)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok for renaming it, it bugged me a few times 馃憤
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current name of the
run
method for defining the path is alsopath
. Shouldn't we also make sure they're identical?Why not something like
workingPath
orworkingDirectory
to indicate the working directory?