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

Improve PhpStorm's auto-complete of "DriverInterface" method arguments #847

Merged
merged 3 commits into from
Feb 7, 2025

Conversation

uuf6429
Copy link
Member

@uuf6429 uuf6429 commented Jun 10, 2023

The changes are fully backword compatible.

@codecov
Copy link

codecov bot commented Jun 10, 2023

Codecov Report

Merging #847 (e96bbea) into master (2bec260) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #847   +/-   ##
=========================================
  Coverage     98.50%   98.50%           
  Complexity      345      345           
=========================================
  Files            23       23           
  Lines          1002     1002           
=========================================
  Hits            987      987           
  Misses           15       15           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@uuf6429 uuf6429 marked this pull request as ready for review June 10, 2023 14:05
src/Driver/DriverInterface.php Outdated Show resolved Hide resolved
src/Driver/DriverInterface.php Outdated Show resolved Hide resolved
src/Driver/DriverInterface.php Outdated Show resolved Hide resolved
@uuf6429 uuf6429 requested a review from stof November 2, 2024 13:53
@uuf6429
Copy link
Member Author

uuf6429 commented Feb 6, 2025

@stof / @aik099 any chance getting this PR merged?

Sadly, I keep ending up adding such attributes at the driver level, which only benefits the drivers themselves (and would be redundant with this PR merged).

@aik099 aik099 changed the title Improve DriverInterface Improve PhpStorm's auto-complete of "DriverInterface" interface methods Feb 7, 2025
@aik099 aik099 changed the title Improve PhpStorm's auto-complete of "DriverInterface" interface methods Improve PhpStorm's auto-complete of "DriverInterface" methods Feb 7, 2025
@aik099 aik099 changed the title Improve PhpStorm's auto-complete of "DriverInterface" methods Improve PhpStorm's auto-complete of "DriverInterface" method arguments Feb 7, 2025
@aik099 aik099 merged commit 916471d into minkphp:master Feb 7, 2025
12 checks passed
@aik099
Copy link
Member

aik099 commented Feb 7, 2025

Merging, thank you @uuf6429 .

Please explain the difference in working with DriverInterface with/without having these PhpStorm-specific attributes available.

@uuf6429 uuf6429 deleted the feature/improve-driverinterface branch February 7, 2025 10:11
@uuf6429
Copy link
Member Author

uuf6429 commented Feb 7, 2025

Please explain the difference in working with DriverInterface with/without having these PhpStorm-specific attributes available.

If you mean why those attributes are useful:

Participating IDEs* (mainly JetBrains) should be able to highlight such parameters, which improves developer experience for maintainers but also direct driver users.
image
(instead of a plain green text for the entire strings)

At the moment, it works for me already, because the IDE realises that I'm using the classic driver, which already has those attributes.

For driver maintainers, unless they add the attributes themselves, they also will not get that hinting. These attributes are useful for driver maintainers as well, since drivers tend to call such methods themselves.

Hope that answers your question, let me know if you meant something else.

*theoretically, anyone can make use of those attributes, since they're available as a composer package.

@stof
Copy link
Member

stof commented Feb 7, 2025

Projects should almost never use the driver directly though. They are expected to work with the high-level Mink API.

@uuf6429
Copy link
Member Author

uuf6429 commented Feb 7, 2025

@stof agreed, I believe there are other cases in mink where it makes sense to support that.

For example, in my current project, we developed our own selector/locator system with slightly better DX that is still based on mink:
image

(maybe at one point I'll make it into a mink PR)


In any case, I believe those attributes are low-effort/impact quick wins.

@aik099
Copy link
Member

aik099 commented Feb 7, 2025

@uuf6429 , at least you can create an issue in Mink with that idea. This way at it won't be forgotten.

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.

3 participants