dsifford / yarn-completion

Bash completion for Yarn
MIT License
277 stars 25 forks source link

Targets with a : don't work correctly (special characters require escaping) #47

Closed dkarlovi closed 4 years ago

dkarlovi commented 4 years ago

Example (generated by Nest.js CLI):

{
    "name": "hello-nest-js",
    "version": "0.0.1",
    "description": "",
    "author": "",
    "private": true,
    "license": "UNLICENSED",
    "scripts": {
        "prebuild": "rimraf dist",
        "build": "nest build",
        "format": "prettier --write \"src/**/*.ts\" \"test/**/*.ts\"",
        "start": "nest start",
        "start:dev": "nest start --watch",
        "start:debug": "nest start --debug --watch",
        "start:prod": "node dist/main",
        "lint": "eslint \"{src,apps,libs,test}/**/*.ts\" --fix",
        "test": "jest",
        "test:watch": "jest --watch",
        "test:cov": "jest --coverage",
        "test:debug": "node --inspect-brk -r tsconfig-paths/register -r ts-node/register node_modules/.bin/jest --runInBand",
        "test:e2e": "jest --config ./test/jest-e2e.json"
    },
    "dependencies": {
        "@nestjs/common": "^7.0.0",
        "@nestjs/core": "^7.0.0",
        "@nestjs/platform-express": "^7.0.0",
        "reflect-metadata": "^0.1.13",
        "rimraf": "^3.0.2",
        "rxjs": "^6.5.4"
    },
    "devDependencies": {
        "@nestjs/cli": "^7.0.0",
        "@nestjs/schematics": "^7.0.0",
        "@nestjs/testing": "^7.0.0",
        "@types/express": "^4.17.6",
        "@types/jest": "25.1.4",
        "@types/node": "^13.9.1",
        "@types/supertest": "^2.0.8",
        "@typescript-eslint/eslint-plugin": "^2.23.0",
        "@typescript-eslint/parser": "^2.23.0",
        "eslint": "^6.8.0",
        "eslint-config-prettier": "^6.10.0",
        "eslint-plugin-import": "^2.20.1",
        "jest": "^25.1.0",
        "prettier": "^1.19.1",
        "supertest": "^4.0.2",
        "ts-jest": "25.2.1",
        "ts-loader": "^6.2.1",
        "ts-node": "^8.6.2",
        "tsconfig-paths": "^3.9.0",
        "typescript": "^3.7.4"
    },
    "jest": {
        "moduleFileExtensions": [
            "js",
            "json",
            "ts"
        ],
        "rootDir": "src",
        "testRegex": ".spec.ts$",
        "transform": {
            "^.+\\.(t|j)s$": "ts-jest"
        },
        "coverageDirectory": "../coverage",
        "testEnvironment": "node"
    }
}

Sample:

$ yarn run <TAB><TAB>
build        lint         start:debug  test         test:e2e     
env          prebuild     start:dev    test:cov     test:watch   
format       start        start:prod   test:debug   
$ yarn run test<TAB><TAB>
test        test:cov    test:debug  test:e2e    test:watch  
$ yarn run test:<TAB><TAB>
$ yarn run test:c<TAB><TAB>overage 
yarn run v1.21.1
error Command "test:coverage" not found.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
dsifford commented 4 years ago

I'm not sure I understand what the error here is.

There is not a test:coverage command. That is a valid error.

dkarlovi commented 4 years ago

It was autocompleted, for whatever reason. Notice that cov was not offered, for example.

Also, nothing gets completed with just test:.

dkarlovi commented 4 years ago

Oh, just figured out why coverage was completed, I have a folder named like that:

$ ls
coverage  nest-cli.json  package.json  src   tsconfig.build.json  yarn.lock
dist      node_modules   README.md     test  tsconfig.json
$ touch x123
$ yarn run test:x<TAB>123 ^C
dsifford commented 4 years ago

Did you try the suggestion in the bottom of the readme?

dkarlovi commented 4 years ago

Just now (didn't notice it before). It behaves the same, only the options are presented differently. It would still complete coverage, for example.

dsifford commented 4 years ago

Then yeah not sure I'm able to do anything about it. I tried looking into this in the past and came up short.

Happy to hear suggestions though.

dsifford commented 4 years ago

I've got package.json files with a similar convention and have found it not a big deal to do yarn t<Tab><Tab><Tab> to get test:cov completed.

dkarlovi commented 4 years ago

This looks related: https://stackoverflow.com/a/12495727/672885

dsifford commented 4 years ago

Already do that

https://github.com/dsifford/yarn-completion/blob/2821c67d204bbb6ac9f3d49f04a6d3c100a2a1e3/yarn-completion.bash#L1028

dkarlovi commented 4 years ago

No, this specifically avoids redefining it, quote:

modifying COMP_WORDBREAKS in your completion script is not safe (as it is a global variable and it has the side effect of affecting the behavior of other completion scripts - for example scp). Therefore, bash completion offers some helper methods which you can use to achieve your goal in a better and more safer way.

I've checked with Maven, which uses this and the difference is their completion escapes the character:

$ mvn <TAB><TAB>
assembly:assembly                  dependency:sources
clean                              dependency:tree
compile                            dependency:unpack
dependency:analyze                 dependency:unpack-dependencies
dependency:analyze-dep-mgt         deploy
dependency:analyze-duplicate       eclipse:eclipse
dependency:analyze-only            idea:idea
dependency:analyze-report          install
dependency:build-classpath         package
dependency:copy                    plexus:app
dependency:copy-dependencies       plexus:bundle-application
dependency:get                     plexus:bundle-runtime
dependency:go-offline              plexus:descriptor
dependency:list                    plexus:runtime
dependency:properties              plexus:service
dependency:purge-local-repository  site
dependency:resolve                 test/
dependency:resolve-plugins         verify
$ mvn depe<TAB>ndency\:<TAB>
dependency:analyze                 dependency:list
dependency:analyze-dep-mgt         dependency:properties
dependency:analyze-duplicate       dependency:purge-local-repository
dependency:analyze-only            dependency:resolve
dependency:analyze-report          dependency:resolve-plugins
dependency:build-classpath         dependency:sources
dependency:copy                    dependency:tree
dependency:copy-dependencies       dependency:unpack
dependency:get                     dependency:unpack-dependencies
dependency:go-offline              
$ mvn dependency\:ana<TAB>lyze<TAB>
dependency:analyze            dependency:analyze-only
dependency:analyze-dep-mgt    dependency:analyze-report
dependency:analyze-duplicate  
dkarlovi commented 4 years ago

Adding the backslash manually does work, I get test\:cov completed correctly (but it again removes the backslash).

Escaping special chars like colons, spaced etc would probably fix it without other changes.

dsifford commented 4 years ago

Fixed in #48