-
Notifications
You must be signed in to change notification settings - Fork 46
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
Allow variables to have a leading digit #317
Allow variables to have a leading digit #317
Conversation
PR Review ChecklistDo not edit the content of this comment. The PR reviewer should simply update this comment by ticking each review item below, as they get completed. Trivial Change
Code
Architecture
|
grammar/TypeQL.g4
Outdated
IID_ : '0x' [0-9a-f]+ ; | ||
LABEL_ : TYPE_CHAR_H_ TYPE_CHAR_T_* ; | ||
LABEL_ : IDENTIFIER_LABEL_H IDENTIFIER_LABEL_T* ; |
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.
TYPE_
prefix was a hangover from the past when "label" was called "type" in the grammar. Fixed here.
d39e760
to
7237e05
Compare
rust/parser/typeql.pest
Outdated
LABEL_SCOPED_ = @{ IDENTIFIER ~ ":" ~ IDENTIFIER } | ||
IDENTIFIER = @{ TYPE_CHAR_H_ ~ TYPE_CHAR_T_* ~ WB } | ||
LABEL_ = @{ (IDENTIFIER_LABEL_H_ ~ IDENTIFIER_LABEL_T_* ~ WB) } | ||
LABEL_SCOPED_ = @{ (IDENTIFIER_LABEL_H_ ~ IDENTIFIER_LABEL_T_* ) ~ ":" ~ (IDENTIFIER_LABEL_H_ ~ IDENTIFIER_LABEL_T_* ~ WB) } |
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.
Duplicated Label
pattern here, so we don't end up with WB
inside the scoped label? Is this necessary?
b554ae9
to
9adfb59
Compare
LABEL_ = @{ (IDENTIFIER_LABEL_H_ ~ IDENTIFIER_LABEL_T_* ~ WB) } | ||
LABEL_SCOPED_ = @{ (IDENTIFIER_LABEL_H_ ~ IDENTIFIER_LABEL_T_*) ~ ":" ~ (IDENTIFIER_LABEL_H_ ~ IDENTIFIER_LABEL_T_*) ~ WB } |
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.
We should probably have an IDENTIFIER_LABEL_
rule.
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.
Although the ANTLR grammar reuses LABEL_
instead. If we do that, IDENTIFIER_VAR_
might have to go as well. I'd rather we introduce both in ANTLR though.
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.
I just canceled them all, so we don't end up with confusion re. ~wb
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.
This can still be @{ LABEL_ ~ ":" ~ LABEL_ }
4458b18
to
f1bfd32
Compare
Usage and product changes
We modify the behaviour of #310 which unified variables and labels to have the same valid identifier syntax, which eliminated the capability of variables to have a leading number. For example, the variable
$0
was banned.This PR reverts this specific behaviour, and enables usage of variables with leading digits:
is made valid again.
Testing specified in typedb/typedb-behaviour#281
Implementation
We split the identifier regexes and grammar rules that validated labels and variables before, to be specific to
Variable
andLabel
identifiers.