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

Allow slashes properly within escape blocks #567

Merged
merged 8 commits into from
Apr 1, 2024

Conversation

Hoeksema
Copy link
Contributor

closes #566

The path parsing currently doesn't work properly when there are embedded slashes within an ignore block.

This PR fixes this issue:

  • No more exceptions thrown when using // within an escaped block
  • Allowing multiple / to occur within an escape block without breakage

Before, the individual segments between slashes in addition to the entire escaped block were returned by PathInfo. Now, it returns just the latter, which is correct. All existing unit tests still pass and new tests were added to exercise the failing cases in #566.

@Hoeksema Hoeksema marked this pull request as ready for review February 14, 2024 15:59
Copy link

@Hoeksema
Copy link
Contributor Author

Hoeksema commented Feb 19, 2024

Hey @oformaniuk @StefH, hope you're having a great week! Any chance you could review this?

We're looking to keep our code base unforked from origin as much as possible, also so we can keep sending pull requests for issues we find as we productionize this in our system, and need this fix in particular for a critical use case of ours.


EDIT (Mar 7): Did not see any activity here, so we went ahead with forking unfortunately. No need to complete this soon anymore.

@oformaniuk
Copy link
Member

@Hoeksema, unfortunately, I was not following the repo closely lately. If you (and your team) is interested in contributing into the project further I'd be happy to include you as contributors.

@oformaniuk oformaniuk enabled auto-merge April 1, 2024 01:26
Copy link

sonarqubecloud bot commented Apr 1, 2024

@oformaniuk oformaniuk merged commit 9fc63f8 into Handlebars-Net:master Apr 1, 2024
7 checks passed
@oformaniuk oformaniuk added the bug label Apr 1, 2024
@Hoeksema
Copy link
Contributor Author

Hoeksema commented Apr 1, 2024

@Hoeksema, unfortunately, I was not following the repo closely lately. If you (and your team) is interested in contributing into the project further I'd be happy to include you as contributors.

Thank you for merging the PRs! Feel free to add me as contributor if that feels appropriate. We might need to make some more bug fixes in a quarter or two once we use it more broadly.

@Hoeksema Hoeksema deleted the patch-1 branch April 1, 2024 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consecutive slashes and more than 1 slash within square brackets does not escape properly
2 participants