Skip to content

Conversation

dvdmgl
Copy link

@dvdmgl dvdmgl commented Sep 18, 2025

This PR adds support for the Ntex framework.

* feat: add ntex support
maud/src/lib.rs Outdated
Comment on lines 374 to 378
if self.0.is_empty() {
Poll::Ready(None)
} else {
Poll::Ready(Some(Ok(Bytes::from(mem::take(&mut self.0).into_bytes()))))
}
Copy link
Owner

Choose a reason for hiding this comment

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

Can we forward to the implementation for String?

Suggested change
if self.0.is_empty() {
Poll::Ready(None)
} else {
Poll::Ready(Some(Ok(Bytes::from(mem::take(&mut self.0).into_bytes()))))
}
self.0.poll_next_chunk(context)

Copy link
Author

Choose a reason for hiding this comment

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

The implementation follows ntex impl MessageBody for String, avoiding unnecessary allocations and one-shot consumption of the body, performance wise it's the best.

use crate::PreEscaped;
use alloc::{rc::Rc, string::String};

impl MessageBody for PreEscaped<String> {
Copy link
Owner

Choose a reason for hiding this comment

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

Is implementing MessageBody necessary here, given that we implement Responder directly?

Copy link
Author

Choose a reason for hiding this comment

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

The MessageBody adds flexibility, allowing maud to be used anywhere the body it's accepted, not only in the Responder.

@lambda-fairy
Copy link
Owner

lambda-fairy commented Sep 28, 2025 via email

Copy link
Author

@dvdmgl dvdmgl left a comment

Choose a reason for hiding this comment

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

I'm more in favor to keep PR as-is, it's copy&paste of actix-web implementation to ntex's, making the transition between frameworks actix-web and ntex easier.

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