Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Windows paths for --dirty option #150

Merged
merged 1 commit into from
Feb 17, 2023
Merged

Conversation

Rigby90
Copy link
Contributor

@Rigby90 Rigby90 commented Feb 16, 2023

Currently the --dirty option does not work on Windows, no files are processed regardless of their status within git.

This appears to be caused by the paths being returned by the git status process using different slashes than than those returned from the ConfigurationFactory::finder() which results in the array_intersect returning an empty array.

Git Status Results

C:\...\project-path>git status --short -- *.php
 M app/Models/ModelA.php
 M app/Models/ModelB.php
 M app/Models/ModelC.php

Before Changes

Array of Files from the Git Status Process Output -

array:16 [
  0 => "<project path>\app/Models/ModelA.php"
  1 => "<project path>\app/Models/ModelB.php"
  2 => "<project path>\app/Models/ModelC.php"
	...
]

Array of Files returned by the finder (To show slashes) -

array:10 [
  0 => "<project path>\app\Actions\Fortify\CreateNewUser.php"
  1 => "<project path>\app\Actions\Fortify\PasswordValidationRules.php"
  2 => "<project path>\app\Actions\Fortify\ResetUserPassword.php"
	...
]

Array of intersected values -

[]

After Changes

Array of Files from the Git Status Process Output -

array:16 [
  0 => "<project path>\app\Models\ModelA.php"
  1 => "<project path>\app\Models\ModelB.php"
  2 => "<project path>\app\Models\ModelC.php"
  ...
]

Array of Files returned by the finder -

array:10 [
  0 => "<project path>\app\Actions\Fortify\CreateNewUser.php"
  1 => "<project path>\app\Actions\Fortify\PasswordValidationRules.php"
  2 => "<project path>\app\Actions\Fortify\ResetUserPassword.php"
	...
]

Array of intersected values -

array:16 [
  0 => "<project path>\app\Models\ModelA.php"
  1 => "<project path>\app\Models\ModelB.php"
  2 => "<project path>\app\Models\ModelC.php"
  ...
]

I'm not sold on this solution in terms of whether to change the slashes on the git status result or the results of the finder; however in either case it's a similar fix to one which was achieved in Vapor CLI from several years ago laravel/vapor-cli#10

@nunomaduro nunomaduro merged commit cc54af0 into laravel:main Feb 17, 2023
@nunomaduro
Copy link
Member

@Rigby90 Can you pull "dev-main" and check if the issue is now working for you?

@Rigby90
Copy link
Contributor Author

Rigby90 commented Feb 17, 2023

Appears to be working -

git status --short -- *.php
?? app/Models/ModelA.php

Before -

pint --dirty


  ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Laravel
    PASS   ........................................................................................................................................ 0 files

After -

composer require laravel/pint:dev-main
./composer.json has been updated
Running composer update laravel/pint
Loading composer repositories with package information
Updating dependencies
Lock file operations: 0 installs, 1 update, 0 removals
  - Upgrading laravel/pint (v1.5.0 => dev-main 17d74f9)

...
pint --dirty

  ✓

  ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Laravel
    FIXED   ....................................................................................................................................................................... 1 file, 1 style issue fixed
  ✓ app\Models\ModelA.php                                                                                                                                                                                braces

Thank you!

@nunomaduro
Copy link
Member

Great news!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants