chore: bumping tabtab to 2.2.2 #595

willmendesneto · 2018-07-15T12:48:30Z

Purpose of this pull request?

Documentation update
Bug fix
Enhancement
Other, please explain: upgrade a package dependency

What changes did you make?

Upgrade tabtab to v2 and, since the default the old method was deprecated, some code changes were applied to support this version.

Is there anything you'd like reviewers to focus on?

Refers to #508

Ping @SBoudrias @mischah @mklabs @sindresorhus

willmendesneto · 2018-07-15T12:58:39Z

Folks, I can't simulate this issue on Travis-CI locally, the tests are passing for me. Could you take a look at that, please?

NodeJS: v8.9.4
NPM: 5.6.0
YO: 2.0.3
OS: macOS High Sierra 10.13.5

mklabs · 2018-07-16T13:07:20Z

@willmendesneto Thanks for dealing with this issue. I really appreciate that. 👍

willmendesneto · 2018-07-16T13:10:53Z

@mklabs My pleasure, mate. Do you know if the is there any system dependency that we need to install in this new version?

mklabs · 2018-07-16T13:25:31Z

@willmendesneto I don't think so. But maybe Travis prevents tabtab from touching files like ~/.bashrc. Tabtab needs to add a line in there.

mklabs · 2018-07-16T13:35:27Z

@willmendesneto I'm available right now if you'd like to talk about this issue, if you can have a call I can hangout maybe.

mklabs · 2018-07-16T14:06:31Z

I forked your fork @willmendesneto Trying to run these tests on Windows 10 (git bash on windows) and got an error.

willmendesneto · 2018-07-16T14:15:38Z

@mklabs If you don't mind, can we try that tomorrow? These tests are passing locally, but they're not passing on Travis-CI. I'm suspecting that can be related to some OS dependency that are not installed on Travis-CI agent. The problem is that I can't check that running on debug mode.

In the meantime, that will be great if you can check the changes between the versions 1.3.2 and 2.0.0 later. The code start to break on Travis after this version (which was the release after the one that yo is currently using, based on npm website).

mklabs · 2018-07-16T14:18:16Z

@willmendesneto No worries, we can do that tomorrow. Meanwhile, I'm trying to run them on Windows. I may open up a pull request on your repository to test out Travis. Have a great day or night ^^

mklabs · 2018-07-16T18:24:42Z

After hours fighting against git bash and npm links, I finally managed to reproduce the error locally (with Bash on Windows). There is indeed an issue I can't figure out right now. I'll take a close look tomorrow.

mklabs · 2018-07-17T14:10:19Z

I'm still not sure what to do exactly, but it seems like we can't use both package.json completion and event emitter in https://github.com/willmendesneto/yo/blob/bumping-tabtab/lib/completion/index.js#L16

If I comment out this line, I get completion results based on package.json file.

cmd: DEBUG="tabtab*" COMP_POINT="4" COMP_LINE="yo " COMP_CWORD="1" node /mnt/c/Users/mk/dev/yo/lib/completion/index.js completion -- yo
error null
OUTPUT -f
--force
--version
--no-color
--no-insight
--insight
--generators

