repl: added support for custom completions. by diosney · Pull Request #7527 · nodejs/node · GitHub

Checklist

make -j4 test (UNIX), or vcbuild test nosign (Windows) passes

It threw errors but not related to the new changes for what I can say.
I ran ./node ./test/parallel/./test/parallel/test-repl-tab-complete.js and it does it OK. At the end is the error log JIC.

UPDATE:

It seems that is an issue with my dev environment, @lance (see in commets) ran the tests and they ran OK.

tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

repl, doc

Description of change

Original description of PR, since this is an old PR of mine to the extint v0.13.x, references of old PR at the end.

Currently I'm working on a project that uses the REPL to implement a mid complex CLI app found out that the REPL module doesn't have a way to override the default readline complete() function, which is used for TAB completions of commands.

I think that the REPL module can benefit from this addition, and specifically the repl applications that may wants to use this functionality.

I added the docs & tests, and the tests passed fine in my environment, this is just a little change, after all.

Simple usage example:

var repl = require('repl')
  .start({
    terminal : true,
    completer: function completer(line) {
      var completions = 'aaa aa1 aa2 bbb ccc ddd eee'.split(' ');

      var hits = completions.filter(function (item) {
        return item.indexOf(line) == 0;
      });

      // Show all completions if none was found.
      return [
        hits.length
          ? hits
          : completions,
        line
      ];
    }
  });

References for the old PR:

nodejs/node-v0.x-archive#8484

Error thrown by make -j4 test:

make -C out BUILDTYPE=Release V=1
make[1]: Entering directory '/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out'
  touch _media_diosney_3E2AC0882AC03F21_Users_diosney_Documents_Dev_node_deps_v8_inspector_platform_v8_inspector_v8_inspector_gyp_protocol_sources_target_generateV8InspectorProtocolBackendSources.intermediate
  LD_LIBRARY_PATH=/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out/Release/lib.host:/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out/Release/lib.target:$LD_LIBRARY_PATH; export LD_LIBRARY_PATH; cd ../deps/v8_inspector/platform/v8_inspector; mkdir -p /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out/Release/obj/gen/blink/platform/v8_inspector/protocol; python ../inspector_protocol/CodeGenerator.py --protocol js_protocol.json --string_type String16 --export_macro PLATFORM_EXPORT --output_dir "/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out/Release/obj/gen/blink/platform/v8_inspector/protocol" --output_package platform/v8_inspector/protocol
rm _media_diosney_3E2AC0882AC03F21_Users_diosney_Documents_Dev_node_deps_v8_inspector_platform_v8_inspector_v8_inspector_gyp_protocol_sources_target_generateV8InspectorProtocolBackendSources.intermediate
make[1]: Leaving directory '/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out'
ln -fs out/Release/node node
make build-addons
make[1]: Entering directory '/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node'
make -C out BUILDTYPE=Release V=1
make[2]: Entering directory '/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out'
  touch _media_diosney_3E2AC0882AC03F21_Users_diosney_Documents_Dev_node_deps_v8_inspector_platform_v8_inspector_v8_inspector_gyp_protocol_sources_target_generateV8InspectorProtocolBackendSources.intermediate
  LD_LIBRARY_PATH=/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out/Release/lib.host:/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out/Release/lib.target:$LD_LIBRARY_PATH; export LD_LIBRARY_PATH; cd ../deps/v8_inspector/platform/v8_inspector; mkdir -p /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out/Release/obj/gen/blink/platform/v8_inspector/protocol; python ../inspector_protocol/CodeGenerator.py --protocol js_protocol.json --string_type String16 --export_macro PLATFORM_EXPORT --output_dir "/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out/Release/obj/gen/blink/platform/v8_inspector/protocol" --output_package platform/v8_inspector/protocol
rm _media_diosney_3E2AC0882AC03F21_Users_diosney_Documents_Dev_node_deps_v8_inspector_platform_v8_inspector_v8_inspector_gyp_protocol_sources_target_generateV8InspectorProtocolBackendSources.intermediate
make[2]: Leaving directory '/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out'
ln -fs out/Release/node node
make[1]: Leaving directory '/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node'
make cctest
make[1]: Entering directory '/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node'
make -C out BUILDTYPE=Release V=1
make[2]: Entering directory '/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out'
  touch _media_diosney_3E2AC0882AC03F21_Users_diosney_Documents_Dev_node_deps_v8_inspector_platform_v8_inspector_v8_inspector_gyp_protocol_sources_target_generateV8InspectorProtocolBackendSources.intermediate
  LD_LIBRARY_PATH=/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out/Release/lib.host:/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out/Release/lib.target:$LD_LIBRARY_PATH; export LD_LIBRARY_PATH; cd ../deps/v8_inspector/platform/v8_inspector; mkdir -p /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out/Release/obj/gen/blink/platform/v8_inspector/protocol; python ../inspector_protocol/CodeGenerator.py --protocol js_protocol.json --string_type String16 --export_macro PLATFORM_EXPORT --output_dir "/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out/Release/obj/gen/blink/platform/v8_inspector/protocol" --output_package platform/v8_inspector/protocol
rm _media_diosney_3E2AC0882AC03F21_Users_diosney_Documents_Dev_node_deps_v8_inspector_platform_v8_inspector_v8_inspector_gyp_protocol_sources_target_generateV8InspectorProtocolBackendSources.intermediate
make[2]: Leaving directory '/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/out'
ln -fs out/Release/node node
Running main() from gtest_main.cc
[==========] Running 22 tests from 2 test cases.
[----------] Global test environment set-up.
[----------] 4 tests from UtilTest
[ RUN      ] UtilTest.ListHead
[       OK ] UtilTest.ListHead (0 ms)
[ RUN      ] UtilTest.StringEqualNoCase
[       OK ] UtilTest.StringEqualNoCase (0 ms)
[ RUN      ] UtilTest.StringEqualNoCaseN
[       OK ] UtilTest.StringEqualNoCaseN (0 ms)
[ RUN      ] UtilTest.ToLower
[       OK ] UtilTest.ToLower (0 ms)
[----------] 4 tests from UtilTest (0 ms total)

[----------] 18 tests from InspectorSocketTest
[ RUN      ] InspectorSocketTest.ReadsAndWritesInspectorMessage
[       OK ] InspectorSocketTest.ReadsAndWritesInspectorMessage (1 ms)
[ RUN      ] InspectorSocketTest.BufferEdgeCases
[       OK ] InspectorSocketTest.BufferEdgeCases (0 ms)
[ RUN      ] InspectorSocketTest.AcceptsRequestInSeveralWrites
[       OK ] InspectorSocketTest.AcceptsRequestInSeveralWrites (0 ms)
[ RUN      ] InspectorSocketTest.ExtraTextBeforeRequest
[       OK ] InspectorSocketTest.ExtraTextBeforeRequest (1 ms)
[ RUN      ] InspectorSocketTest.ExtraLettersBeforeRequest
[       OK ] InspectorSocketTest.ExtraLettersBeforeRequest (0 ms)
[ RUN      ] InspectorSocketTest.RequestWithoutKey
[       OK ] InspectorSocketTest.RequestWithoutKey (0 ms)
[ RUN      ] InspectorSocketTest.KillsConnectionOnProtocolViolation
[       OK ] InspectorSocketTest.KillsConnectionOnProtocolViolation (1 ms)
[ RUN      ] InspectorSocketTest.CanStopReadingFromInspector
[       OK ] InspectorSocketTest.CanStopReadingFromInspector (0 ms)
[ RUN      ] InspectorSocketTest.CloseDoesNotNotifyReadCallback
[       OK ] InspectorSocketTest.CloseDoesNotNotifyReadCallback (1 ms)
[ RUN      ] InspectorSocketTest.CloseWorksWithoutReadEnabled
[       OK ] InspectorSocketTest.CloseWorksWithoutReadEnabled (0 ms)
[ RUN      ] InspectorSocketTest.ReportsHttpGet
[       OK ] InspectorSocketTest.ReportsHttpGet (2 ms)
[ RUN      ] InspectorSocketTest.HandshakeCanBeCanceled
[       OK ] InspectorSocketTest.HandshakeCanBeCanceled (0 ms)
[ RUN      ] InspectorSocketTest.GetThenHandshake
[       OK ] InspectorSocketTest.GetThenHandshake (1 ms)
[ RUN      ] InspectorSocketTest.WriteBeforeHandshake
[       OK ] InspectorSocketTest.WriteBeforeHandshake (0 ms)
[ RUN      ] InspectorSocketTest.CleanupSocketAfterEOF
[       OK ] InspectorSocketTest.CleanupSocketAfterEOF (3 ms)
[ RUN      ] InspectorSocketTest.EOFBeforeHandshake
[       OK ] InspectorSocketTest.EOFBeforeHandshake (1 ms)
[ RUN      ] InspectorSocketTest.Send1Mb
[       OK ] InspectorSocketTest.Send1Mb (19 ms)
[ RUN      ] InspectorSocketTest.ErrorCleansUpTheSocket
[       OK ] InspectorSocketTest.ErrorCleansUpTheSocket (1 ms)
[----------] 18 tests from InspectorSocketTest (31 ms total)

