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

Support syntax of global variables #47

Merged
merged 2 commits into from
Mar 1, 2019

Conversation

fernandreu
Copy link
Contributor

Hi,

I attempted to implement issue #32, while adding test files for it and ensuring syntax highlighting works. I am completely new to lexers and IntelliJ plugins, so feel free to modify / reject this!

While doing this, I have seen that lines such as "global a if b == 2 c = 3 end" do not flag any errors, but I think they should. However, I saw that the same occurs with other keywords, such as "try if b == 2 c = 3 end end". Hence, if this is actually an issue, hopefully it is not directly concerning the global keywords.

Finally, MATLAB would also highlight the global variables in a different color, both when you declare them as global and later every time they are used. Should this behaviour be implemented here as well? I have not checked how this could be done, but I suspect it will have less to do with updating the lexer and more to do with the Kotlin classes.

@kornilova203
Copy link
Owner

That’s awesome! I’ll review it tomorrow :)

Copy link
Owner

@kornilova203 kornilova203 left a comment

Choose a reason for hiding this comment

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

Looks good!
Only couple of small changes required

@@ -110,6 +111,8 @@ meta block ::= (<<p>> NEWLINE *)*
private block_that_recovers_until_end ::= <<block element>> { recoverWhile=not_end_or_oef }
private not_end_or_oef ::= !( NEWLINE* ( end | <<eof>> ) )

global_block ::= global br* IDENTIFIER (br* IDENTIFIER)* [',' | sep] { pin=1 }
Copy link
Owner

Choose a reason for hiding this comment

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

Let's rename the rule to global_variable_declaration
Also you do not need [',' | sep] at the end because this suffix is in element rule

Copy link
Owner

Choose a reason for hiding this comment

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

I think global_variable_declarations would be better ('s' on the end)

PsiElement(IDENTIFIER)('f')
PsiWhiteSpace(' ')
PsiElement(IDENTIFIER)('g')
PsiElement(NEWLINE)('\n')
Copy link
Owner

Choose a reason for hiding this comment

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

Seems like tests should trim extra whitespaces in test data
When I run the test I see that there is no last line PsiElement(NEWLINE)('\n'), so just remove it (and re-run test to check that it works)

@kornilova203
Copy link
Owner

"try if b == 2 c = 3 end end" is valid octave statement. Does it cause error in Matlab?

@kornilova203
Copy link
Owner

We can add color to global variables
But we need to resolve them, it's should not be difficult. I created the issue about it #48 you may try to implement it
Note that currently resolve works only within one file (see #24)

@fernandreu
Copy link
Contributor Author

Thanks for the feedback! I will try to make all those changes today / tomorrow.

"try if b == 2 c = 3 end end" is valid octave statement. Does it cause error in Matlab?

In MATLAB (R2018b at least) I obtain this:
matlab error
(c has just a warning, and the error on the try simply says "Code analysis did not complete. File contains too many syntax errors.")

Anyway, I guess it is good as it is now if Octave is fine with it.

Once I am done, I will also have a look at #48 and see if I can implement it.

@kornilova203 kornilova203 merged commit 57d6ecf into kornilova203:master Mar 1, 2019
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.

2 participants