STDERR Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete handle: options {"name":"yo","cache":true,"ttl":300000}
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete handle: env {"args":["completion","--","yo"],"complete":true,"words":1,"point":4,"line":"yo ","partial":"yo ","last":"","lastPartial":"","prev":"yo"}
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete handle: env.line yo
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete complete package {"args":["completion","--","yo"],"complete":true,"words":1,"point":4,"line":"yo ","partial":"yo ","last":"","lastPartial":"","prev":"yo"}
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete TEST Received ["-f","--force","--version","--no-color","--no-insight","--insight","--generators"]
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete completion item -f string
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete completion item --force string
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete completion item --version string
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete completion item --no-color string
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete completion item --no-insight string
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete completion item --insight string
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete completion item --generators string
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete TEST results ["-f","--force","--version","--no-color","--no-insight","--insight","--generators"]
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete results: ["-f","--force","--version","--no-color","--no-insight","--insight","--generators"] {"args":["completion","--","yo"],"complete":true,"words":1,"point":4,"
line":"yo ","partial":"yo ","last":"","lastPartial":"","prev":"yo"}
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete Sendind events yo
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete With env {"args":["completion","--","yo"],"complete":true,"words":1,"point":4,"line":"yo ","partial":"yo ","last":"","lastPartial":"","prev":"yo"}
Tue, 17 Jul 2018 14:06:39 GMT tabtab:complete Emit event (yo)

      ✓ Returns the completion candidates for both options and installed generators (1717ms)

I need more time working on this, but I feel like those completion results should be handled in the completer file instead of in package.json

mklabs · 2018-07-17T17:51:55Z

Related pull request on @willmendesneto repository: willmendesneto#1

mklabs

Great work

mklabs · 2018-07-18T15:40:58Z

.travis.yml

+  -  npm install -s --no-progress
+before_script:
+  - npm run tabtab-install
+  - source /home/travis/.bashrc


As far as I can tell (now that I worked a bit on this, sorry for my previous answers), and unless I didn't understood the expected installation step, I'm not sure you need this anymore. It'll be done by yo completion and up to the user to choose when and how to setup the bridge in its bashrc (or zshrc, or fish config).

The tests don't really test the autocompletion but the actual output of yo-completer (on yo completion -- yo <command or options>)

mklabs · 2018-07-18T15:42:04Z

lib/cli.js

  name: 'yo',
-  completer: 'yo-complete'
+  completer: 'yo-complete',
+  cache: !process.env.YO_TEST


The cache was a nightmare to test and to prone for errors, I decided to remove this feature in the upcoming tabtab version

mklabs · 2018-07-18T15:42:49Z

package.json

@@ -19,6 +19,7 @@
    "postupdate": "yodoctor",
    "pretest": "xo",
    "test": "nyc mocha --timeout=10000",
+    "tabtab-install": "tabtab install --auto --name yo --completer yo-complete",


If the user is intended to use yo completion command to install the completion mechanism, I think this is not needed anymore.

mklabs · 2018-07-18T15:43:47Z

package.json

@@ -64,7 +65,7 @@
    "root-check": "^1.0.0",
    "sort-on": "^3.0.0",
    "string-length": "^2.0.0",
-    "tabtab": "^1.3.2",
+    "tabtab": "^2.2.2",


As soon as it's ready (probably this weekend), it'll need to be bumped to ^3.0.0

Okay, we'll hold off merging until then.

Sounds good to me 👍 I want to take a little bit more time. I won't be able to work much more this weekend, and I want to take time with Wilson to coordinate our efforts. Thanks @SBoudrias

mklabs · 2018-07-18T15:45:49Z

test/completion.js

@@ -29,28 +29,26 @@ describe('Completion', () => {
  });

  describe('Test completion STDOUT output', () => {
-    it('Returns the completion candidates for both options and installed generators', done => {
+    it.only('Returns the completion candidates for both options and installed generators', () => {
      const yocomplete = path.join(__dirname, '../lib/completion/index.js');
      const yo = path.join(__dirname, '../lib/cli');

      let cmd = 'export cmd="yo" && YO_TEST=true DEBUG="tabtab*" COMP_POINT="4" COMP_LINE="$cmd" COMP_CWORD="$cmd"';


Here, you probably need to add a space at the end of the line, otherwise the cmd will look like something along these lines:

... COMP_CWORD="$cmd"node ...

mklabs · 2018-07-18T15:47:35Z

test/completion.js

-          return;
+      exec(cmd, (error, out, stderr) => {
+        console.log('stdout: ', out);
+        console.log('stderr: ', stderr);


Testing and logging stderr here indeed helps a ton, you might get debugging statement this way.

willmendesneto · 2018-08-20T21:36:59Z

Closing this pull request since it needs more time to apply some changes on tabtab package.

willmendesneto force-pushed the bumping-tabtab branch 6 times, most recently from c8f1760 to 91ae562 Compare July 16, 2018 13:00

willmendesneto force-pushed the bumping-tabtab branch from 12c7c0c to 91ae562 Compare July 16, 2018 13:14

willmendesneto force-pushed the bumping-tabtab branch from 8f3fd69 to 2dbae58 Compare July 16, 2018 14:11

willmendesneto added 9 commits July 19, 2018 00:15

chore: bumping tabtab to 2.2.2

42611fa

chore: fixing typo

e4c1af7

fix: trying to use version 2.0.0

43b3b20

ci(.travis.yml): running without sudo

8c68a21

ci(.travis.yml): running with sudo

2d394c2

chore: forcing tabtab install step

83217b7

chore: forcing tabtab install step

186d649

chore: forcing tabtab install step

349f72d

fix: applying travis-ci changes

9c2a056

willmendesneto force-pushed the bumping-tabtab branch from 2dbae58 to 9c2a056 Compare July 18, 2018 14:29

fix: adding return on completion command

a988891

willmendesneto added 12 commits July 19, 2018 00:44

fix: debugging travis-ci

06c3a29

fix: debugging env vars

ebe8a60

fix: reloading .bashrc

2179442

fix: using execSync

6fc9bd0

fix: using execFileSync in tests

674f3a7

fix: using execFileSync in tests

2d9b7a4

fix: using execSync in tests

c2e00b6

fix: using execSync in tests

9ec9c09

fix: using exec in tests

7f9a957

fix: removing cmd from last parameter

ca660ba

fix: removing cmd from parameter

be0cc7d

fix: running as bash command

7b70227

mklabs reviewed Jul 18, 2018

View reviewed changes

fix: applying feedback

23a0055

willmendesneto closed this Aug 20, 2018

MikeMcC399 mentioned this pull request Oct 13, 2024

Deprecations from tabtab@1.3.2 #815

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: bumping tabtab to 2.2.2 #595

chore: bumping tabtab to 2.2.2 #595

willmendesneto commented Jul 15, 2018 •

edited

Loading

willmendesneto commented Jul 15, 2018 •

edited

Loading

mklabs commented Jul 16, 2018

willmendesneto commented Jul 16, 2018

mklabs commented Jul 16, 2018

mklabs commented Jul 16, 2018

mklabs commented Jul 16, 2018

willmendesneto commented Jul 16, 2018

mklabs commented Jul 16, 2018

mklabs commented Jul 16, 2018

mklabs commented Jul 17, 2018

mklabs commented Jul 17, 2018

mklabs left a comment

mklabs Jul 18, 2018

mklabs Jul 18, 2018

mklabs Jul 18, 2018 •

edited

Loading

mklabs Jul 18, 2018

SBoudrias Jul 19, 2018

mklabs Jul 19, 2018

mklabs Jul 18, 2018

mklabs Jul 18, 2018

willmendesneto commented Aug 20, 2018

chore: bumping tabtab to 2.2.2 #595

chore: bumping tabtab to 2.2.2 #595

Conversation

willmendesneto commented Jul 15, 2018 • edited Loading

Purpose of this pull request?

What changes did you make?

Is there anything you'd like reviewers to focus on?

willmendesneto commented Jul 15, 2018 • edited Loading

mklabs commented Jul 16, 2018

willmendesneto commented Jul 16, 2018

mklabs commented Jul 16, 2018

mklabs commented Jul 16, 2018

mklabs commented Jul 16, 2018

willmendesneto commented Jul 16, 2018

mklabs commented Jul 16, 2018

mklabs commented Jul 16, 2018

mklabs commented Jul 17, 2018

mklabs commented Jul 17, 2018

mklabs left a comment

Choose a reason for hiding this comment

mklabs Jul 18, 2018

Choose a reason for hiding this comment

mklabs Jul 18, 2018

Choose a reason for hiding this comment

mklabs Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

mklabs Jul 18, 2018

Choose a reason for hiding this comment

SBoudrias Jul 19, 2018

Choose a reason for hiding this comment

mklabs Jul 19, 2018

Choose a reason for hiding this comment

mklabs Jul 18, 2018

Choose a reason for hiding this comment

mklabs Jul 18, 2018

Choose a reason for hiding this comment

willmendesneto commented Aug 20, 2018

willmendesneto commented Jul 15, 2018 •

edited

Loading

willmendesneto commented Jul 15, 2018 •

edited

Loading

mklabs Jul 18, 2018 •

edited

Loading