Fix regression in Kotlin error/exception names. #1842
Conversation
mhammond
force-pushed
the
issue/1828-error-enum-rename
branch
from
f1e700c
to
63d06e7
Compare
| @@ -401,43 +440,41 @@ pub mod filters { | |||
| use super::*; | |||
| pub use crate::backend::filters::*; | |||
|
|
|||
| pub fn type_name(as_ct: &impl AsCodeType) -> Result<String, askama::Error> { | |||
| pub(super) fn type_name(as_ct: &impl AsCodeType) -> Result<String, askama::Error> { | |||
There was a problem hiding this comment.
It's not a big deal either way, but I'd lean towards keeping these pub.
For one, it'll make the PR smaller, but I also generally prefer that functions are either public or private. If we don't want outside code to access these methods, then we can make gen_python a private module. If we need more complicated access control, we can selectively import its public functions into the parent module.
There was a problem hiding this comment.
Ah yeah - this was needed before I "unforked" otherwise I got errors about leaking private types. Note AsCodeType is again pub I can remove this for python!
There was a problem hiding this comment.
oops, I was wrong - I unforked that trait, but Python is still using the fork. Without this I get:
385 | trait AsCodeType {
| ---------------- `gen_python::AsCodeType` declared as private
...
443 | pub fn type_name(as_ct: &impl AsCodeType) -> Result<String, askama::Error> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't leak private trait
(which TBH I don't quite understand seeing as the filters module, while nominally pub isn't actually exported anywhere.)
There was a problem hiding this comment.
(and trying to make the trait itself pub then confuses even more things)
There was a problem hiding this comment.
Ahh I see, then pub(super) makes sense to me.
Fixes #1828.
I'm frustrated with myself that I left this so broken - I think I was trying to avoid this larger Kotlin refactor that is sadly necessary. I ended up forking
CodeTypeas only Kotlin needs error renaming.