feat(air-parser): canon stream syntax #618

raftedproc · 2023-06-27T20:23:33Z

Maps proposal adds a new way to access canon streams and maps using # sign prefix, e.g. #$canon_stream, #%canon_map. This patch adds support for # prefix accessing canon structures.

linear · 2023-06-27T20:23:35Z

VM-293 Change canon stream syntax to accommodate canon maps

raftedproc · 2023-07-03T09:36:50Z

crates/air-lib/air-parser/src/parser/lexer/call_variable_parser.rs

-        if self.current_offset() == 0 && stream_tag.is_tag() {
+    fn try_parse_as_tagged_token(&mut self) -> LexerResult<bool> {
+        let tag = MetTag::from_tag(self.current_char());
+        if self.current_offset() == 0 && tag.is_tag() {
            if self.string_to_parse.len() == 1 {
                let error_pos = self.pos_in_string_to_parse();
                return Err(LexerError::empty_stream_name(error_pos..error_pos));


I will also change the error name if you @mikevoronov find the change suitable.

yep, it seems obsolete

mikevoronov · 2023-07-03T13:59:59Z

crates/air-lib/air-parser/src/parser/lexer/call_variable_parser.rs

@@ -45,6 +51,7 @@ struct ParserState {
    pub(self) digit_met: bool,
    pub(self) flattening_met: bool,
    pub(self) met_tag: MetTag,
+    pub(self) canon_type_tag: CanonTypeTag,


What about placing it into MetTag struct?

mikevoronov · 2023-07-03T14:01:18Z

crates/air-lib/air-parser/src/parser/lexer/call_variable_parser.rs

-        if self.current_offset() == 0 && stream_tag.is_tag() {
+    fn try_parse_as_tagged_token(&mut self) -> LexerResult<bool> {
+        let tag = MetTag::from_tag(self.current_char());
+        if self.current_offset() == 0 && tag.is_tag() {
            if self.string_to_parse.len() == 1 {
                let error_pos = self.pos_in_string_to_parse();
                return Err(LexerError::empty_stream_name(error_pos..error_pos));


yep, it seems obsolete

monoid · 2023-07-11T15:16:24Z

What is the intended syntax change? Please, fill the description.

monoid · 2023-07-18T10:07:09Z

crates/air-lib/air-parser/src/parser/lexer/call_variable_parser.rs

            '%' => Self::StreamMap,
            _ => Self::None,
        }
    }

+    fn update_type(&self, tag: char) -> Self {


IMHO should be updated_canon_type. It doesn't update self, BTW.

There might be other types in future this is why this is not a canon type update.

But most likely it will be context-dependent, won't it?

Doesn't look like it. Match + if will do the trick.

Then you will need to update all the call site, as you call it now under if. Perhaps, it worth moving that check inside the function right now.

I will rename the function into maybe_change_type and move if into it.

You should save reader's "cycles", not yours.

Generally, a verb in a method's name means it changes state. It's not the case here.

The future programmer will be the reader/editor whose cycles I am saving.
maybe_change_type will return a value thus a reader will see the assignment. Moreover the variable it returns to is named tag thus it is reasonable good to deduce the new tag is returned.

IMHO, a method can update state and return something as well.
Anyway, all this is mental burden. Just invent some clear name to save the reader's "cycles" you so care about.

monoid · 2023-07-18T10:09:28Z

crates/air-lib/air-parser/src/parser/lexer/call_variable_parser.rs

+    }
+
+    fn is_canon(&self) -> bool {
+        matches!(self, Self::Canon)


If you derive PartialEq on MetTag, you may simply write *self == Self::Canon.

It worth deriving Eq too, as it OK for this type.

What is a benefit comparing with the current code?

Simple equality seems to be more straightforward.

I tend to stick to whatever is used now and see no real benefit changing this to equality b/c it is internally the same.

Code style and idioms are not about internal changes at all.

IMHO it is unwise to use Eq here given that is_tag() uses !matches and I am not changing is_tag b/c it is outside the scope of this patch. You suggest to break the style from my point of view.

Using a PR for small style/idiomatic changes is OK, otherwise they will never be changed.

matches! should be used for types that cannot be compared for some reason ("I haven't defined PartialEq just because" is not the reason). This enum is essentially simple C-style enum.

monoid · 2023-07-18T10:11:31Z

crates/air-lib/air-parser/src/parser/lexer/call_variable_parser.rs

-    fn try_parse_as_stream_start(&mut self) -> LexerResult<bool> {
-        let stream_tag = MetTag::from_tag(self.current_char());
-        if self.current_offset() == 0 && stream_tag.is_tag() {
+    fn try_parse_as_tagged_token(&mut self) -> LexerResult<bool> {


It worth commenting what is "tagged token".

Can you clarify, please?

Sure. I am going to add comments on this matter. Not sure yet what is a suitable place for the comment.

Added the comments near MetTag implementation.

mikevoronov · 2023-07-18T12:56:45Z

crates/air-lib/air-parser/src/parser/lexer/call_variable_parser.rs

+        if self.current_offset() == 1 && tag.is_canon_stream() {
+            if self.string_to_parse.len() == 2 && tag.is_tag() {
+                let error_pos = self.pos_in_string_to_parse();
+                return Err(LexerError::empty_canon_name(error_pos..error_pos));


I think that it could be expressed in a bit different way, please don't merge the PR, I'll take a look later

I am going to rename try_parse_as_tagged_token to try_parse_as_stream as was discussed.

mikevoronov

utACK

raftedproc changed the title ~~Feat/vm 293 canon stream syntax~~ feat(air-parser) canon stream syntax Jun 28, 2023

raftedproc force-pushed the feat/VM-293-canon-stream-syntax branch from aaa3a5e to 4489ad1 Compare June 28, 2023 11:58

raftedproc changed the title ~~feat(air-parser) canon stream syntax~~ feat(air-parser): canon stream syntax Jul 3, 2023

raftedproc commented Jul 3, 2023

View reviewed changes

mikevoronov reviewed Jul 3, 2023

View reviewed changes

raftedproc requested a review from monoid July 4, 2023 11:01

raftedproc added P-low priority: low A-air-parser area: air-parser labels Jul 4, 2023

raftedproc force-pushed the feat/VM-293-canon-stream-syntax branch from 4489ad1 to 90d6f79 Compare July 4, 2023 18:19

raftedproc marked this pull request as ready for review July 5, 2023 09:06

raftedproc force-pushed the feat/VM-293-canon-stream-syntax branch from 90d6f79 to 681ccbd Compare July 10, 2023 17:52

monoid reviewed Jul 18, 2023

View reviewed changes

mikevoronov requested changes Jul 18, 2023

View reviewed changes

raftedproc requested review from mikevoronov and monoid July 20, 2023 11:59

raftedproc force-pushed the feat/VM-293-canon-stream-syntax branch from 681ccbd to babb1dd Compare July 20, 2023 12:00

mikevoronov approved these changes Jul 24, 2023

View reviewed changes

github-actions bot added the e2e Run e2e workflow label Jul 24, 2023

feat(air-parser): improved canon stream syntax support [fixes VM-293]

e40b8bc

raftedproc force-pushed the feat/VM-293-canon-stream-syntax branch from babb1dd to e40b8bc Compare July 24, 2023 18:17

raftedproc added e2e Run e2e workflow and removed e2e Run e2e workflow labels Jul 24, 2023

mikevoronov assigned raftedproc Jul 25, 2023

Merge branch 'master' into feat/VM-293-canon-stream-syntax

0effe24

mikevoronov merged commit 8871465 into master Jul 25, 2023
19 of 20 checks passed

mikevoronov deleted the feat/VM-293-canon-stream-syntax branch July 25, 2023 09:38

fluencebot mentioned this pull request Jul 25, 2023

chore: release master #634

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(air-parser): canon stream syntax #618

feat(air-parser): canon stream syntax #618

raftedproc commented Jun 27, 2023 •

edited

Loading

linear bot commented Jun 27, 2023

raftedproc Jul 3, 2023

mikevoronov Jul 3, 2023

mikevoronov Jul 3, 2023

mikevoronov Jul 3, 2023

monoid commented Jul 11, 2023

monoid Jul 18, 2023 •

edited

Loading

raftedproc Jul 18, 2023

monoid Jul 18, 2023

raftedproc Jul 18, 2023

monoid Jul 18, 2023

raftedproc Jul 18, 2023

monoid Jul 18, 2023

monoid Jul 18, 2023

raftedproc Jul 18, 2023 •

edited

Loading

monoid Jul 18, 2023 •

edited

Loading

monoid Jul 18, 2023

raftedproc Jul 18, 2023

monoid Jul 18, 2023

raftedproc Jul 18, 2023

monoid Jul 19, 2023 •

edited

Loading

raftedproc Jul 19, 2023

monoid Jul 20, 2023

monoid Jul 18, 2023

raftedproc Jul 18, 2023

monoid Jul 18, 2023

raftedproc Jul 18, 2023

raftedproc Jul 20, 2023

mikevoronov Jul 18, 2023

raftedproc Jul 18, 2023

mikevoronov left a comment

feat(air-parser): canon stream syntax #618

feat(air-parser): canon stream syntax #618

Conversation

raftedproc commented Jun 27, 2023 • edited Loading

linear bot commented Jun 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monoid commented Jul 11, 2023

monoid Jul 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raftedproc Jul 18, 2023 • edited Loading

Choose a reason for hiding this comment

monoid Jul 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monoid Jul 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikevoronov left a comment

Choose a reason for hiding this comment

raftedproc commented Jun 27, 2023 •

edited

Loading

monoid Jul 18, 2023 •

edited

Loading

raftedproc Jul 18, 2023 •

edited

Loading

monoid Jul 18, 2023 •

edited

Loading

monoid Jul 19, 2023 •

edited

Loading