check diffusion locate file if works
Details
- Reviewers
valerio.bozzolan - Group Reviewers
O1: Blessed Committers - Commits
- rPf75b66b27a34: fix PHP8 array_slice($results, null) which cause diffusion locate file broken
Diff Detail
- Repository
- rP Phorge
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
I skiped the linting and unit testing because they take time to process. too much for this tiny change
Thanks. Tested 👍
Please evaluate the same change also here to fix the "Pattern Search"
diff --git a/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php index ba4c824061..5f40aa06bb 100644 --- a/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php @@ -40,7 +40,7 @@ final class DiffusionSearchQueryConduitAPIMethod throw $ex; } - $offset = $request->getValue('offset'); + $offset = $request->getValue('offset', 0); $results = array_slice($results, $offset); return $results;
I skiped the linting and unit testing because they take time to process. too much for this tiny change
Well, tests took 1 second on my laptop :D But yes: setting this up took 1 hour to me the first time... so I completely understand. Don't worry.
Tested! thanks 🌈
To land, run arc land
P.S.
Do you also have this error now?
strlen(): Passing null to parameter #1 ($string) of type string is deprecated at [DiffusionQueryPathsConduitAPIMethod.php:94]
To fix, probably we need to replace strlen() with phutil_nonempty_str in a couple of places, or, just cast to string. Something like this:
diff --git a/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php index e511492fc4..7f48133c30 100644 --- a/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php @@ -33,13 +33,13 @@ final class DiffusionQueryPathsConduitAPIMethod protected function getGitResult(ConduitAPIRequest $request) { $drequest = $this->getDiffusionRequest(); - $path = $drequest->getPath(); + $path = phutil_string_cast($drequest->getPath()); $commit = $request->getValue('commit'); $repository = $drequest->getRepository(); // Recent versions of Git don't work if you pass the empty string, and // require "." to list everything. - if (!strlen($path)) { + if ($path === '') { $path = '.'; } @@ -78,11 +78,11 @@ final class DiffusionQueryPathsConduitAPIMethod } protected function filterResults($lines, ConduitAPIRequest $request) { - $pattern = $request->getValue('pattern'); + $pattern = phutil_string_cast($request->getValue('pattern')); $limit = (int)$request->getValue('limit'); $offset = (int)$request->getValue('offset'); - if (strlen($pattern)) { + if ($pattern !== '') { // Add delimiters to the regex pattern. $pattern = '('.$pattern.')'; } @@ -90,7 +90,7 @@ final class DiffusionQueryPathsConduitAPIMethod $results = array(); $count = 0; foreach ($lines as $line) { - if (strlen($pattern) && !preg_match($pattern, $line)) { + if ($pattern !== '' && !preg_match($pattern, $line)) { continue; }
Thanks again. Feel free to land as-is.
Many thanks for your patience. you're very kind!
but I do have trouble landing this diff.
not sure what I'm doing wrong.
I think the troubleshooting could be inconvenient for both of us. could you make this diff on your own first?
even though I really want to make the contribution on my own. :[
it's actually arc land. but just with GIT_TRACE env variable to see what's going on deep down
You didn't have Trusted Contributors membership, which provides access to the repo. Try now