Skip to content

repl: allow autocompletion for scoped packages#10296

Closed
evanlucas wants to merge 1 commit intonodejs:masterfrom
evanlucas:replscope
Closed

repl: allow autocompletion for scoped packages#10296
evanlucas wants to merge 1 commit intonodejs:masterfrom
evanlucas:replscope

Conversation

@evanlucas
Copy link
Contributor

@evanlucas evanlucas commented Dec 15, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

repl

Description of change

Previously, autocompletion of scoped packages was not supported by the
repl due to not including the @ character in the regular expression.

@nodejs-github-bot nodejs-github-bot added repl Issues and PRs related to the REPL subsystem. lts-watch-v6.x labels Dec 15, 2016
@evanlucas
Copy link
Contributor Author

evanlucas commented Dec 15, 2016

CI: https://ci.nodejs.org/job/node-test-pull-request/5426/

bnoordhuis
bnoordhuis reviewed Dec 16, 2016
View reviewed changes
test/parallel/test-repl-tab-complete.js Outdated
Copy link
Member

@bnoordhuis bnoordhuis Dec 16, 2016

Choose a reason for hiding this comment

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

Creating files or directories outside of common.tmpDir is kind of yuck. I think it should be possible to process.chdir(common.fixturesDir), then you can check in @nodejsscope/ into the existing node_modules directory.

Copy link
Contributor Author

@evanlucas evanlucas Dec 16, 2016

Choose a reason for hiding this comment

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

100% agree. I'll try and get that working. That should simplify a lot of this test. Thanks!

@evanlucas
Copy link
Contributor Author

evanlucas commented Dec 16, 2016

Updated. PTAL. Thanks! CI: https://ci.nodejs.org/job/node-test-pull-request/5437/

bnoordhuis
bnoordhuis approved these changes Dec 16, 2016
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM with a comment.

test/common.js Outdated
Copy link
Member

@bnoordhuis bnoordhuis Dec 16, 2016

Choose a reason for hiding this comment

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

You can drop this change now.

@evanlucas
repl: allow autocompletion for scoped packages
d04b6ff 
Previously, autocompletion of scoped packages was not supported by the
repl due to not including the `@` character in the regular expression.
jasnell pushed a commit that referenced this pull request Dec 23, 2016
@evanlucas @jasnell
repl: allow autocompletion for scoped packages
e248f7f 
Previously, autocompletion of scoped packages was not supported by the
repl due to not including the `@` character in the regular expression.

PR-URL: #10296
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Dec 23, 2016

Landed in e248f7f

@jasnell jasnell closed this Dec 23, 2016
@evanlucas evanlucas mentioned this pull request Jan 3, 2017
Merged
evanlucas added a commit that referenced this pull request Jan 3, 2017
@evanlucas
repl: allow autocompletion for scoped packages
a402652 
Previously, autocompletion of scoped packages was not supported by the
repl due to not including the `@` character in the regular expression.

PR-URL: #10296
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
evanlucas added a commit that referenced this pull request Jan 4, 2017
@evanlucas
repl: allow autocompletion for scoped packages
6796bf4 
Previously, autocompletion of scoped packages was not supported by the
repl due to not including the `@` character in the regular expression.

PR-URL: #10296
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@evanlucas evanlucas deleted the replscope branch January 5, 2017 23:23
MylesBorins pushed a commit that referenced this pull request Jan 23, 2017
@evanlucas @MylesBorins
repl: allow autocompletion for scoped packages
b38be64 
Previously, autocompletion of scoped packages was not supported by the
repl due to not including the `@` character in the regular expression.

PR-URL: #10296
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

MylesBorins commented Jan 23, 2017

this will need to be manually backported to v4.x

MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
@evanlucas @MylesBorins
repl: allow autocompletion for scoped packages
6d92391 
Previously, autocompletion of scoped packages was not supported by the
repl due to not including the `@` character in the regular expression.

PR-URL: #10296
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
@evanlucas @MylesBorins
repl: allow autocompletion for scoped packages
57c4c6f 
Previously, autocompletion of scoped packages was not supported by the
repl due to not including the `@` character in the regular expression.

PR-URL: #10296
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins added a commit that referenced this pull request Feb 21, 2017
@MylesBorins
2017-02-21, Version 6.10.0 'Boron' (LTS)
1b6fd44 
Notable Changes:

The SEMVER-MINOR changes include:

*  crypto: allow adding extra certs to well-known CAs (Sam Roberts)
   #9139
*  deps: Upgrade INTL ICU to version 58 (Steven R. Loomis)
   #9234
*  process: add `process.memoryUsage.external` (Fedor Indutny)
   #9587
*  src: add wrapper for process.emitWarning() (Sam Roberts)
   #9139

Notable SEMVER-PATCH changes include:

*  fs: cache non-symlinks in realpathSync. (Jeremy Yallop)
   #10253
*  repl: allow autocompletion for scoped packages (Evan Lucas)
   #10296
MylesBorins added a commit that referenced this pull request Feb 22, 2017
@MylesBorins
2017-02-21, Version 6.10.0 'Boron' (LTS)
aecb085 
Notable Changes:

The SEMVER-MINOR changes include:

*  crypto: allow adding extra certs to well-known CAs (Sam Roberts)
   #9139
*  deps: Upgrade INTL ICU to version 58 (Steven R. Loomis)
   #9234
*  process: add `process.memoryUsage.external` (Fedor Indutny)
   #9587
*  src: add wrapper for process.emitWarning() (Sam Roberts)
   #9139

Notable SEMVER-PATCH changes include:

*  fs: cache non-symlinks in realpathSync. (Jeremy Yallop)
   #10253
*  repl: allow autocompletion for scoped packages (Evan Lucas)
   #10296

PR-URL: #10974
imyller added a commit to imyller/meta-nodejs that referenced this pull request Mar 2, 2017
@imyller
2017-02-21, Version 6.10.0 'Boron' (LTS)
edbc1bf 
    Notable Changes:

    The SEMVER-MINOR changes include:

    *  crypto: allow adding extra certs to well-known CAs (Sam Roberts)
       nodejs/node#9139
    *  deps: Upgrade INTL ICU to version 58 (Steven R. Loomis)
       nodejs/node#9234
    *  process: add `process.memoryUsage.external` (Fedor Indutny)
       nodejs/node#9587
    *  src: add wrapper for process.emitWarning() (Sam Roberts)
       nodejs/node#9139

    Notable SEMVER-PATCH changes include:

    *  fs: cache non-symlinks in realpathSync. (Jeremy Yallop)
       nodejs/node#10253
    *  repl: allow autocompletion for scoped packages (Evan Lucas)
       nodejs/node#10296

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
imyller added a commit to imyller/meta-nodejs that referenced this pull request Mar 2, 2017
@imyller
2017-02-21, Version 6.10.0 'Boron' (LTS)
f99e583 
    Notable Changes:

    The SEMVER-MINOR changes include:

    *  crypto: allow adding extra certs to well-known CAs (Sam Roberts)
       nodejs/node#9139
    *  deps: Upgrade INTL ICU to version 58 (Steven R. Loomis)
       nodejs/node#9234
    *  process: add `process.memoryUsage.external` (Fedor Indutny)
       nodejs/node#9587
    *  src: add wrapper for process.emitWarning() (Sam Roberts)
       nodejs/node#9139

    Notable SEMVER-PATCH changes include:

    *  fs: cache non-symlinks in realpathSync. (Jeremy Yallop)
       nodejs/node#10253
    *  repl: allow autocompletion for scoped packages (Evan Lucas)
       nodejs/node#10296

Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

@cjihrig cjihrig cjihrig approved these changes

+2 more reviewers

@Fishrock123 Fishrock123 Fishrock123 approved these changes

@princejwesley princejwesley princejwesley approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

repl Issues and PRs related to the REPL subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@evanlucas @jasnell @MylesBorins @bnoordhuis @addaleax @Fishrock123 @princejwesley @cjihrig @nodejs-github-bot