[----------] Global test environment tear-down
[==========] 22 tests from 2 test cases ran. (31 ms total)
[  PASSED  ] 22 tests.
make[1]: Leaving directory '/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node'
/usr/bin/python tools/test.py --mode=release -J \
        addon doctool known_issues message pseudo-tty parallel sequential
=== release test-child-process-uid-gid ===                                     
Path: parallel/test-child-process-uid-gid
assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: Missing expected exception..
    at _throws (assert.js:337:5)
    at Function.assert.throws (assert.js:361:3)
    at Object.<anonymous> (/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-child-process-uid-gid.js:8:8)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
    at Module.runMain (module.js:575:10)
    at run (bootstrap_node.js:340:7)
Command: out/Release/node /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-child-process-uid-gid.js
=== release test-fs-access ===                                                 
Path: parallel/test-fs-access
assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: Got unwanted exception..
    at _throws (assert.js:348:5)
    at Function.assert.doesNotThrow (assert.js:366:3)
    at Object.<anonymous> (/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-fs-access.js:105:8)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
    at Module.runMain (module.js:575:10)
    at run (bootstrap_node.js:340:7)
Command: out/Release/node /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-fs-access.js
=== release test-fs-append-file ===              
Path: parallel/test-fs-append-file
assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: 448 == 384
    at /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-fs-append-file.js:84:12
    at FSReqWrap.oncomplete (fs.js:123:15)
Command: out/Release/node /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-fs-append-file.js
=== release test-fs-append-file-sync ===                    
Path: parallel/test-fs-append-file-sync
assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: 448 == 384
    at Object.<anonymous> (/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-fs-append-file-sync.js:61:10)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
    at Module.runMain (module.js:575:10)
    at run (bootstrap_node.js:340:7)
    at startup (bootstrap_node.js:132:9)
    at bootstrap_node.js:455:3
Command: out/Release/node /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-fs-append-file-sync.js
=== release test-fs-chmod ===                              
Path: parallel/test-fs-chmod
33279
assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: 420 == 511
    at /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-fs-chmod.js:73:14
    at FSReqWrap.oncomplete (fs.js:123:15)
Command: out/Release/node /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-fs-chmod.js
=== release test-fs-readdir-ucs2 ===                                    
Path: parallel/test-fs-readdir-ucs2
fs.js:640
  return binding.open(pathModule._makeLong(path), stringToFlags(flags), mode);
                 ^

Error: Unknown system error -84: Unknown system error -84, open '/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/tmp.3/=��'
    at Error (native)
    at Object.fs.openSync (fs.js:640:18)
    at Object.<anonymous> (/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-fs-readdir-ucs2.js:19:17)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
    at Module.runMain (module.js:575:10)
    at run (bootstrap_node.js:340:7)
Command: out/Release/node /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-fs-readdir-ucs2.js
=== release test-fs-write-file ===                                       
Path: parallel/test-fs-write-file
assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: 448 == 384
    at /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-fs-write-file.js:60:12
    at FSReqWrap.oncomplete (fs.js:123:15)
Command: out/Release/node /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-fs-write-file.js
=== release test-fs-write-file-sync ===                                  
Path: parallel/test-fs-write-file-sync
assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: 493 == 511
    at Object.<anonymous> (/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-fs-write-file-sync.js:38:8)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
    at Module.runMain (module.js:575:10)
    at run (bootstrap_node.js:340:7)
    at startup (bootstrap_node.js:132:9)
    at bootstrap_node.js:455:3
Command: out/Release/node /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-fs-write-file-sync.js
=== release test-https-connect-address-family ===                              
Path: parallel/test-https-connect-address-family
events.js:160
      throw er; // Unhandled 'error' event
      ^

Error: getaddrinfo EAI_AGAIN localhost:12346
    at Object.exports._errnoException (util.js:1007:11)
    at errnoException (dns.js:33:15)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:79:26)
Command: out/Release/node /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-https-connect-address-family.js
=== release test-net-better-error-messages-port-hostname ===                   
Path: parallel/test-net-better-error-messages-port-hostname
assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: 'EAI_AGAIN' == 'ENOTFOUND'
    at Socket.<anonymous> (/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-net-better-error-messages-port-hostname.js:11:10)
    at Socket.<anonymous> (/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/common.js:407:15)
    at emitOne (events.js:96:13)
    at Socket.emit (events.js:188:7)
    at connectErrorNT (net.js:1016:8)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)
    at process._tickCallback (internal/process/next_tick.js:98:9)
Command: out/Release/node /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-net-better-error-messages-port-hostname.js
=== release test-net-connect-immediate-finish ===                       
Path: parallel/test-net-connect-immediate-finish
assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: 'EAI_AGAIN' === 'ENOTFOUND'
    at Socket.<anonymous> (/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-net-connect-immediate-finish.js:11:10)
    at Socket.<anonymous> (/media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/common.js:407:15)
    at Socket.g (events.js:286:16)
    at emitOne (events.js:96:13)
    at Socket.emit (events.js:188:7)
    at connectErrorNT (net.js:1016:8)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)
    at process._tickCallback (internal/process/next_tick.js:98:9)
Command: out/Release/node /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-net-connect-immediate-finish.js
=== release test-repl-history-perm ===                                         
Path: parallel/test-repl-history-perm
assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: REPL history file should be mode 0600
    at /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-repl-history-perm.js:40:10
    at /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/common.js:407:15
    at REPLServer.<anonymous> (internal/repl.js:180:7)
    at REPLServer.g (events.js:286:16)
    at emitNone (events.js:86:13)
    at REPLServer.emit (events.js:185:7)
    at onwritten (internal/repl.js:215:14)
    at FSReqWrap.wrapper [as oncomplete] (fs.js:747:5)
Command: out/Release/node --expose_internals /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-repl-history-perm.js
=== release test-tls-connect-address-family ===                                
Path: parallel/test-tls-connect-address-family
events.js:160
      throw er; // Unhandled 'error' event
      ^

Error: getaddrinfo EAI_AGAIN localhost:12346
    at Object.exports._errnoException (util.js:1007:11)
    at errnoException (dns.js:33:15)
    at GetAddrInfoReqWrap.onlookup [as oncomplete] (dns.js:79:26)
Command: out/Release/node /media/diosney/3E2AC0882AC03F21/Users/diosney/Documents/Dev/node/test/parallel/test-tls-connect-address-family.js
[01:28|% 100|+ 1146|-  13]: Done                                               
Makefile:118: recipe for target 'test' failed
make: *** [test] Error 1

This change looks like it introduces a number of whitespace-related lint errors. Try running make jslint (which would run after the tests in make test if there weren't any test failures).

why is this binding necessary? (It looks like the context handling is already done by the apply, below.)

@Fishrock123 It is because the completer function is passed immediately to the Interface.completer option, so if is removed, then the function will not have the correct binding when executed by the readline module.

The apply binding is for the case when the complete function is called directly through the REPL instance, such as the tests.

Is this still viable or maybe you can give me any idea on what change I need to make in order to be accepted?

However, I will change this to self for consistency, it seems that I missed that.

@Trott You are right, it seems that my IDE added those on save. Sorry for that, I will fix it.

Maybe we can add the make jslint into the CONTRIBUTING guide in a separate PR to let other know about it? It could helped me to know about it.

@Trott @Fishrock123 I added a commit with the code correctly linted, can you check it out and let me know about other changes? Thanks!

Do I have to squash commits into one? Tell me if so.

@Trott I added another PR fo the docs on make lint, can you review it too?

It is @ #7534

Any ideas on why the those tests are failing?

Since my dev partition is in ecryptfs I had to move the node project to a NTFS partition in the same machine, this could be the issue? They are failing even on master, without any changes applied.

I strongly suspect NFS will mess up some of the fs tests. The other issues appear to be DNS-configuration related, perhaps. Not sure if these comments are helpful or not...

NFS != NTFS, this seems more like permission mixups, and that is probably outside of Node’s scope… only the getaddrinfo failures seem a bit weird, that’s true.

FWIW, I checked out your branch and ran the tests with no failures.

"will be used"
Nit: I would prefer an active tense usage here, such as An optional function that is used for custom tab auto completion.

@lance: Updated doc to fix your nit. Check it out now and let me know.

@lance

FWIW, I checked out your branch and ran the tests with no failures.

Nice! Thanks for running the tests for me!

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

Is the bind() necessary here?

@cjihrig See #7527 (comment)

Have you tested and seen this behavior? Prior to this change, complete() was not bound. You also don't seem to need to bind any of the custom completers in your tests.

complete() was not bound, but the inner self.complete was, which I think it makes complete.bind a must to not broken its implementation, since it relies on this.

What can we further improve FICS and IMHO, is whether:

custom options.completer needs to be binded too or not, since users could have access to useful REPL properties like this.lines, used in the default completer provided by the REPL.
the apply is deemed as not needed if the params are directly coded in REPL.prototype.complete, which will require then some logic to pass to the self.completer the correct parameters since it accepts one for sync operation and two for async operation.

I see what you mean. Could you add a test that fails when it is not bound? I just pulled down your changes, removed the bind(), and the entire test suite still passed.

@cjihrig You're right. I delve a little more and now I know exactly why it works without the extra binding:

1- In the following code snippet complete will set the correct this to self since it is defined as part of the object self, so it doesn't matter the implementation, but the object that defined it as part of one of its attributes.

That will allow to call REPL.completer always with the correct binding.

self.completer = (typeof options.completer === 'function')
    ? options.completer
    : complete;

2- The next snippet, enforces the this on Interface, so it does resets the internal this used by Interface.completer in the readline module, so it will always work: so no extra binding needed.

Interface.call(this, {
    input: self.inputStream,
    output: self.outputStream,
    completer: self.completer,
    terminal: options.terminal,
    historySize: options.historySize,
    prompt
  });

3- As long as the complete function is not called by directly, but through the REPL related method, the this will have the correct reference, and since all tests use replInstance.complete, then it works.

So, in conclusion, it seems that while 3 is enforced, meaning no one calls directly complete without setting the proper this, inside the repl file, it is not needed the extra binding :) so I will remove it and update the thread.

Code updated.

The FreeBSD failure on CI is an unrelated flaky test that will be fixed in #7555.

LGTM

CI: https://ci.nodejs.org/job/node-test-commit/4026/

repl: added support for custom completions.

1953eab

nodejs-github-bot added the repl Issues and PRs related to the REPL subsystem. label Jul 3, 2016

Fishrock123 reviewed Jul 4, 2016
View reviewed changes

diosney added 2 commits July 4, 2016 12:12

repl: Make linter happy.

30c680b

repl: Removed unneeded console.log call.

3e368ec

lance reviewed Jul 5, 2016
View reviewed changes

jasnell added the semver-minor PRs that contain new features and should be released in the next minor version. label Jul 5, 2016

repl: Improved doc on completer function.

095d147

diosney mentioned this pull request Jul 6, 2016

docs: Added information on how to run the linter. #7534

Closed

2 tasks

cjihrig reviewed Jul 6, 2016
View reviewed changes

diosney added 2 commits July 6, 2016 10:34

repl: Changed vars for consts in new tests.

44688dd

repl: Removed unneeded extra binding.

a89f9fe

                    `undefined`. Defaults to `false`.
                 * `writer` {Function} The function to invoke to format the output of each
                    command before writing to `output`. Defaults to [`util.inspect()`][].
+                * `completer` {Function} An optional function that will used for custom Tab

repl: added support for custom completions. by diosney · Pull Request #7527 · nodejs/node · GitHub

Uh oh!

Uh oh!

repl: added support for custom completions. - #7527

repl: added support for custom completions.#7527diosney wants to merge 6 commits intonodejs:masternodejs/node:masterfrom diosney:repl-add-custom-completion-supportdiosney/node:repl-add-custom-completion-supportCopy head branch name to clipboard

Conversation

diosney commented Jul 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Affected core subsystem(s)

Description of change

Uh oh!

Trott commented Jul 4, 2016

Uh oh!

Fishrock123 Jul 4, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diosney Jul 4, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diosney commented Jul 4, 2016

Uh oh!

diosney commented Jul 4, 2016

Uh oh!

diosney commented Jul 4, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

diosney commented Jul 5, 2016

Uh oh!

Trott commented Jul 5, 2016

Uh oh!

addaleax commented Jul 5, 2016

Uh oh!

lance commented Jul 5, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diosney commented Jul 5, 2016

Uh oh!

Trott commented Jul 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Trott commented Jul 6, 2016

Uh oh!

cjihrig commented Jul 7, 2016

Uh oh!

jasnell commented Jul 7, 2016

Uh oh!

lance commented Jul 7, 2016

Uh oh!

lance commented Jul 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

repl: added support for custom completions.#7527
diosney wants to merge 6 commits into
nodejs:masterfrom
diosney:repl-add-custom-completion-support

diosney commented Jul 3, 2016 •
edited
Loading

Fishrock123 Jul 4, 2016 •
edited
Loading

diosney Jul 4, 2016 •
edited
Loading

diosney commented Jul 4, 2016 •
edited
Loading