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

(🎁) Add | operator on Locators #1882

Closed
wants to merge 1 commit into from

Conversation

KotlinIsland
Copy link

umm, based python moment????

@KotlinIsland
Copy link
Author

@microsoft-github-policy-service agree

@mxschmitt
Copy link
Member

I guess the same makes sense for and? Will discuss with the team later!

@KotlinIsland
Copy link
Author

@mxschmitt Do you mean and as in an operator for filter?

@mxschmitt
Copy link
Member

(Never mind, Locator.and(...) was something we were experimenting with and I got confused, we didn't ship it)

@mxschmitt
Copy link
Member

We decided for now to keep or instead of | since or also provides auto completion support and shows what it is actually doing. Thanks for you suggestion tho!

@mxschmitt mxschmitt closed this May 2, 2023
@KotlinIsland
Copy link
Author

KotlinIsland commented May 3, 2023

Workaround

from playwright.sync_api import Locator

Locator.__or__ = Locator.or_

Unfortunately usages of this will be an error in mypy 😢

@DetachHead
Copy link

@mxschmitt it seems silly to not accept this PR when the alternative is to append an underscore to the method name as a workaround for the fact that "or" causes a syntax error.

page.locator('div') | page.locator('span')

looks much less gross than

page.locator('div').or_(page.locator('span'))

python specifically supports this use case, why not take advantage of it?

@mxschmitt
Copy link
Member

Feel free to file a feature request for it. We can add it as an alternative syntax, but it requires multiple changes like upstream playwright docs updates, generator updates, and then the actual implementation like in this PR.

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