Repository navigation
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The focused change preserves existing formatting and adds regression coverage, with no identified blocking issues.
No open issues 🎉
What changed in this PR
Fixes regular-expression serialization in shell-bson-parser so flags are preserved.
Changes:
- Uses
RegExp.flagsinstead of collecting onlyg,i, andm. - Adds regression coverage for preserving
dgimsuy.
| File | Description |
|---|---|
| packages/shell-bson-parser/src/stringify.ts | Preserves all flags when serializing regular expressions. |
| packages/shell-bson-parser/src/stringify.spec.ts | Tests serialization with multiple flags. |
Review effort: Balanced
|
|
||
| return `RegExp(${JSON.stringify(v.source)}${hasOptions ? `, '${o}'` : ''})`; | ||
| const flags = v.flags; | ||
| return `RegExp(${JSON.stringify(v.source)}${flags ? `, '${flags}'` : ''})`; |
There was a problem hiding this comment.
g, i and m is the common overlap between JS regular expressions (which this function handles) and BSON regular expressions (which do not support other flags). Are we sure this is the right change?
There was a problem hiding this comment.
I am trying to figure out what the fix would look like in this case. We can handle this in parse as well and throw for invalid flags (which would be same as bson's allowed regex flags). It would still leave the issue as is in stringify, but we usually do that after parse inside compass.
There was a problem hiding this comment.
Can we articulate what the bug is that we're trying to fix here? Unfortunately, neither the JIRA ticket, nor the PR description, nor the commit message actually explain this
There was a problem hiding this comment.
IIRC it was within Compass querybar that a user would type regex and in saved list it showed without all the flags that typed. I am not able to get that state anymore within Compass and will close the ticket and PR.
Description
Open Questions
Checklist