search

Argument-Clinic / cpython

The Python programming language
https://www.python.org/
Other
1 stars 0 forks source link

[alt] Refactor `DSLParser.format_docstring()` #18

Closed AlexWaygood closed 1 year ago

AlexWaygood commented 1 year ago

Turn it into a method on Function objects

@erlend-aasland, what do you think of this approach, as an alternative to https://github.com/python/cpython/pull/107623 + #16? It gets rid of more code ;)

erlend-aasland commented 1 year ago

@erlend-aasland, what do you think of this approach, as an alternative to python#107623 + #16? It gets rid of more code ;)

AFAICS, this only takes both of those PRs a step further :) Makes sense to me. I think we could start with #16, then python/cpython#107623, and then apply the changes from this PR.

AlexWaygood commented 1 year ago

AFAICS, this only takes both of those PRs a step further :)

Yes, exactly!

I think we could start with #16, then python#107623, and then apply the changes from this PR.

Hmm... I guess we just disagree on process, then :)

For me, https://github.com/python/cpython/pull/107623 looks kinda odd as-is, because you're making all these new @staticmethods... which aren't really static at all! They're pretty stateful, it's just that it's not the DSLParser state that they're accessing and modifying -- it's the state of the DSLParser.function attribute that they're accessing and modifying. For me, the diff makes much more sense -- and is actually more readable -- if we do it all in one go, like this PR?

erlend-aasland commented 1 year ago

Hmm... I guess we just disagree on process, then :)

For me, python#107623 looks kinda odd as-is, because you're making all these new @staticmethods... which aren't really static at all! They're pretty stateful, it's just that it's not the DSLParser state that they're accessing and modifying -- it's the state of the DSLParser.function attribute that they're accessing and modifying. For me, the diff makes much more sense -- and is actually more readable -- if we do it all in one go, like this PR?

After Serhiy's @text_signature, there's only one @staticmethod left in that PR. I would like to break it up in multiple PRs, since it is easier to bisect in case a bug sneaks in.

AlexWaygood commented 1 year ago

See https://github.com/python/cpython/pull/107840