Skip to content

Conversation

remo5000
Copy link
Contributor

Features

  • display and timed work
  • Context now has a generic type that can be used to provide external properties, named externalContext. In this case, it is a WorkspaceLocation.

Issues fixed

  • Fixed an issue with WorkspaceLocation enums (untracked as blocking PR not merged)

Caveat

@remo5000 remo5000 requested a review from ning-y June 27, 2018 12:33
remo5000 added 11 commits June 28, 2018 12:00
Any instance of createContext() has been given the appropriate
parameter. As a result, the context has a property that corresponds to
the location it is in (externalContext === 'assessment' or 'playground')
timed does not work as of yet, as the initial implementation does not
work in the first place.
- This does not care about context, and runs the same as map or filter.
I think this should be changed in the future. Unfortunately, I had
little reference as all implementations of `timed` failed when I tried
them (sourceacademy2).

- To make this run, I disabled redux-persist. I shall add what happened
into the tracked issue. To get this to run, checkout createStore.ts and
index.tsx from a commit prior to adding the cache (from master).

- I will move timed back into the "infinity" chapter after getting it to
work.

- Logging does not work as of now.
We should find a way to type the action payloads to catch erros like
these (where we are forced to pass things as untyped objects)
Missed out a todo there (but its coming up in the next commit)
@remo5000 remo5000 changed the base branch from segregate-workspace to master June 28, 2018 04:07
@remo5000 remo5000 force-pushed the display-at-location branch from 038a71f to 5607191 Compare June 28, 2018 04:07
@coveralls
Copy link

Pull Request Test Coverage Report for Build 115

  • 11 of 18 (61.11%) changed or added relevant lines in 5 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.05%) to 30.371%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/slang/createContext.ts 4 5 80.0%
src/slang/stdlib/misc.ts 2 8 25.0%
Files with Coverage Reduction New Missed Lines %
src/slang/stdlib/misc.ts 1 39.02%
Totals Coverage Status
Change from base Build 113: 0.05%
Covered Lines: 1092
Relevant Lines: 3176

💛 - Coveralls

Copy link
Member

@ning-y ning-y left a comment

Choose a reason for hiding this comment

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

Interpreter no longer evaluates asynchronously.

const f = (i) => i < 3 ? 0 : f(i-1) + f(i-2);

const delay = () => f(13);  // increase value for longer delay

function iter(i) {
    delay();
    display(i);
    return i === 0 ? 0 : iter(i-1);
}

iter(15);

@remo5000
Copy link
Contributor Author

remo5000 commented Jun 29, 2018

Interesting. The handleConsoleLog actions seem to run before the render method is called, causing the state to be correct but the stop button and display output not rendering till the very end. Really weird, good catch!

@remo5000
Copy link
Contributor Author

remo5000 commented Jun 29, 2018

I've confirmed this to be a bug present previously, even in the master branch. I've opened up an issue (#142).

@ning-y ning-y merged commit 58453a3 into master Jun 29, 2018
@ning-y ning-y deleted the display-at-location branch July 2, 2018 09:47
Aulud pushed a commit to Aulud/cadet-frontend that referenced this pull request May 25, 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.

3 participants