Skip to content

Conversation

LilyFirefly
Copy link
Contributor

@LilyFirefly LilyFirefly commented Aug 1, 2025

Depends on #26.

  • Convert to PyO3's declarative module syntax
  • Replace .expect with raising a ValueError for malformed language
  • Move tuple unpacking into the for loop
  • Simplify setting FluentArgs by only doing the fallback in one place
  • Move errors Vec next to the bundle.format_pattern that uses it

@LilyFirefly LilyFirefly force-pushed the refactor-pyo3-usage branch from 3d9cadf to f196892 Compare August 1, 2025 11:51
@LilyFirefly LilyFirefly force-pushed the refactor-pyo3-usage branch from 855819c to e0e068d Compare August 4, 2025 08:16
@LilyFirefly LilyFirefly marked this pull request as ready for review August 4, 2025 08:18
@LilyFirefly LilyFirefly requested a review from a team as a code owner August 4, 2025 08:18
@LilyFirefly LilyFirefly self-assigned this Aug 4, 2025
@LilyFirefly LilyFirefly added the rust Pull requests that update rust code label Aug 4, 2025
Copy link
Collaborator

@seddonym seddonym left a comment

Choose a reason for hiding this comment

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

Lovely stuff!

with pytest.raises(ValueError) as exc_info:
fluent.Bundle("$", [])

assert str(exc_info.value) == "Invalid language: '$'"
Copy link
Collaborator

Choose a reason for hiding this comment

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

🐼 Could do this using with pytest.raises(Value, match="..."):.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could, but I don't really like it because then I have to deal with regexes and escaping. This way it's an exact match.


if let Some(variables) = variables {
for variable in variables {
for (python_key, python_value) in variables {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Much nicer. 😄

create_exception!(rustfluent, ParserError, pyo3::exceptions::PyException);

#[pymodule]
fn rustfluent(m: &Bound<'_, PyModule>) -> PyResult<()> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good shout, I much prefer the new syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, me too!

}
}
} else if python_value.is_instance_of::<PyInt>()
&& let Ok(int_value) = python_value.extract::<i32>()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't know you could do this, cool!

@LilyFirefly LilyFirefly merged commit 2634c56 into kraken-tech:main Aug 4, 2025
1 check passed
@LilyFirefly LilyFirefly deleted the refactor-pyo3-usage branch August 4, 2025 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rust Pull requests that update rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants