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

Feat/seqgen grammar v2 #1362

Merged
merged 20 commits into from
Jul 16, 2024
Merged

Feat/seqgen grammar v2 #1362

merged 20 commits into from
Jul 16, 2024

Conversation

joswig
Copy link
Collaborator

@joswig joswig commented Jul 5, 2024

Closes #1361

@shaheerk94
Copy link

shaheerk94 commented Jul 10, 2024

@joswig I see ground epochs have been implemented like so:

`@GROUND_EPOCH("Name","+3:00") @REQUEST_BEGIN("request.name") # Description Text
  C CMD_0 1 2 3
  @METADATA "foo" "bar"
  @MODEL "a" 1 "00:00:00"
  R100 CMD_1 "1 2 3"
  C CMD_2 1 2 3
  R100 CMD_3 "1 2 3"
@REQUEST_END

I propose that we instead support ground epochs at the same level that we support absolute/relative/command complete times like so:

`E NAME+03:00:00 @REQUEST_BEGIN("request.name") # Description Text
  C CMD_0 1 2 3
  @METADATA "foo" "bar"
  @MODEL "a" 1 "00:00:00"

  R100 CMD_1 "1 2 3"

  C CMD_2 1 2 3

  R100 CMD_3 "1 2 3"

@REQUEST_END
@METADATA "sub_object" {
  "boolean": true
}

I added a space between the E and the epoch name because otherwise you end up with ENAME+03:00:00 which isn't the most readable. Maybe this is why you went with @GROUND_EPOCH instead? I do think we should try to have the ground epoch time tag match the format of the other time tags if possible. We should also support using ground epochs not just with request start times, but also on any random command, such as:

E NAME+03:01 CMD_1 "1 2 3" 

@joswig
Copy link
Collaborator Author

joswig commented Jul 10, 2024

@joswig I see ground epochs have been implemented like so:

`@GROUND_EPOCH("Name","+3:00") @REQUEST_BEGIN("request.name") # Description Text
  C CMD_0 1 2 3
  @METADATA "foo" "bar"
  @MODEL "a" 1 "00:00:00"
  R100 CMD_1 "1 2 3"
  C CMD_2 1 2 3
  R100 CMD_3 "1 2 3"
@REQUEST_END

I propose that we instead support ground epochs at the same level that we support absolute/relative/command complete times like so:

`E NAME+03:00:00 @REQUEST_BEGIN("request.name") # Description Text
  C CMD_0 1 2 3
  @METADATA "foo" "bar"
  @MODEL "a" 1 "00:00:00"

  R100 CMD_1 "1 2 3"

  C CMD_2 1 2 3

  R100 CMD_3 "1 2 3"

@REQUEST_END
@METADATA "sub_object" {
  "boolean": true
}

I added a space between the E and the epoch name because otherwise you end up with ENAME+03:00:00 which isn't the most readable. Maybe this is why you went with @GROUND_EPOCH instead? I do think we should try to have the ground epoch time tag match the format of the other time tags if possible. We should also support using ground epochs not just with request start times, but also on any random command, such as:

E NAME+03:01 CMD_1 "1 2 3" 

What guarantees do we have on delta format? String is in the schema

@joswig
Copy link
Collaborator Author

joswig commented Jul 10, 2024

We will address ground epoch update with the implementation of ground epoch time tags for commands

Copy link
Contributor

@goetzrrGit goetzrrGit left a comment

Choose a reason for hiding this comment

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

On thing to note is we don't have the autocomplete hooked up. Not sure if you want to do that later?

Copy link
Collaborator

@duranb duranb left a comment

Choose a reason for hiding this comment

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

Is it ok if we hold off on merging this? I would like to prioritize @cohansen's refactor first. I don't expect to see many conflicts other than the globals change.

@joswig joswig merged commit dbe5435 into develop Jul 16, 2024
5 checks passed
@joswig joswig deleted the feat/seqgen_grammar_v2 branch July 16, 2024 21:11
duranb added a commit that referenced this pull request Jul 16, 2024
@duranb duranb restored the feat/seqgen_grammar_v2 branch July 16, 2024 23:17
joswig pushed a commit that referenced this pull request Jul 17, 2024
Revert "Feat/seqgen grammar v2 (#1362)"

This reverts commit dbe5435.
JosephVolosin pushed a commit that referenced this pull request Aug 20, 2024
* grammar test cleanup

* error cleanup

* alternate syntax for seqgen constructs

* serialization and highlighting for load, activate, ground_block, ground_event

* Round trip support for activate, load, ground event, ground block, request

* corrected relative time format typo

* merge in updates for string handling from develop

* merge in grammar tests from develop

* merged in fixes for quoted symbols and missing new lines

* run prettier

* include day separator

* move time format code, simple selected step support

* Generate lint errors on non command constructs unless enabled in adaptation

* allow non-command steps by default

* remove wip vml

* color highlighting for ground epoch

* Don't add space between consecutive CAPITAL letters
JosephVolosin pushed a commit that referenced this pull request Aug 20, 2024
Revert "Feat/seqgen grammar v2 (#1362)"

This reverts commit c017474.
JosephVolosin pushed a commit that referenced this pull request Oct 21, 2024
* grammar test cleanup

* error cleanup

* alternate syntax for seqgen constructs

* serialization and highlighting for load, activate, ground_block, ground_event

* Round trip support for activate, load, ground event, ground block, request

* corrected relative time format typo

* merge in updates for string handling from develop

* merge in grammar tests from develop

* merged in fixes for quoted symbols and missing new lines

* run prettier

* include day separator

* move time format code, simple selected step support

* Generate lint errors on non command constructs unless enabled in adaptation

* allow non-command steps by default

* remove wip vml

* color highlighting for ground epoch

* Don't add space between consecutive CAPITAL letters
JosephVolosin pushed a commit that referenced this pull request Oct 21, 2024
Revert "Feat/seqgen grammar v2 (#1362)"

This reverts commit dbe5435.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Phoenix Editor - add support for non-command steps and requests
4 participants