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

Fix #774: Allow object methods to be used as extraction-keywords #1136

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

embray
Copy link

@embray embray commented Oct 3, 2024

Proof of concept fix for #774. I did this quickly using peekable() from the more-itertools package, but if adding a new dependency just for this is (understandably) undesirable, it could be fixed just as easily with slight improvements to the generate tokens loop (in fact I had a version of that working earlier, but it gets a bit hairier with the nested case).

This would conflict with #1127, so if this is otherwise acceptable as a feature can rebase on top of that and without using more-itertools.

this is just a quick proof-of-concept; could also be done without adding
more-itertools as a new dependency
@akx
Copy link
Member

akx commented Oct 19, 2024

Thank you! Yeah, this makes sense, but we definitely don't want an extra dependency :)

Maybe we can vendor (a subset of) peekable if that makes implementing this easier. Tests should also ensure multi-dotted names such as my.corporate.app.translations.execute() work :)

@embray
Copy link
Author

embray commented Oct 19, 2024

Great, given the go-ahead I'll rework this. peekable() is nice but I don't think it's really needed either.

I like how your example pokes fun at the "Enterprise-y" nature of this ;). But I swear I had a really good use case for this and wanted to make it work.

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

Successfully merging this pull request may close these issues.

2 participants