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

repl: Tab completion issue when useGlobal is set to false #7353

Closed
princejwesley opened this issue Jun 21, 2016 · 5 comments
Closed

repl: Tab completion issue when useGlobal is set to false #7353

princejwesley opened this issue Jun 21, 2016 · 5 comments
Labels
repl Issues and PRs related to the REPL subsystem.

Comments

@princejwesley
Copy link
Contributor

Version: v6.1.0 , (master: v7.0.0-pre)
Platform: Darwin Princes-MacBook-Pro.local 15.5.0 Darwin Kernel Version 15.5.0: Tue Apr 19 18:36:36 PDT 2016; root:xnu-3248.50.21~8/RELEASE_X86_64 x86_64
Subsystem: repl

When useGlobal is set to false, Tab complete function returns fewer results.

// With useGlobal: true
Desktop 🙈  node
> I<<TAB>>
Infinity

Int16Array  Int32Array  Int8Array   Intl

> In

// With useGlobal: false
Desktop 🙈  cat test.js
const repl = require('repl');
repl.start({useGlobal: false});

Desktop 🙈  node test.js
> In<<TAB>> // expand to 'Infinity'
@addaleax addaleax added the repl Issues and PRs related to the REPL subsystem. label Jun 21, 2016
princejwesley added a commit to princejwesley/node that referenced this issue Jun 21, 2016
princejwesley added a commit to princejwesley/node that referenced this issue Jun 21, 2016
@princejwesley
Copy link
Contributor Author

@addaleax princejwesley@e1884d5 fixes this issue.

@addaleax
Copy link
Member

I’m still not sure if that is the right fix, but then again I’m not actually too familiar with how non-useGlobal REPLs are usually used outside of core.

My first intuition would have been that implementing .scope by default could solve this (apparently that’s what the debugger REPL does?), but ultimately I can’t speak up for which approach would be the best one.

@addaleax
Copy link
Member

I’d still think that side effects of tab completion should be avoided as far as possible.

@lance
Copy link
Member

lance commented Jun 22, 2016

@princejwesley hope I'm not stepping on your toes here, but I think this is the better way to accomplish what you are trying to do. /cc @addaleax

@princejwesley
Copy link
Contributor Author

@lance Please find my comments

@lance lance closed this as completed in c0e48bf Jun 27, 2016
Fishrock123 pushed a commit that referenced this issue Jul 5, 2016
When `useGlobal` is false, tab completion in the repl does not enumerate
global properties. Instead of just setting these properties blindly on
the global context, e.g.

    context[prop] = global[prop]

Use `Object.defineProperty` and the property descriptor found on
`global` for the new property in `context`.

Also addresses a previously unnoticed issue where `console` is writable
when `useGlobal` is false.

If the binary has been built with `./configure --without-intl` then the
`Intl` builtin type will not be available in a repl runtime. Check for
this in the test.

Fixes: #7353
PR-URL: #7369
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this issue Oct 10, 2016
When `useGlobal` is false, tab completion in the repl does not enumerate
global properties. Instead of just setting these properties blindly on
the global context, e.g.

    context[prop] = global[prop]

Use `Object.defineProperty` and the property descriptor found on
`global` for the new property in `context`.

Also addresses a previously unnoticed issue where `console` is writable
when `useGlobal` is false.

If the binary has been built with `./configure --without-intl` then the
`Intl` builtin type will not be available in a repl runtime. Check for
this in the test.

Fixes: #7353
PR-URL: #7369
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this issue Oct 10, 2016
When `useGlobal` is false, tab completion in the repl does not enumerate
global properties. Instead of just setting these properties blindly on
the global context, e.g.

    context[prop] = global[prop]

Use `Object.defineProperty` and the property descriptor found on
`global` for the new property in `context`.

Also addresses a previously unnoticed issue where `console` is writable
when `useGlobal` is false.

If the binary has been built with `./configure --without-intl` then the
`Intl` builtin type will not be available in a repl runtime. Check for
this in the test.

Fixes: #7353
PR-URL: #7369
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
rvagg pushed a commit that referenced this issue Oct 18, 2016
When `useGlobal` is false, tab completion in the repl does not enumerate
global properties. Instead of just setting these properties blindly on
the global context, e.g.

    context[prop] = global[prop]

Use `Object.defineProperty` and the property descriptor found on
`global` for the new property in `context`.

Also addresses a previously unnoticed issue where `console` is writable
when `useGlobal` is false.

If the binary has been built with `./configure --without-intl` then the
`Intl` builtin type will not be available in a repl runtime. Check for
this in the test.

Fixes: #7353
PR-URL: #7369
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this issue Oct 26, 2016
When `useGlobal` is false, tab completion in the repl does not enumerate
global properties. Instead of just setting these properties blindly on
the global context, e.g.

    context[prop] = global[prop]

Use `Object.defineProperty` and the property descriptor found on
`global` for the new property in `context`.

Also addresses a previously unnoticed issue where `console` is writable
when `useGlobal` is false.

If the binary has been built with `./configure --without-intl` then the
`Intl` builtin type will not be available in a repl runtime. Check for
this in the test.

Fixes: #7353
PR-URL: #7369
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants