Skip to content
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

Support partially dynamic segments #509

Merged
merged 4 commits into from
May 31, 2023
Merged

Support partially dynamic segments #509

merged 4 commits into from
May 31, 2023

Conversation

brandoncc
Copy link
Contributor

What it is

Adds matching for dynamic portions of a segment. This is in addition to the current dynamic matching which matches only entire segments.

Why

Currently, "cats.tenderlove-bio.name" would be marked unused even if the source code contains t "cats.#{cat}-bio.name".

Closes #508

How

The library uses two special characters in regex matches right now: * and :. I combined these characters to create a "lazy match" similar the the way you combine .+ and ? to lazily match in standard regular expressions.

I created tests which verify that the partial match is only done in a single segment, rather than matching across segments.

Since there isn't currently a way to change which replacement string is used from ":", I changed the default value to the new "*:".

Questions

  • What are the repercussions of changing the default replacement value?
    • In particular, I don't know if this has any negative repercussions on this line and other lines that use this variable.
  • Is this likely to have any negative effects on existing users of the library?
    • I think it might be a good idea to introduce an "opt-in" option in the configuration so that existing users don't see a change in behavior. This option could become the default in a future version. Alternatively, this PR could be a breaking change and therefore warrant a major version bump according to semver.

Rather than book-ending the regex with the expectation one of ^/$/. on
each side, `*:` is a lazy matcher which matches only part of a segment.
Since there isn't a way to override or configure the dynamic segment
type used, the default has been updated to allow for partially-dynamic
segments.
@glebm glebm merged commit 4392d99 into glebm:main May 31, 2023
5 of 6 checks passed
@glebm
Copy link
Owner

glebm commented May 31, 2023

Thanks!

@brandoncc brandoncc deleted the support-partially-dynamic-segments branch June 1, 2023 19:00
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.

Partially-dynamic keys are reported as unused
2 participants