Skip to content

Conversation

@targos
Copy link
Member

@targos targos commented Dec 10, 2019

This fixes two issues in the REPL when it is started with a new context
(useGlobal option set to false):

  • The primordials object does not contain all builtins, so the
    filtering based on property names from primordials was wrong.
  • The autocompleter did not take builtin names into account because
    they are not properties of the context object.

A list of all global builtin names is created lazily when needed. It is
used for filtering for the copy and for adding those names to the
autocompleter list.

Fixes: #30792

I did not add a new test because this will be covered when #30740 has landed.

@targos targos requested a review from BridgeAR December 10, 2019 15:28
@nodejs-github-bot nodejs-github-bot added the repl Issues and PRs related to the REPL subsystem. label Dec 10, 2019
This fixes two issues in the REPL when it is started with a new context (useGlobal option set to `false`): - The `primordials` object does not contain all builtins, so the filtering based on property names from `primordials` was wrong. - The autocompleter did not take builtin names into account because they are not properties of the context object. A list of all global builtin names is created lazily when needed. It is used for filtering for the copy and for adding those names to the autocompleter list. Fixes: nodejs#30792
@targos targos added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 13, 2019
@Trott
Copy link
Member

Trott commented Dec 14, 2019

Landed in 8e58c75

@Trott Trott closed this Dec 14, 2019
Trott pushed a commit to Trott/io.js that referenced this pull request Dec 14, 2019
This fixes two issues in the REPL when it is started with a new context (useGlobal option set to `false`): - The `primordials` object does not contain all builtins, so the filtering based on property names from `primordials` was wrong. - The autocompleter did not take builtin names into account because they are not properties of the context object. A list of all global builtin names is created lazily when needed. It is used for filtering for the copy and for adding those names to the autocompleter list. Fixes: nodejs#30792 PR-URL: nodejs#30883 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
This fixes two issues in the REPL when it is started with a new context (useGlobal option set to `false`): - The `primordials` object does not contain all builtins, so the filtering based on property names from `primordials` was wrong. - The autocompleter did not take builtin names into account because they are not properties of the context object. A list of all global builtin names is created lazily when needed. It is used for filtering for the copy and for adding those names to the autocompleter list. Fixes: #30792 PR-URL: #30883 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Dec 17, 2019
targos added a commit that referenced this pull request Jan 14, 2020
This fixes two issues in the REPL when it is started with a new context (useGlobal option set to `false`): - The `primordials` object does not contain all builtins, so the filtering based on property names from `primordials` was wrong. - The autocompleter did not take builtin names into account because they are not properties of the context object. A list of all global builtin names is created lazily when needed. It is used for filtering for the copy and for adding those names to the autocompleter list. Fixes: #30792 PR-URL: #30883 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
This fixes two issues in the REPL when it is started with a new context (useGlobal option set to `false`): - The `primordials` object does not contain all builtins, so the filtering based on property names from `primordials` was wrong. - The autocompleter did not take builtin names into account because they are not properties of the context object. A list of all global builtin names is created lazily when needed. It is used for filtering for the copy and for adding those names to the autocompleter list. Fixes: #30792 PR-URL: #30883 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
@targos targos deleted the fix-use-global branch October 17, 2020 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. repl Issues and PRs related to the REPL subsystem.

5 participants