Remove any trailing slash from application name #368
Conversation
|
This test case may not be possible in other environments as they have different behavior for Might be easiest to remove this test case. But I will leave it in case the reviewer (on Mac) wishes to run it locally to verify my change first. |
| @@ -159,7 +159,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int | |||
| $this->validateDatabaseOption($input); | |||
| $this->validateStackOption($input); | |||
|
|
|||
| $name = $input->getArgument('name'); | |||
| $name = mb_rtrim($input->getArgument('name'), '/\\'); | |||
There was a problem hiding this comment.
This change breaks the minimum required PHP version. This function is introduced on PHP 8.4
There was a problem hiding this comment.
Symfony has a "polyfill" which adds native PHP functions like this to older versions. Tests pass for me locally running PHP 8.3 on Mac.
Did you experience an error or just review the code statically and see this function?
There was a problem hiding this comment.
This function is safe to be used since Laravel Framework requires ext-mbstring in it's own composer.json
#373 PR will address this further by throwing an exception if it detects PHP doesn't have the required extensions.
This PR addresses two, minor issues.
First, due to a logic bug when testing for Herd or Valet, a default
.testextension could never be reached. This ultimately leads toAPP_URLalways beinghttp://localhostinstead of a potentially validhttp://app-name.test.Second, if a developer appends a trailing slash to the name everything is installed and the application runs. However, all tests will fail until the developer realizes the
APP_URLwas set tohttp://app-name/.test. While an arguably an incorrect use case, it's a pretty easy fix to avoid such an annoying experience.