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

[askscript] How about we implement push/append to an array? #387

Closed
czerwinskilukasz1 opened this issue Jul 9, 2020 · 5 comments · Fixed by #510
Closed

[askscript] How about we implement push/append to an array? #387

czerwinskilukasz1 opened this issue Jul 9, 2020 · 5 comments · Fixed by #510
Labels
AskVM ./src/askvm/** enhancement New feature or request first-timers-only good first issue Good for newcomers help wanted Extra attention is needed type:resources related to AskVM resources
Milestone

Comments

@czerwinskilukasz1
Copy link
Collaborator

While writing AskScript examples, I wanted to append to an array twice already.
Alas, the best I could do is to write:

arr:set(arr:length, value)

which is not that easy readable.

I'd prefer to have push/append, like in Javascript:

  let result = [];
  for (let i = 1; i <= m; ++i) {
    result.push(n*i);
  }
@czerwinskilukasz1 czerwinskilukasz1 added enhancement New feature or request AskVM ./src/askvm/** discussion A discussion is taking place, please don't work on it yet type:resources related to AskVM resources labels Jul 9, 2020
@mhagmajer
Copy link
Contributor

@czerwinskilukasz1 push is agains the principle of immutable data. If we come up with some syntax sugar it will still translate to:

result = result:push(n*i)

@czerwinskilukasz1
Copy link
Collaborator Author

Sure sure, I'm totally fine with result = result:push(elem).

@czerwinskilukasz1 czerwinskilukasz1 removed the discussion A discussion is taking place, please don't work on it yet label Jul 17, 2020
@czerwinskilukasz1 czerwinskilukasz1 added first-timers-only good first issue Good for newcomers help wanted Extra attention is needed labels Jul 17, 2020
@avats-dev
Copy link

Hey @mhagmajer and @czerwinskilukasz1 , as I have asked elsewhere also, if you could guide me where and what needs to be done to get this fix, I would do so.

I'm new to this codebase, but want to contribute.

@czerwinskilukasz1
Copy link
Collaborator Author

Hi Aditya, welcome to AskQL again :)
This task is pretty straightforward, comparing to the one with double let expression. The task is to create a new resource (aka function) called push, which will live in src/askvm/resources/list/, same as set resource, which is currently defined.
Please use src/askvm/resources/list/set.ts as an example how resources are defined and how they work.
Let me know if you have more questions. @avats-dev

@mhagmajer
Copy link
Contributor

Closed with #510 from @bardeutsch 👏

@czerwinskilukasz1 czerwinskilukasz1 added this to the v1.3 milestone Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AskVM ./src/askvm/** enhancement New feature or request first-timers-only good first issue Good for newcomers help wanted Extra attention is needed type:resources related to AskVM resources
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants