-
Notifications
You must be signed in to change notification settings - Fork 2k
fix: Don't strip underscores from variables #39121
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 file reviewed, no comments
|
Size Change: 0 B Total Size: 3.07 MB ℹ️ View Unchanged
|
|
awesome |
| // Filter out all characters that is not a letter, number or space | ||
| .replace(/[^a-zA-Z0-9\s]/g, '') | ||
| // Filter out all characters that is not a letter, number or space or underscore | ||
| .replace(/[^a-zA-Z0-9\s_]/g, '') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does https://github.com/PostHog/posthog/blob/master/posthog/api/insight_variable.py#L22-L24 need updating as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooops good catch
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
|
@Gilbert09 What's the failing e2e test about? Doesnt' look like this is related to my PR. Do I need a rebase? Edit: Asked around, seems unrelated, I'm gonna move this PR forward it's pretty stale. Please yell at me on Slack if I broke something so I can take credit ;) |
Problem
Variable names in sql editor get
_stripped out, BUT if you input a space, it gets turned into an_.This means
hello_worldturns intohelloworldbuthello worldsaves ashello_world.Changes
_hello worldandhello_worldgets mapped to{variable.hello_world}How did you test this code?
Manual testing
Screen.Recording.2025-10-03.at.2.35.19.PM.mov
Also tested in some queries
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Changelog: (features only) Is this feature complete?