Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new slugify helper to mq’s built-in module and wires it into the existing mq-level builtin test suite.
Changes:
- Add
slugify(s, separator = "-")tocrates/mq-lang/builtin.mqusingdowncase()+gsub()normalization. - Add
test_slugifyand register it in therun_testslist incrates/mq-lang/builtin_tests.mq.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/mq-lang/builtin.mq | Introduces slugify function implemented via regex-based substitutions. |
| crates/mq-lang/builtin_tests.mq | Adds/registrations mq-level tests for slugify’s default behavior. |
| end | ||
| end | ||
|
|
||
| # Converts a string into a URL-friendly slug by lowercasing, replacing non-alphanumeric characters with hyphens, and trimming hyphens from the ends. |
There was a problem hiding this comment.
The doc comment says the function replaces non-alphanumeric characters with hyphens, but the function signature allows a custom separator. Either document that the separator is configurable (and clarify ASCII vs Unicode behavior, since the regex is [a-z0-9]) or adjust the implementation/doc to match the intended semantics.
| # Converts a string into a URL-friendly slug by lowercasing, replacing non-alphanumeric characters with hyphens, and trimming hyphens from the ends. | |
| # Converts a string into a URL-friendly slug by lowercasing, replacing runs of non-ASCII-alphanumeric | |
| # characters with the given `separator` (default "-"), and trimming leading/trailing hyphens. |
| def test_slugify(): | ||
| let result1 = slugify("Hello World!") | ||
| | assert_eq(result1, "hello-world") | ||
|
|
||
| | let result2 = slugify("This is a test.") | ||
| | assert_eq(result2, "this-is-a-test") | ||
|
|
||
| | let result3 = slugify("Special characters: @#$%^&*()") | ||
| | assert_eq(result3, "special-characters") | ||
| end |
There was a problem hiding this comment.
Tests cover the default slugify behavior, but there’s no coverage for the newly added separator parameter (and its edge cases like leading/trailing separators). Add at least one test that passes a non-"-" separator and asserts the expected trimming/collapsing behavior.
| # Converts a string into a URL-friendly slug by lowercasing, replacing non-alphanumeric characters with hyphens, and trimming hyphens from the ends. | ||
| def slugify(s, separator = "-"): | ||
| s | ||
| | downcase() | ||
| | gsub("[^a-z0-9]+", separator) | ||
| | gsub("^-+|-+$", "") |
There was a problem hiding this comment.
slugify exposes a separator parameter, but the trimming step hard-codes hyphens (gsub("^-+|-+$", "")). If callers pass a different separator (e.g. _), leading/trailing separators won’t be removed, and the doc comment also implies hyphens regardless of the parameter. Consider either removing the separator parameter or updating the implementation to trim the chosen separator (and avoid regex-replacement surprises for characters like $ in the separator).
| # Converts a string into a URL-friendly slug by lowercasing, replacing non-alphanumeric characters with hyphens, and trimming hyphens from the ends. | |
| def slugify(s, separator = "-"): | |
| s | |
| | downcase() | |
| | gsub("[^a-z0-9]+", separator) | |
| | gsub("^-+|-+$", "") | |
| # Converts a string into a URL-friendly slug by lowercasing, replacing non-alphanumeric characters with the given separator, and trimming that separator from the ends. | |
| def slugify(s, separator = "-"): | |
| s | |
| | downcase() | |
| | gsub("[^a-z0-9]+", separator) | |
| | split(separator) | |
| | map(select(. != "")) | |
| | join(separator) |
No description provided.