Skip to content

[UNTESTED] Add support for renaming to kebap-case - #175

Merged
jcrist merged 2 commits into
msgspec:mainfrom
ljrk0:pr-kebap-case
Aug 30, 2022
Merged

[UNTESTED] Add support for renaming to kebap-case#175
jcrist merged 2 commits into
msgspec:mainfrom
ljrk0:pr-kebap-case

Conversation

@ljrk0

@ljrk0 ljrk0 commented Aug 26, 2022

Copy link
Copy Markdown
Contributor

Hi,

this should implement builtin renaming to kebap-case. I didn't test the code due to me virtually never working with Python and less with its C API, so it'd be good if someone else tested it. Also, I only added one test case where I saw fit/understood the testing harness, more could be added but I wouldn't know how.

@jcrist jcrist left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! This mostly looks good to me, I can finish up the few final tweaks needed.

I didn't test the code due to me virtually never working with Python and less with its C API, so it'd be good if someone else tested it.

Out of curiosity - if you never work with Python and didn't test this feature locally, is this a feature you needed yourself? Or just something you noticed was missing?

Comment thread msgspec/__init__.pyi Outdated
tag_field: Union[None, str] = None,
rename: Union[
None, Literal["lower", "upper", "camel", "pascal"], Callable[[str], str]
None, Literal["lower", "upper", "kebap", "camel", "pascal"], Callable[[str], str]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL some people spell this "kebap" instead of "kebab". Since other tools like serde use the "kebab" spelling (https://serde.rs/container-attrs.html#rename_all), I think we should go with that one.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it seems like kebap is a bit more common in Germany as this is the spelling I see at my local Döner Kebap :-)

But I have absolutely nothing against naming it kebab, especially if this is more common

@ljrk0

ljrk0 commented Aug 29, 2022

Copy link
Copy Markdown
Contributor Author

Thanks for the PR! This mostly looks good to me, I can finish up the few final tweaks needed.

Great! Thanks for your work on maintaining this piece of software!

I didn't test the code due to me virtually never working with Python and less with its C API, so it'd be good if someone else tested it.

Out of curiosity - if you never work with Python and didn't test this feature locally, is this a feature you needed yourself? Or just something you noticed was missing?

I simply helped out a colleague who was struggling to find the right place in the API and defined them a simple kebap_rename=lambda s: s.replace('_', '-') for their usage, however, since a built-in was missing, I decided to quickly hack on it (they simply used the python implementation though) :-)

- Rename "kebap" -> "kebab"
- Strip leading and trailing `_`
@jcrist
jcrist merged commit 0f699d2 into msgspec:main Aug 30, 2022
@jcrist

jcrist commented Aug 30, 2022

Copy link
Copy Markdown
Member

Thanks!

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.

2 participants