Skip to content
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

Skip whitespace in SeqN grammar #1626

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 36 additions & 36 deletions src/utilities/codemirror/sequence.grammar
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
@top Sequence {
optSpace
newLine?
(
commentLine* ~maybeComments
(IdDeclaration | ParameterDeclaration | LocalDeclaration | GenericDirective)
Expand All @@ -19,22 +19,28 @@
}

GenericDirective {
genericDirective (whiteSpace String)* newLine
genericDirective String* newLine
}

IdDeclaration {
idDirective (whiteSpace (String | Enum | Number)?)? newLine
idDirective (String | Enum | Number)? newLine
}

ParameterDeclaration {
(parameterDirective whiteSpace Variable{((Enum whiteSpace?) | (Enum whiteSpace? Object whiteSpace?))}+ newLine) |
(parameterStartDirective newLine (whiteSpace? Variable{((Enum whiteSpace?) | (Enum whiteSpace? Type { identifier } whiteSpace? ( (EnumName { identifier}) | (Range { String } (whiteSpace Values {String})? ) | (EnumName { identifier} whiteSpace (Range { String } (whiteSpace Values {String})?) )? whiteSpace?)) )} newLine)* parameterEndDirective newLine )
(parameterDirective Variable{ Enum Object? }+ newLine) |
(parameterStartDirective newLine (Variable newLine)* parameterEndDirective newLine )
}

LocalDeclaration {
(localsDirective whiteSpace Variable{((Enum whiteSpace?) | (Enum whiteSpace? Object whiteSpace?))}+ newLine) |
(localsStartDirective newLine (whiteSpace? Variable{((Enum whiteSpace?) | (Enum whiteSpace? Type { identifier } whiteSpace? ( (EnumName { identifier}) | (Range { String } (whiteSpace Values {String})? ) | (EnumName { identifier} whiteSpace (Range { String } (whiteSpace Values {String})?) )? whiteSpace?)) )} newLine)* localsEndDirective newLine )
(localsDirective Variable{ Enum Object? }+ newLine) |
(localsStartDirective newLine (Variable newLine)* localsEndDirective newLine )
}

Variable {
Enum (Type { identifier } (
(EnumName { identifier } (Range { String } (Values {String})?)? )?
| (Range { String } (Values {String})? )
))?
}

commandBlock {
Expand All @@ -51,10 +57,6 @@ commentLine {
LineComment newLine
}

optSpace {
(newLine | whiteSpace)?
}

Commands {
(LoadAndGoDirective newLine)?
commandBlock
Expand All @@ -70,15 +72,11 @@ HardwareCommands {
commandBlock
}

TimeTag { TimeAbsolute | (TimeGroundEpoch Name { String } whiteSpace) | TimeEpoch | TimeRelative | TimeComplete }
TimeTag { TimeAbsolute | (TimeGroundEpoch Name { String }) | TimeEpoch | TimeRelative | TimeComplete }

Args {
(whiteSpace (arg))* whiteSpace?
}
Args { arg* }

RepeatArg {
"[" (whiteSpace? arg)* whiteSpace? "]"
}
RepeatArg { "[" (arg)* "]" }

arg[@isGroup=Arguments] { Number | String | Boolean | Enum | RepeatArg }

Expand All @@ -101,8 +99,8 @@ commonLoadActivate {
Args
LineComment?
newLine
Engine { engineDirective whiteSpace Number newLine }?
Epoch { epochDirective whiteSpace String newLine }?
Engine { engineDirective Number newLine }?
Epoch { epochDirective String newLine }?
Metadata?
Models?
}
Expand All @@ -123,7 +121,7 @@ commonGround {
Request {
TimeTag
requestStartDirective "(" RequestName { String } ")"
whiteSpace? LineComment? newLine
LineComment? newLine
// json schema requires step+, catch error in linter for cleaner message
Steps { step* }
requestEndDirective newLine
Expand All @@ -133,8 +131,8 @@ Request {
Metadata {
MetaEntry {
metadataDirective
whiteSpace Key { String }
whiteSpace Value { metaValue }
Key { String }
Value { metaValue }
newLine
}+
}
Expand All @@ -143,20 +141,20 @@ metaValue {
String | Number | Boolean | Null | Array | Object
}

Object { "{" (optSpace | list<Property>) "}" }
Array { "[" (optSpace | list<metaValue>) "]" }
Object { "{" (newLine? | list<Property>) "}" }
Array { "[" (newLine? | list<metaValue>) "]" }

Property { PropertyName optSpace ":" optSpace metaValue }
Property { PropertyName newLine? ":" newLine? metaValue }
PropertyName[isolate] { String }

list<item> { optSpace item (optSpace "," optSpace item)* optSpace }
list<item> { newLine? item (newLine? "," newLine? item)* newLine? }

Models {
Model {
modelDirective
whiteSpace Variable { String }
whiteSpace Value { String | Number | Boolean }
whiteSpace Offset { String }
Variable { String }
Value { String | Number | Boolean }
Offset { String }
newLine
}+
}
Expand All @@ -174,15 +172,15 @@ Stem { !stemStart identifier }

timeSecond { $[1-9] @digit* ("."@digit+)? }

TimeAbsolute { 'A'@digit@digit@digit@digit"-"@digit@digit@digit"T"timeHhmmss whiteSpace }
TimeAbsolute { 'A'@digit@digit@digit@digit"-"@digit@digit@digit"T"timeHhmmss }

TimeRelative { 'R'(timeSecond | timeDOY | timeHhmmss) whiteSpace}
TimeRelative { 'R'(timeSecond | timeDOY | timeHhmmss) }

TimeEpoch { 'E'$[+\-]?(timeSecond | timeDOY | timeHhmmss) whiteSpace}
TimeEpoch { 'E'$[+\-]?(timeSecond | timeDOY | timeHhmmss) }

TimeGroundEpoch { 'G'$[+\-]?(timeSecond | timeDOY | timeHhmmss) whiteSpace}
TimeGroundEpoch { 'G'$[+\-]?(timeSecond | timeDOY | timeHhmmss) }

TimeComplete { 'C' whiteSpace }
TimeComplete { 'C ' }

String { '"' (!["\\] | "\\" _)* '"' }

Expand All @@ -202,7 +200,7 @@ Stem { !stemStart identifier }

LineComment { "#"![\n\r]* }

newLine { ($[ \t]* "\n")+ $[ \t]* | (whiteSpace? @eof) }
newLine { @whitespace* ("\n" | @eof) }

whiteSpace { $[ \t]+ }

Expand Down Expand Up @@ -257,3 +255,5 @@ Stem { !stemStart identifier }
identifier
}
}

@skip { whiteSpace }
14 changes: 3 additions & 11 deletions src/utilities/sequence-editor/grammar.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -393,17 +393,9 @@ const errorTests = [
'Bad Input - Invalid stems',
`C 2_STEM_NAME
STEM$BAR`,
`Sequence(Commands(Command(TimeTag(TimeComplete),⚠(Number),Stem,Args),Command(Stem,⚠),Command(Stem,Args)))`,
],
[
'Stem with disallowed characters',
`FSW_CMD%BAR$BAZ`,
`Sequence(Commands(
Command(Stem,⚠),
Command(Stem,⚠),
Command(Stem,Args)
))`,
`Sequence(Commands(Command(TimeTag(TimeComplete),⚠(Number),Stem,Args),Command(Stem,⚠,Args(Enum))))`,
],
['Stem with disallowed characters', `FSW_CMD%BAR$BAZ`, `Sequence(Commands(Command(Stem,⚠,Args(Enum,⚠,Enum))))`],
Comment on lines -397 to +398
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curios why this test was changed? I don't think it needed to be changed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the new grammar just parsed it slightly differently, so I changed the expected output to match. It winds up parsing as a single command with some errors and enum arguments, rather than multiple commands with errors in between.
The formatting was changed by the formatting and linting tools. I'm not sure if those were being applied regularly to this file... they might be doing more harm than good here.

[
'Stem ending in disallowed character',
`FSW_CMD%`,
Expand All @@ -418,7 +410,7 @@ CMD2 [
CMD3 ]`,
`Sequence(Commands(
Command(Stem,Args(RepeatArg(RepeatArg,⚠))),
Command(Stem,Args(RepeatArg(⚠))),Command(Stem,Args(⚠))))`,
Command(Stem,Args(RepeatArg(⚠))),Command(Stem,⚠,Args)))`,
],
['locals with wrong value types', `@LOCALS "string_not_enum"`, `Sequence(LocalDeclaration(⚠(String)))`],
];
Expand Down
Loading