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(askscript): allow else if construct #459

Merged
merged 42 commits into from
Oct 7, 2020

Conversation

markkulube
Copy link
Contributor

Closes #378: Allow 'else if' construct

@markkulube markkulube changed the title Allow 'else if' construct #378 feat(askscript): Allow 'else if' construct #378 Sep 12, 2020
@markkulube markkulube changed the title feat(askscript): Allow 'else if' construct #378 feat(askscript): allow else if construct #378 Sep 12, 2020
dependabot bot and others added 23 commits September 12, 2020 22:17
…rnado#429)

Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 14.0.26 to 14.0.27.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [nanoid](https://github.com/ai/nanoid) from 3.1.10 to 3.1.12.
- [Release notes](https://github.com/ai/nanoid/releases)
- [Changelog](https://github.com/ai/nanoid/blob/master/CHANGELOG.md)
- [Commits](ai/nanoid@3.1.10...3.1.12)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
)

Bumps [webpack](https://github.com/webpack/webpack) from 4.44.0 to 4.44.1.
- [Release notes](https://github.com/webpack/webpack/releases)
- [Commits](webpack/webpack@v4.44.0...v4.44.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…o#433)

Bumps [@types/cors](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/cors) from 2.8.6 to 2.8.7.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/cors)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Karol Milewski <km4@pc50.home>
Bumps [jest](https://github.com/facebook/jest) from 26.1.0 to 26.2.2.
- [Release notes](https://github.com/facebook/jest/releases)
- [Changelog](https://github.com/facebook/jest/blob/master/CHANGELOG.md)
- [Commits](jestjs/jest@v26.1.0...v26.2.2)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* feat(askvm): toInt

* Update toInt.test.ts

* Updates toInt test

* Extends tests for toInt

Co-authored-by: Richard <ricmoran@ebay.com>
* fix: create OS agnostic build command/s for rm and mkdir (CatchTheTornado#395)

* feat: implement mod resource(CatchTheTornado#250)

* feat: implement mod resource (CatchTheTornado#250)
…ado#444)

Bumps [@types/jest](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/jest) from 26.0.3 to 26.0.9.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/jest)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [eslint](https://github.com/eslint/eslint) from 7.5.0 to 7.6.0.
- [Release notes](https://github.com/eslint/eslint/releases)
- [Changelog](https://github.com/eslint/eslint/blob/master/CHANGELOG.md)
- [Commits](eslint/eslint@v7.5.0...v7.6.0)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…heTornado#441)

Bumps [@types/lodash](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/lodash) from 4.14.158 to 4.14.159.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/lodash)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Copy link
Collaborator

@czerwinskilukasz1 czerwinskilukasz1 left a comment

Choose a reason for hiding this comment

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

Hi Mark,
Great to see your PR, congrats!
There is just a couple of details to fix.

Most importantly, it seems to me that *.formatted.ask files are not just reformatted .ask files. They are missing their else and else if branches, e.g. file program10d_3d-if_else_if_else.ask.formatted.ask is:

ask() {
  let len = 10
  if ((get('len')==12)) {
    return '3DF'
  }
}

while it should be a reformatted version of program10d_3d-if_else_if_else.ask file, which is:

ask {
    let len = 10
    if (len == 12) {
        return '3DF';
    } else if (len == 14) {
        return '3DF';
    } else if (len == 16) {
        return '3DF';
    } else {
        return '3DP';
    }
}

@@ -1,25 +1,34 @@
{
"name": "askscript-language",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file is heavily changed for no good reason.
I think it's just reformatting. For readability, how about you revert the changes in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's only a reformatting issue. I executed npm run prettier, and inadvertently reformatted reformatted files that were not to intended to be.

Previous formatting has been restored.

@@ -2,28 +2,28 @@

## What's in the folder

* This folder contains all of the files necessary for your extension.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any good reason for these formatting changes? I would like to avoid you changing from * to - and then somebody else changing back from - to *, so if reasonable, please revert changes in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's only a reformatting issue. I executed npm run prettier, and inadvertently reformatted reformatted files that were not to intended to be.

Previous formatting has been restored.

@@ -0,0 +1,46 @@
export = (
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there should be much more files added than just one .tsx file.
Look what I got after running npm run build && npm test:

image

All these files should be added to your commit.

Also, your changes to the code resulted in changes in the existing .ast.tsx files. You should commit them too:
image

Copy link
Contributor Author

@markkulube markkulube Sep 16, 2020

Choose a reason for hiding this comment

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

Yes, indeed, all missing *.askcode and *.ast.tsx have been committed and pushed.

I am using the git client sourcetree which does makes it easier to identify, stage, and commit untracked or updated files.

Let me know if anything is still missing.

@markkulube
Copy link
Contributor Author

markkulube commented Sep 16, 2020

@czerwinskilukasz1 the main issue has been resolved.

It was great putting theory into practice.

Since an if-else block amounts to a linked list (If->ElseIf->ElseIf->...->Else), we have to traverse the prettier-plugin-askscript/print.ts/node to access inner/subsequent blocks or branches. It's the cleanest way I could do it.

Thus we can generate the correct and complete *.askcode and *.ask.formatted.ask files on executing the test scripts.

Non of the existing *.askcode have been changed, which tells us that the else if else if implementation is does not affect the behavior of the current if and if-else clauses.

Still do let me know if anything is amiss.

@markkulube markkulube changed the title feat(askscript): allow else if construct #378 feat(askscript): allow else if construct Sep 20, 2020
@markkulube
Copy link
Contributor Author

markkulube commented Sep 21, 2020

@czerwinskilukasz1 just a couple notes:

  • Added another test for the case when the if else if else ladder terminates with an else if branch.
  • In reality, since we are reusing the If jsx structure, the behavior is the same as when the ladder only constitutes an if clause.
Welcome to AskQL CLI!

For multi-line mode please type:
     .editor
🦄 .editor
// Entering editor mode (^D to finish, ^C to cancel)
let len = 333
if(len == 3) {
return '3A FAILED';
} else if(len == 10) {
return '3A FAILED';
} else if(len == 333) {
return '3A PASSED';
}

string ask(let('len',333),if(call(get('=='),get('len'),3),block(return('3A FAILED')),block(if(call(get('=='),get('len'),10),block(return('3A FAILED')),block(if(call(get('=='),get('len'),333),block(return('3A PASSED')),block()))))))
'3A PASSED'
🦄              

Copy link
Collaborator

@czerwinskilukasz1 czerwinskilukasz1 left a comment

Choose a reason for hiding this comment

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

LGTM

@czerwinskilukasz1 czerwinskilukasz1 merged commit 7264236 into CatchTheTornado:master Oct 7, 2020
@YonatanKra YonatanKra added the hacktoberfest-accepted PR accepted for Hacktoberfest label Oct 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted PR accepted for Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow 'else if' construct
6 participants