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 for single stats if/else, loops and functions #409

Merged
merged 8 commits into from
Dec 16, 2020

Conversation

shobhit10058
Copy link
Contributor

concerns issue #27
The changes are highlighted here #404 . This contains the same code as that branch just the name is changed.

@shobhit10058 shobhit10058 changed the title In 27 Support for single stats if/else, loops and functions Dec 14, 2020
@Math-O5
Copy link
Collaborator

Math-O5 commented Dec 14, 2020

@shobhit10058 Tests passed.

@Math-O5
Copy link
Collaborator

Math-O5 commented Dec 16, 2020

@shobhit10058 It's a bit complex understand what you did. This variable func_st and it's comments is not very clear what is happing. May you think in another name? Like func_scope, and instead sum += on it, would be nice if you declare some constant right before while loop as bellow:

    # Function not called
    NO_FUNC_CALLED = -1

    # Function start body
    START_FUNCTION = 0

    # Function end body 
    END_FUNCTION = 1 

Now, one suggestion of comment of func_st is Flag which mark scope of functions without braces.

Plus, remember resolve the conflict with master, then I can merge it!

Tnx by the PR.

@shobhit10058
Copy link
Contributor Author

So, I have to change the name of func_st. I thought It as a flag for a start of function with a single statement i.e one who's body is used without the braces.

@shobhit10058
Copy link
Contributor Author

Ok, I understood what you are saying. I will update the PR.
So, I have to change the name of the flag
I have to give names to the constants -1,0 and 1.

@shobhit10058
Copy link
Contributor Author

now the unmatched braces one is not working it will give error. Rest all are working.

Copy link
Collaborator

@Math-O5 Math-O5 left a comment

Choose a reason for hiding this comment

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

@shobhit10058 Well done!

Copy link
Collaborator

@Math-O5 Math-O5 left a comment

Choose a reason for hiding this comment

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

@shobhit10058 Well done!

@Math-O5 Math-O5 merged commit 499c1be into cimplec:master Dec 16, 2020
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