Expand formatter coverage#675
Conversation
…ix test of vector_norm test, manual addition of createAlias/createDb handling + testing
🦋 Changeset detectedLatest commit: f93aec7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
… into fixFormatting
Add handling of showCommandYieldWhere like existing showCommandYield Expand auth rule testing
OskarDamkjaer
left a comment
There was a problem hiding this comment.
Formatting changes good to go, the claude changes would be nice to pair on or take in another PR
| *.npmrc | ||
| benchmarks.txt | ||
| .idea/ | ||
| .claude/ |
There was a problem hiding this comment.
Git-ignoring this one but commiting CLAUDE.md?
There was a problem hiding this comment.
IMO it could be interesting to add some skills/etc to the repo, for new contributors, or for automated code review? Let's discuss later in our 121?
There was a problem hiding this comment.
Like we said in 1-2-1 I agree. Will clean up this PR when it's ready for a full review
| @@ -0,0 +1,109 @@ | |||
| # CLAUDE.md | |||
There was a problem hiding this comment.
I think a shared claude.md would be a good idea, but I think it'd be nice to collaborate on it. I found some of the best practises from anthropic counter intuitive:
https://code.claude.com/docs/en/best-practices#write-an-effective-claude-md
They say to exclude for example anything Claude can figure out by reading code.
|
|
||
| test('show indexes with where and return', () => { | ||
| const query = `SHOW INDEXES YIELD aaaaaa, aaaaaa, aaaaaa, aaaaaa, aaaaaa | ||
| const query = `SHOW INDEXES |
There was a problem hiding this comment.
How did you find the queries we fail to format? I'm thinking it could be nice if we could find them automatically? Perhaps we could extract samples from the main repos tests?
There was a problem hiding this comment.
I agree, ive discussed this with some surface people before, and they mentioned some .feature files that often contain queries with new syntax. I've made a card here
anderson4j
left a comment
There was a problem hiding this comment.
Here are some replies to your comments. I still have a few months of changes to the grammar left, so I'll fix that first before a re-review
|
|
||
| test('show indexes with where and return', () => { | ||
| const query = `SHOW INDEXES YIELD aaaaaa, aaaaaa, aaaaaa, aaaaaa, aaaaaa | ||
| const query = `SHOW INDEXES |
There was a problem hiding this comment.
I agree, ive discussed this with some surface people before, and they mentioned some .feature files that often contain queries with new syntax. I've made a card here
| *.npmrc | ||
| benchmarks.txt | ||
| .idea/ | ||
| .claude/ |
There was a problem hiding this comment.
Like we said in 1-2-1 I agree. Will clean up this PR when it's ready for a full review
| @@ -0,0 +1,109 @@ | |||
| # CLAUDE.md | |||
…, both with invalid and valid argument parsing paths. fix spacing in commandstest.
avoid spaces/breaks before LPAREN in allReduce like in functionInvocation. Similar fix to RPAREN handling
… into fixFormatting
| } | ||
| } | ||
|
|
||
| _emitResidualErrorNodes = () => { |
There was a problem hiding this comment.
We already have a check to emit missed error nodes when visiting a terminal node. The one case we missed was when the error node was in the end. This change is to emit the leftover error nodes too.
|
To find new syntax we hadn't covered I checked changes to the Cypher25Parser.g4 file in he monorepo. To then figure how it should be formatted I checked what PRs the changes were for and checked what the canonical syntax looked like for this feature in the corresponding CIPs. I also checked the cypher styling guide for the general formatting rules. |
Things have been busy over the past couple of months, and the formatter has not been kept up to date with recent grammar updates. In addition, it seems like some old syntax like
CREATE DATABASEwas not covered. This PR is to fix this, updating the formatter and adding test with example syntax.Closes LS-145