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

Fix variable substitution matching #102

Closed

Conversation

MaxGfeller
Copy link

Previously it would only match if the name for a variable was at least two characters long, due to the \w and . identifiers. This solution will also match dots that could be in the identifier - although i'm not sure if that's correct.

@davidpelayo
Copy link

Related with #104?

@MaxGfeller
Copy link
Author

@davidpelayo Looks like it! Does this solution work for you? I only realized that we're getting problems with variables that were only one character long.

@tikiatua
Copy link
Member

Hi @MaxGfeller

Thank you for the pull request. We actually put the word-character matcher there on purpose as it seems somewhat strange to have key identifiers that start with a character outside the range of [A-Za-z0-9_].

It will also more in line with the previous matching pattern, therefore hopefully not breaking to many things.

@MaxGfeller
Copy link
Author

@tikiatua That makes sense. But this new rule actually sets a new requirement of two characters for a variable, which seems strange to me.

Many of our variables are just single-digit numbers (as generated so by vue-translation-manager and those don't work anymore.

@tikiatua
Copy link
Member

Very good point. I will adapt the regex matcher to also match single characters that match [A-Za-z0-9_]

@MaxGfeller
Copy link
Author

Sweet, thank you! And merry christmas to you and your family :)

@tikiatua
Copy link
Member

Should be incorporated v1.10.9. However the regex is adapted slightly different, to only allow word characters (\w) if the length of the replacement is only one character, i.e. {0}

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.

3 participants