Splits parsing of statements #171
Conversation
ncordon
force-pushed
the
independent-st-parsing
branch
from
f1ab795 to
71299a7
Compare
ncordon
force-pushed
the
independent-st-parsing
branch
from
71299a7 to
be38e8c
Compare
| } | ||
| } | ||
|
|
||
| export function findCaret( |
There was a problem hiding this comment.
Just moves and reshapes this from the signature help, hopefully it can be reused for the auto-completion
There was a problem hiding this comment.
I feel like there should be a good way to find where the cursor is in the tree without having to go looking for it, but I've not found anything either, a bit dissapointing
ncordon
force-pushed
the
independent-st-parsing
branch
from
f25383d to
02586b0
Compare
| @@ -85,15 +84,14 @@ function warnOnUndeclaredLabels( | |||
|
|
|||
| type FixSemanticPositionsArgs = { | |||
| semanticElements: SemanticAnalysisElement[]; | |||
| cmd: ParsedCypherCmd; | |||
| parseResult: EnrichedParsingResult; | |||
| parseResult: StatementParsing; | |||
There was a problem hiding this comment.
This is a nit, but I think we could find a better name for StatementParsing since it doesn't sound like the result of an operation to me. Perhaps something like ParsedStatements, PartialParseResult or StatementParseResult ?
| }); | ||
| /** | ||
| * Assumes the provided query has no parse errors | ||
| */ |
There was a problem hiding this comment.
Since we do the parse here anyway, why assume that there are no parse errors when we can just check cachedParse.diagnostics?
| export function signatureHelp( | ||
| fullQuery: string, | ||
| dbSchema: DbSchema, | ||
| caretPosition: number, | ||
| ): SignatureHelp { | ||
| let result: SignatureHelp = emptyResult; | ||
|
|
||
| // TODO Is this check needed |
There was a problem hiding this comment.
are you leaving this for the next pr as well?
| expectParsedCommands(':history;', [{ type: 'history' }]); | ||
| expectParsedCommands(':history;', [ | ||
| { type: 'history' }, | ||
| { type: 'parse-error' }, |
There was a problem hiding this comment.
My gut feeling is that this should not give a parse error here. To me it makes sense that empty string on it's own should be a parse error, but empty string as the last statement in a list should not be considered a statement at all?
There was a problem hiding this comment.
It should be considered a statement cause I do need the auto-completion to pick up that we are in a new statement to suggest new things.
I've changed this for { statement: ''", type: "cypher"}
packages/react-codemirror/src/e2e_tests/syntax-validation.spec.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| const anySyntacticError = | ||
| statements.filter((statement) => statement.diagnostics.length !== 0) | ||
| .length > 0; |
There was a problem hiding this comment.
For convenience it could be interesting to have something aggregating helper functions on the top level parse result?
const parse = parserWrapper.parse(query);
if(parse.getAllDiagnostics().length > 0) {There was a problem hiding this comment.
Wouldn't that be creating extra objects (cause we need to concatenate possibly a few arrays from the internal statements)?
Not sure there's a logical view for arrays in Typescript or a similar concept
What
Parses a query with independent statements.
Why
Because we want to have independent statements for the collected variables, which get offered in auto-completions, but also to avoid destroying and recreating the parsing several times on every keystroke.