Skip to content

feat: a bit more useful NgxStr #157

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

Merged
merged 5 commits into from
Jun 2, 2025
Merged

Conversation

bavshin-f5
Copy link
Member

Prerequisite for the allocator-api work, but useful on its own.

  • nginx_sys::detail could be another crate (nginx-common?), but there's just not enough code to split it now.
  • no fancy fmt::Debug for ngx_str_t. Need to figure out how to override bindgen generated trait implementation without nuking Debug impls from most of the bindings. Builder::no_debug() does exactly that -- omits Debug on the ngx_str_t itself and on any structs that include ngx_str_t recursively.
  • I attempted to implement precision and padding for display_bytes, but apparently precision requires a honest UTF-8 character width detection algorithm (unicode-width). The code without that can panic on an attempt to break an UTF-8 sequence. Maybe next time.
  • Conversions and comparisons might be excessive. I took an inspiration from the bstr and left only types we realistically expect to use.
    I wish I could replace NgxStr with bstr, but it does not offer allocators support :(

Copy link
Contributor

@xeioex xeioex left a comment

Choose a reason for hiding this comment

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

nginx_sys::detail could be another crate (nginx-common?), but there's just not enough code to split it now.

Agree.

Overall looks good to me.

From implementations for NgxStr allowed to create a reference that lives
longer than the original object.
fmt::LowerHex was applying the inherited Formatter options, resulting in
the following incorrect output:
```
assertion `left == right` failed
  left: "\\x          f0\\x          90\\x          80"
 right: "\\xf0\\x90\\x80"
```
@bavshin-f5 bavshin-f5 merged commit 27c495b into nginx:master Jun 2, 2025
14 checks passed
@bavshin-f5 bavshin-f5 deleted the strings branch June 2, 2025 22:09
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